Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Merged by Bors] - Initial attempt at making resource requests, limits and pvcs configurable #265

Closed
wants to merge 23 commits into from

Conversation

soenkeliebau
Copy link
Member

@soenkeliebau soenkeliebau commented May 4, 2022

Description

Fixes #252

This doesn't work with the currently released version of the operator-rs, and is only intended for code study, as this might serve as a template for making similar changes in other operators.

Most notably it depends on stackabletech/operator-rs#368 and a PR to make small changes to the Resource structs to make them work with the Merging code, which can in turn only be created once the above mentioned PR has been merged.

Please add a description here. This will become the commit message of the merge request later.

Review Checklist

  • Code contains useful comments
  • (Integration-)Test cases added (or not applicable)
  • Documentation added (or not applicable)
  • Changelog updated (or not applicable)
  • Cargo.toml only contains references to git tags (not specific commits or branches)
  • Helm chart can be installed and deployed operator works (or not applicable)

Once the review is done, comment bors r+ (or bors merge) to merge. Further information

@soenkeliebau soenkeliebau marked this pull request as draft May 4, 2022 21:10
@soenkeliebau soenkeliebau requested review from razvan and nightkr May 4, 2022 21:11
@razvan razvan linked an issue May 19, 2022 that may be closed by this pull request
3 tasks
@razvan razvan marked this pull request as ready for review May 19, 2022 16:06
soenkeliebau and others added 8 commits June 1, 2022 16:08
# Conflicts:
#	rust/operator-binary/src/controller.rs
…o the best practice in https://dzone.com/articles/best-practices-java-memory-arguments-for-container

Relevant quote:

If you are running *only your Java application* within the container, then set initial heap size (i.e., using either one of ‘-XX:InitialRAMFraction’, ‘-XX:InitialRAMPercentage’, -Xms) to the same size as max heap size. Setting initial heap size and max heap has certain advantages. One of them is: you will incur lower Garbage Collection pause times. Because whenever heap size grows from the initial allocated size, it will pause the JVM. It can be circumvented when you set initial and max heap sizes to be the same. Besides that, if you have an under-allocated container’s memory size, then JVM will not even start (which is better than experiencing OutOfMemoryError when transactions are in flight).
…ets into main first) and enabled CI for testing (same deal, also from #277).
soenkeliebau and others added 3 commits June 8, 2022 10:40
Addressed comment by Razvan

Co-authored-by: Razvan-Daniel Mihai <[email protected]>
@soenkeliebau soenkeliebau requested review from razvan and nightkr June 8, 2022 15:08
Copy link
Member

@razvan razvan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@soenkeliebau
Copy link
Member Author

bors merge

bors bot pushed a commit that referenced this pull request Jun 9, 2022
…able (#265)

## Description
Fixes #252

This doesn't work with the currently released version of the operator-rs, and is only intended for code study, as this might serve as a template for making similar changes in other operators.

Most notably it depends on stackabletech/operator-rs#368 and a PR to make small changes to the Resource structs to make them work with the Merging code, which can in turn only be created once the above mentioned PR has been merged.


*Please add a description here. This will become the commit message of the merge request later.*



Co-authored-by: Razvan-Daniel Mihai <[email protected]>
@bors
Copy link
Contributor

bors bot commented Jun 9, 2022

Pull request successfully merged into main.

Build succeeded:

@bors bors bot changed the title Initial attempt at making resource requests, limits and pvcs configurable [Merged by Bors] - Initial attempt at making resource requests, limits and pvcs configurable Jun 9, 2022
@bors bors bot closed this Jun 9, 2022
@bors bors bot deleted the feat/resource_config branch June 9, 2022 10:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make resources configurable (CPU, memory, PVCs)
3 participants