Ma77Ball commented on code in PR #4900:
URL: https://github.com/apache/texera/pull/4900#discussion_r3179645514
##########
amber/src/main/python/core/architecture/managers/statistics_manager.py:
##########
@@ -80,13 +76,7 @@ def increase_control_processing_time(self, time: int) ->
None:
raise ValueError("Time must be non-negative")
self._control_processing_time += time
- def update_total_execution_time(self, time: int) -> None:
- if time < self._worker_start_time:
- raise ValueError(
- "Current time must be greater than or equal to worker start
time"
- )
- self._total_execution_time = time - self._worker_start_time
-
- def initialize_worker_start_time(self, time: int) -> None:
- # Set the worker start time
- self._worker_start_time = time
+ def increase_idle_time(self, time: int) -> None:
+ if time < 0:
+ raise ValueError("Time must be non-negative")
Review Comment:
I can revert to my first commit, which does not change the logic and instead
enforces a max(0,...) clamp.
What I liked about the claude suggested change was tracking idle time
directly as its own accumulator in the run() loop, rather than deriving it from
total - data - control, makes the logic simpler and avoids the negative-idle
scenario by construction. However, I do see your point with the drift over
time.
I'll revert to the subtraction-based approach and add assertions on
method-call order to catch misuse at the source, plus a warning and floor-at-0
in get_statistics() so a bad value never reaches WorkerStatistics if drift is
detected.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
