fix: request admission edge cases#685
Conversation
|
MkDocs preview: https://cbed668e.dd-docs-preview.pages.dev Fern preview: https://nvidia-preview-pr-685.docs.buildwithfern.com/nemo/datadesigner
|
Greptile SummaryThis PR tightens two AIMD edge cases in
|
| Filename | Overview |
|---|---|
| packages/data-designer-engine/src/data_designer/engine/models/request_admission/controller.py | Two related fixes: stale-lease guard is tightened to full dataclass equality (was resource-only), and AIMD decrease condition replaces first_in_cascade with admitted_adaptive_limit <= prev_limit so subsequent cascades can still decrease the limit after the first burst has already cut it. |
| packages/data-designer-engine/src/data_designer/engine/dataset_builders/dataset_builder.py | Single-line change enabling request_pressure_advisory=True in the AsyncTaskScheduler constructor wiring; straightforward and now covered by test. |
| packages/data-designer-engine/tests/engine/models/request_admission/test_controller.py | Adds three new tests covering exact-lease stale rejection, single-decrease-per-burst, and re-decrease after a fresh cascade; all cases align with the new controller semantics. |
| packages/data-designer-engine/tests/engine/dataset_builders/test_async_builder_integration.py | Adds a monkeypatch-based integration test confirming request_pressure_advisory=True is forwarded to AsyncTaskScheduler by _prepare_async_run; approach is sound. |
Sequence Diagram
sequenceDiagram
participant Caller
participant RAC as AdaptiveRequestAdmissionController
participant State as AdaptiveRequestLimitState
Caller->>RAC: try_acquire(item)
RAC->>RAC: _acquire_locked(item, now)
Note over RAC: snapshot current_limit → lease.current_adaptive_limit
RAC-->>Caller: "RequestAdmissionLease (lease_id, current_adaptive_limit=N)"
Caller->>RAC: release(lease, outcome)
RAC->>RAC: "active = _active_leases.pop(lease_id)"
alt "active != lease (stale: any field mismatch)"
RAC->>RAC: "_active_leases[lease_id] = active (restore)"
RAC-->>Caller: "ReleaseResult(released=False, reason=stale_lease)"
else exact match
RAC->>RAC: _apply_outcome(state, resource, active.current_adaptive_limit, outcome, now, events)
alt "outcome == rate_limited"
RAC->>State: "consecutive_rate_limits += 1"
RAC->>RAC: "should_decrease = (admitted_adaptive_limit <= state.current_limit)"
alt should_decrease
RAC->>State: "current_limit = floor(current_limit * MD_factor)"
RAC->>State: "rate_limit_ceiling = min(ceiling, admitted_adaptive_limit)"
end
else "outcome == success (cooldown cleared)"
RAC->>State: "consecutive_rate_limits = 0"
RAC->>State: "success_streak += 1 → maybe increase current_limit"
end
RAC-->>Caller: "ReleaseResult(released=True, reason=released)"
end
Reviews (2): Last reviewed commit: "revert request admission docs changes" | Re-trigger Greptile
Signed-off-by: Eric W. Tramel <eric.tramel@gmail.com>
Signed-off-by: Eric W. Tramel <eric.tramel@gmail.com>
Summary: requires exact request admission leases on release; pins AIMD to one multiplicative cut per rate-limit cascade; enables request-pressure advisory in DatasetBuilder async scheduler wiring; updates docs to remove public ThrottleConfig guidance. Tests: request admission controller, async builder integration, RunConfig, full async scheduler, and git diff --check.