Conversation
Resolved conflicts in: - crates/autopilot/src/infra/api.rs - crates/autopilot/src/run.rs - crates/configs/src/autopilot/solver.rs - crates/driver/Cargo.toml - crates/e2e/src/setup/services.rs - crates/shared/src/lib.rs Kept main's simpler approach for autopilot configuration while integrating feature branch changes for delta sync API and thin solve request support.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces delta sync functionality to reduce solver payload size and lays the foundation for Unified Auction. It refactors solvable orders caching to emit structured change events, adds delta-sync infrastructure and replica logic in the driver, extends DTOs and headers for protocol versions, and includes end-to-end coverage. The changes are comprehensive and well-tested, addressing various aspects of incremental updates and thin solve requests. The severity of the comments has been adjusted to medium.
crates/autopilot/src/run_loop.rs
Outdated
| } else { | ||
| None | ||
| }; | ||
| Metrics::solve_request_body_size(full_request.body_size()); |
There was a problem hiding this comment.
The solve_request_body_size metric currently only records the size of the full_request. If a thin_request is selected and sent to a driver, this metric will be inaccurate, as it will still reflect the larger full request size. This can lead to misleading performance monitoring data regarding the actual payload sizes being sent.
To ensure accurate metrics, please record the body_size() of the request variable after it has been selected by Self::select_solve_request_for_driver.
let selected_request = Self::select_solve_request_for_driver(
driver.supports_thin_solve_request,
full_request,
thin_request,
);
Metrics::solve_request_body_size(selected_request.body_size());| #[test] | ||
| fn delta_sync_resync_retry_values_are_cached() { | ||
| static LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); | ||
| let _lock = LOCK.lock().expect("lock poisoned"); | ||
|
|
||
| let attempts_expected = DELTA_SYNC_RESYNC_RETRY_ATTEMPTS.get().copied().unwrap_or(7); | ||
| let delay_expected = DELTA_SYNC_RESYNC_RETRY_DELAY | ||
| .get() | ||
| .copied() | ||
| .unwrap_or_else(|| Duration::from_millis(123)); | ||
|
|
||
| let _ = DELTA_SYNC_RESYNC_RETRY_ATTEMPTS.set(7); | ||
| let _ = DELTA_SYNC_RESYNC_RETRY_DELAY.set(Duration::from_millis(123)); | ||
|
|
||
| assert_eq!(delta_sync_resync_retry_attempts(), attempts_expected); | ||
| assert_eq!(delta_sync_resync_retry_delay(), delay_expected); | ||
|
|
||
| let _ = DELTA_SYNC_RESYNC_RETRY_ATTEMPTS.set(9); | ||
| let _ = DELTA_SYNC_RESYNC_RETRY_DELAY.set(Duration::from_millis(999)); | ||
|
|
||
| assert_eq!(delta_sync_resync_retry_attempts(), attempts_expected); | ||
| assert_eq!(delta_sync_resync_retry_delay(), delay_expected); | ||
| } |
There was a problem hiding this comment.
The test delta_sync_resync_retry_values_are_cached attempts to test the caching behavior of OnceLock static variables. However, OnceLocks can only be set once. If this test is run as part of a larger suite, or if the OnceLocks are initialized by another test or previous execution, subsequent set calls within this test will fail silently (return Err). This can lead to flaky test results or incorrect assertions.
To ensure reliable testing, please ensure that the OnceLocks are in an uninitialized state before running this test, perhaps by using a test-specific setup/teardown or a dedicated test guard that resets static state if possible, or by only asserting the get() value without attempting to set() if the OnceLock is expected to be initialized elsewhere.
… improve state management
…initialization and parsing logic
|
Closing this issue as is violates the contribution guide.
|
2 participants
Description
Implements auction delta sync to reduce solver payload size by sending incremental updates instead of full orderbook snapshots. This lays the foundation for Unified Auction by introducing structured change events and a driver-side replica that can consume deltas while still serving full auctions to solvers.
Changes
How to Test
Fixes #4207