Yicong-Huang commented on code in PR #4900:
URL: https://github.com/apache/texera/pull/4900#discussion_r3179604120
##########
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 am not a fan of "calculating a time by adding time intervals". Because
this method will easily have accuracy problem. say 1.1 + 2.4 = 3.5 but with int
you will have 1 + 2 = 3. this gradually adds up.
I still prefer total elapse time = (end time - start time).
The bug here is idle time can be negative, if we call methods in a wrong
way. We should find ways to prevent it from happening. For example, using
assertions on method orders, or warnings when drifts are detected and return a
0 for all negative. Please do not change how we calculate time.
--
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]
