-
Notifications
You must be signed in to change notification settings - Fork 9
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
Node densityn ignore iterations #60
Node densityn ignore iterations #60
Conversation
rh-pre-commit.version: 2.2.0 rh-pre-commit.check-secrets: ENABLED Signed-off-by: Auto User <[email protected]>
c491145
to
2bc669e
Compare
ah! Nice! yeah this is something at @shashank-boyapally and I have spoken about in the past given number of pods can change based on what is installed, so iterations might (very likely) drift a bit. As long as the node count is the same, we should be somewhere in the ballpark! |
@vishnuchalla can you speak to the failures here? not familiar with what/why this is broken? |
We will need to get the ES secret updated and re-trigger the tests. Will keep you posted on it. |
This might take use somewhere close to the solution, but we will only get exact apples-to-apples runs once we start filtering based on iterations that are being calculated in the runtime. In order to achieve that I remember we discussing to populate iterations count in the |
Well, even populating in the For example, there could be situations where we are +/- 1 pod off (even with the same version,platform,etc), so the iteration calculation for node-density workloads would differ slightly. This change would allow us to just say, match runs with the same node-count... Correct me if I am off here @paigerube14 |
If our idea was to filter out runs based on number of nodes, then I am good with it. My only question was though we have same number of nodes, there can be difference in number of iterations being calculated in the runtime (.i.e especially for node-density runs). So can we consider runs with same number of nodes to be apples to apples comparison even if they are executed with different number of iterations? OR do we want to take code changes to match iteration in a follow up PR? |
After conducting a set of experiments, noticed that secrets do not get propagated into forks and aren't available during pull requests. They only work against the main repo, for example. So I am going to change the CI tests workflow to Fix PR: #64 |
lgtm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the add, lgtm!
Type of change
Description
This change will allow us to match uuids no matter there jobIterations. This is important for node-density because the number of iterations is calculated based on how many nodes are currently on the cluster and this number can fluctuate.
Related Tickets & Documents
Checklist before requesting a review
Testing
The old way of running produces less results than the new flag
orion cmd --config examples/trt-external-payload-node-density.yaml --lookback 5d --hunter-analyze
orion cmd --config examples/trt-external-payload-node-density.yaml --lookback 5d --hunter-analyze --node-count True