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.

Sign up for GitHub

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

Jump to bottom

Set CPU & Memory requests for compaction job #853

Merged
merged 1 commit into from
Aug 9, 2024
Merged

Set CPU & Memory requests for compaction job #853

merged 1 commit into from
Aug 9, 2024

Conversation

anveshreddy18
Copy link
Contributor

@anveshreddy18 anveshreddy18 commented Aug 8, 2024 •
edited by ishan16696
Loading

How to categorize this PR?

/area backup
/kind enhancement

What this PR does / why we need it:

This PR adds CPU and Memory requests for compaction job. The requests are calculated from seed clusters and heavy load shoot clusters. This is more from a Gardener point of view. To understand why I set these numbers, you can visit the internal backlog issue 14 to know more.

Compaction Jobs originating from druid needs to have Requests & Limits ( limits can be configured after observing the landscapes with just requests set initially ) so as to allow them to get scheduled to nodes with proper resource availability. This will let the scheduler know about the consumption/expectation before hand so that it can schedule optimally so that the compaction jobs can get completed and also not interfere with resources intended for other processes.

Which issue(s) this PR fixes:
Fixes part of #707

Special notes for your reviewer:

Release note:

set cpu and memory requests for compaction pods

@anveshreddy18 anveshreddy18 requested a review from a team as a code owner August 8, 2024 08:19
@gardener-robot gardener-robot added area/backup Backup related kind/enhancement Enhancement, improvement, extension labels Aug 8, 2024
@gardener-robot
Copy link

@anveshreddy18 You have mentioned internal references in the public. Please check.

@anveshreddy18 anveshreddy18 self-assigned this Aug 8, 2024
@gardener-robot gardener-robot added needs/review Needs review size/xs Size of pull request is tiny (see gardener-robot robot/bots/size.py) labels Aug 8, 2024
@gardener-robot-ci-3 gardener-robot-ci-3 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Aug 8, 2024
@gardener-robot-ci-2 gardener-robot-ci-2 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Aug 8, 2024
@anveshreddy18 anveshreddy18 added this to the v0.23.0 milestone Aug 8, 2024
@ishan16696
Copy link
Member

/retest

unmarshall
unmarshall approved these changes Aug 9, 2024
View reviewed changes
Copy link
Contributor

@unmarshall unmarshall left a comment

Choose a reason for hiding this comment

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

Looks good. Please remove the link to github.tools.sap. You can just mention the issue number instead.

ishan16696
ishan16696 approved these changes Aug 9, 2024
View reviewed changes
Copy link
Member

@ishan16696 ishan16696 left a comment

Choose a reason for hiding this comment

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

LGTM

@anveshreddy18 anveshreddy18 merged commit c6231d0 into gardener:master Aug 9, 2024
11 checks passed
@gardener-robot gardener-robot added the status/closed Issue is closed (either delivered or triaged) label Aug 9, 2024
@ishan16696
Copy link
Member

Hi @anveshreddy18 ,
I think this PR is fixing this issue: #652. Can you please close this issue

@anveshreddy18
Copy link
Contributor Author

anveshreddy18 commented Aug 9, 2024 •
edited
Loading

Hi @ishan16696, this PR currently only adds requests but we didn't add limits yet. But that issue is both for requests & limits. I think we can keep it open? or should I edit it to have check list for requests & limits and tick the requests part?

Edit: For now I've added tasks for requests & limits and checked requests.

@unmarshall
Copy link
Contributor

I am not sure if we need limits. Adding requests are good enough for now.

@ishan16696
Copy link
Member

I am not sure if we need limits. Adding requests are good enough for now.

yes, I agree. For now we needed the requests which have been added and right now, requests values are hardcoded so we anyway have to revisit this to make it configurable for which this issue is already in open: #707 which we aren't closing. IMO, one issue is fine to keep a track of it.

@anveshreddy18
Copy link
Contributor Author

Yeah, I closed that now.

anveshreddy18 added a commit to anveshreddy18/etcd-druid that referenced this pull request Sep 19, 2024
@anveshreddy18
Set CPU & Memory requests for compaction job (gardener#853)
715eee0
@anveshreddy18 anveshreddy18 mentioned this pull request Sep 19, 2024
Merged
anveshreddy18 added a commit that referenced this pull request Sep 20, 2024
@anveshreddy18
Set CPU & Memory requests for compaction job (#853) (#878)
c34a115
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ishan16696 ishan16696 ishan16696 approved these changes

@unmarshall unmarshall unmarshall approved these changes

Assignees

@anveshreddy18 anveshreddy18

Labels
area/backup Backup related kind/enhancement Enhancement, improvement, extension needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) needs/review Needs review size/xs Size of pull request is tiny (see gardener-robot robot/bots/size.py) status/closed Issue is closed (either delivered or triaged)
Projects
None yet
Milestone
v0.23.0
Development

Successfully merging this pull request may close these issues.
6 participants
@anveshreddy18 @gardener-robot @ishan16696 @unmarshall @gardener-robot-ci-2 @gardener-robot-ci-3