Skip to content

Conversation

rkuris
Copy link
Member

@rkuris rkuris commented Sep 26, 2025

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

Replaced the deprecated EstimateETA function with EtaTracker.
@rkuris rkuris self-assigned this Sep 26, 2025
@Copilot Copilot AI review requested due to automatic review settings September 26, 2025 00:19
@rkuris rkuris added the cleanup Code quality improvement label Sep 26, 2025
Copy link
Contributor

@Copilot Copilot AI left a 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 with timer.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.

@RodrigoVillar

This comment was marked as resolved.

@rkuris rkuris changed the title feat: Use the new ETA tracker for load tests feat: Use the new ETA tracker for re-execution tests Sep 29, 2025
Copy link
Collaborator

@aaronbuchwald aaronbuchwald left a 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.

Copy link
Contributor

@RodrigoVillar RodrigoVillar left a 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup Code quality improvement
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

3 participants