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

Rename duration based to cost #903

Merged

Conversation

bharathappali
Copy link
Member

This PR renames the duration based engine to cost

Also makes the engine to use 95th percentile for CPU and memory to give out the recommendations

Comment on lines 282 to 286
Double memRecUsage = CommonUtils.percentile(NINETY_FIFTH_PERCENTILE, memUsageList);
Double memRecUsageBuf = memRecUsage + (memRecUsage * MEM_USAGE_BUFFER_DECIMAL);

// Add a small buffer to the current usage spike max and add it to the current usage max
Double memRecSpike = CommonUtils.percentile(HUNDREDTH_PERCENTILE, spikeList);
Double memRecSpike = CommonUtils.percentile(NINETY_FIFTH_PERCENTILE, spikeList);
Copy link
Contributor

Choose a reason for hiding this comment

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

Please keep the memory calculations the same as before. Lowering Memory usage might cause the app to crash. Lowering CPU will only cause the app to stall.

Copy link
Member Author

Choose a reason for hiding this comment

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

Reverted the memory value to have 100th percentile

Copy link
Contributor

@dinogun dinogun left a comment

Choose a reason for hiding this comment

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

  1. Please make the appropriate test changes
  2. Update all Doc to indicate the "duration based" change to "cost"

Signed-off-by: bharathappali <abharath@redhat.com>
Signed-off-by: bharathappali <abharath@redhat.com>
Signed-off-by: bharathappali <abharath@redhat.com>
Signed-off-by: bharathappali <abharath@redhat.com>
@bharathappali bharathappali mentioned this pull request Aug 25, 2023
Signed-off-by: bharathappali <abharath@redhat.com>
Copy link
Contributor

@dinogun dinogun left a comment

Choose a reason for hiding this comment

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

LGTM

@dinogun dinogun merged commit ce3e9a1 into kruize:mvp_demo Sep 6, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants