Set Pyworker errror for Autoscaler to handle reliability descrease only for such - #39
Set Pyworker errror for Autoscaler to handle reliability descrease only for such#39nakinnubis wants to merge 5 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a mechanism to distinguish pyworker-related errors from other types of failures when reporting metrics to the autoscaler. This allows the autoscaler to handle reliability metrics more granularly, penalizing only pyworker errors rather than all failure types when calculating reliability scores.
Key Changes:
- Added
is_pyworker_errorboolean flag parameter to the internalpostfunction for distinguishing error types - Split failed request handling into two categories: status "Error" (pyworker errors) and other failures (e.g., "Rejected")
- Updated request reporting logic to send different
is_pyworker_errorflag values for each failure category
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
edgaratvast
left a comment
edgaratvast
left a comment
There was a problem hiding this comment.
some dead code cleanup required, otherwise looks good.
| @@ -180,28 +177,24 @@ async def post(report_addr: str, idxs: list[int], success_flag: bool) -> bool: | |||
| # Take a snapshot of what we plan to send this tick. | |||
| # New arrivals after this snapshot will remain in the queue for the next tick. | |||
| snapshot = list(self.model_metrics.requests_deleting) | |||
There was a problem hiding this comment.
requests_payload is now just snapshot, so this is now unnecessary
There was a problem hiding this comment.
I miswrote this: this was supposed to be "request_payload is now a snapshot", not "just snapshot, meaning that requests_payload contains all information contained in snapshot, so keeping a separate snapshot is now unnecessary.
Passing snapshot to post won't work work.
| @@ -180,28 +177,24 @@ async def post(report_addr: str, idxs: list[int], success_flag: bool) -> bool: | |||
| # Take a snapshot of what we plan to send this tick. | |||
| # New arrivals after this snapshot will remain in the queue for the next tick. | |||
| snapshot = list(self.model_metrics.requests_deleting) | |||
There was a problem hiding this comment.
I miswrote this: this was supposed to be "request_payload is now a snapshot", not "just snapshot, meaning that requests_payload contains all information contained in snapshot, so keeping a separate snapshot is now unnecessary.
Passing snapshot to post won't work work.
edgaratvast
left a comment
edgaratvast
left a comment
There was a problem hiding this comment.
We need to wait until the autoscaler deploys with the corresponding changes to merge this, but this looks good.
We want to handle reliability by sending distinguish error boolean flag for pyworker related error without penalizing all when it come to dropping reliability.