Extract OffloadingService and test the inference hot path (#45)#77
Open
ff225 wants to merge 1 commit into
Open
Extract OffloadingService and test the inference hot path (#45)#77ff225 wants to merge 1 commit into
ff225 wants to merge 1 commit into
Conversation
…t path (#45) Third and last step of #45. RequestHandler built the offloading algorithm, ran it, unpicked its candidate list and published the decision telemetry inline. OffloadingService now owns that, and returns an OffloadingDecision carrying everything telemetry needs to explain the choice: the layer, the reason, the candidates, the switch penalty and the estimated cost. Deriving the switch penalty and the cost estimate moves onto the decision object, where they belong — they are properties of the decision, not of the request. The handler is left orchestrating: it asks for a decision and reports it. Building the telemetry *events* stays in the handler on purpose: those need `message_data`, and moving them would couple the service to the transport message shape. The algorithms themselves are untouched: `src/server/offloading_algo/` has no changes. Only the caller moved. Adds tests for handle_device_inference_result, which #45 flagged as the most critical component with no dedicated tests: the EMA updates for device and edge layers, the variance recording, the edge-inference skip conditions, the offloading decision (including forced-local, and that the decision sees the freshly smoothed times), the background I/O scheduling for every output, and that the debug snapshot is immune to later mutation of the live tables. request_handler.py: 1158 -> 933 lines; coverage 48% -> 61%.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #45. Third and last step, after #74 (state) and #76 (recorder).
The #45 checklist
DeviceStateManager— Extract per-device state out of RequestHandler (#11) #74InferenceCycleRecorder— Serialize simulation CSV writes behind a recorder (#10) #76OffloadingService— this PRhandle_device_inference_result— this PROffloadingService
RequestHandlerbuilt the algorithm, ran it, unpicked its candidate list and published the decision telemetry inline.OffloadingServiceowns that now, returning anOffloadingDecisionthat carries everything telemetry needs to explain the choice: the layer, the reason, the candidates, the switch penalty and the estimated cost.Deriving the switch penalty and the cost estimate moves onto the decision object — they are properties of the decision, not of the request. The handler is left orchestrating: it asks for a decision and reports it.
Deliberate deviation from the issue. #45 says "OffloadingService (wrapper around
OffloadingAlgo+ decision logging)". The service owns the algorithm and the decision logging (append_and_publish_decision), but building the telemetry events stays in the handler: those needmessage_data, and moving them would couple the service to the transport message shape. Happy to move them if you disagree.The algorithms are untouched.
src/server/offloading_algo/has zero changes in this PR — only the caller moved. SameOffloadingContext, samecreate_offloading_algorithm, sameselect_offloading_layer().Tests for the hot path
#45 called
handle_device_inference_result"the most critical component [with] no dedicated tests".tests/unit/test_handle_device_inference_result.pynow covers:offloading_layer_index == -1, and pastultimo_layer);The handler is built with
object.__new__, so none of the heavy__init__(profiler, config loading, Firestore) runs.Result
request_handler.pyhandle_device_inference_resultrequest_handler.pycoverage287 passed, 1 skipped.
device_state.py,inference_recorder.pyandoffloading_service.pyall at 100% coverage. ruff clean.Being straight about what is left: the handler is meaningfully thinner and its state is now owned elsewhere, but
handle_device_inference_resultis still 279 lines — the bulk is telemetry event assembly. Every box on #45 is ticked and the god-class state problem is solved, but if you want that method genuinely short, extracting an event-builder is a natural fourth step. Say the word and I will file it.Note
Two tests moved out of
test_request_handler_helpers.pyintotests/unit/test_offloading_service.py, since they targeted methods that now live on the service. They gained coverage of branches nothing exercised before: theIndexErrormetadata fallback, forced-local, and theswitch_penaltyderivation (including candidates excluded from selection).