-
Notifications
You must be signed in to change notification settings - Fork 807
feat: Use the new ETA tracker for re-execution tests #4352
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
base: master
Are you sure you want to change the base?
Conversation
Replaced the deprecated EstimateETA function with EtaTracker.
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.
Pull Request Overview
This PR replaces the deprecated EstimateETA
function with the new EtaTracker
for better ETA estimation during VM re-execution load tests.
- Replaces deprecated
timer.EstimateETA
function withtimer.EtaTracker
- Adds proper initialization of ETA tracking with baseline samples
- Improves ETA calculation accuracy during block execution sequences
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
This comment was marked as resolved.
This comment was marked as resolved.
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.
Code change LGTM
Would you mind attaching logs from the two comparison runs before/after this PR to show that the ETA is more accurate with this change, so that there's at least one clear data point?
Approving regardless since either way this is replacing deprecated code.
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.
Code changes LGTM; also agree that a before-and-after comparison would be great here.
Replaced the deprecated EstimateETA function with EtaTracker.
Why this should be merged
Because the new EtaTracker is much better
How this works
Removed the deprecated call and replaced it with the new one
How this was tested
Reran the test and see that it now has better estimates
Need to be documented in RELEASES.md?
No