Repository navigation
Conversation
…g cross-process reproducible (verl-project#7666) Under full_determinism=True, GlobalRequestLoadBalancer used Python's built-in hash(request_id) % len(self._servers) over list(self._servers). Because Python's hash(str) uses a randomized seed (PYTHONHASHSEED) per process and list(self._servers) depends on dictionary insertion order, identical request IDs produced divergent replica assignments across worker processes and training runs. This replaces hash() with zlib.crc32() over sorted(self._servers.keys()), guaranteeing platform- and process-independent deterministic routing. Adds unit tests verifying cross-process hash consistency across different PYTHONHASHSEED environments and server insertion orders.
f8a7df7 to
b2b5f4f
Compare
mynotwo
left a comment
There was a problem hiding this comment.
Thanks for this. Switching hash() to zlib.crc32 is a good change: routing no longer depends on PYTHONHASHSEED being set, which main_ppo.py does today (os.environ["PYTHONHASHSEED"] = str(rollout_cfg.seed) before ray.init, forwarded by get_ppo_ray_runtime_env), but other entry points may not.
I left one inline comment on the sorted() part.
| # arrival timing, which varies run-to-run, so it is bypassed entirely here. | ||
| # Using sorted server keys and crc32 ensures platform and process independence | ||
| # without relying on PYTHONHASHSEED or dict insertion order. | ||
| sorted_servers = sorted(self._servers.keys()) |
There was a problem hiding this comment.
One concern with sorted(self._servers.keys()). The keys are server_address strings (f"{ip}:{port}"), and the port comes from get_free_port, so it changes from run to run. Sorting by address makes the index→replica mapping depend on those ports. The insertion order it replaces follows server_addresses, which is in replica rank order.
I checked this with a small simulation. It used 4 replicas on 2 nodes and 64 det-{i} ids, with ports redrawn in each of 20 runs and PYTHONHASHSEED pinned the way main_ppo does it:
| same request grouping across runs | same request → replica-rank map | |
|---|---|---|
main (hash + insertion order) |
20/20 | 20/20 |
this PR (crc32 + sorted address) |
20/20 | 7/20 |
Which requests are batched together is unaffected. On homogeneous replicas with VLLM_BATCH_INVARIANT=1, outputs should not change either. But the PR describes "the same request always routes to the same replica across runs", and under that reading, replica identity becomes less stable than before. Replica identity matters on mixed hardware, and when per-replica logs are compared across runs.
Suggestion: keep crc32, but index into insertion order (list(self._servers)) rather than sorted addresses, or sort by a stable replica rank if one is available. The insertion-order test could then check that rank order is preserved rather than ignored. Happy to share the simulation script if useful.
What does this PR do?
Fixes #7666.
Under
full_determinism=True,GlobalRequestLoadBalancerinverl/workers/rollout/router.pypreviously routed requests vialist(self._servers)[hash(request_id) % len(self._servers)].This caused non-deterministic request-to-replica distribution across runs and worker processes due to:
hash(str)utilizing a process-randomized seed (PYTHONHASHSEED), producing different values in separate Python processes or subsequent runs.list(self._servers)depending on dictionary insertion order.This PR replaces
hash()withzlib.crc32()oversorted(self._servers.keys()), ensuring platform- and process-independent deterministic routing (consistent with the determinism strategy inverl/experimental/reward_loop/router/naive_router.py).Checklist Before Starting
[{modules}] {type}: {description}(This will be checked by the CI)Test
TestFullDeterminismRoutingintests/workers/rollout/test_router_on_cpu.py:test_full_determinism_cross_process_reproducibility: Validates that separate Python subprocesses initialized with distinctPYTHONHASHSEEDvalues (0,42,12345,random) yield identical request-to-server assignments.test_full_determinism_insertion_order_invariant: Validates that varying dictionary insertion orders produce identical routing.test_full_determinism_sticky_session_and_clear: Validates sticky session retention and cache clearing.tests/workers/rollout/test_router_on_cpu.py(25 passed).ruff check,ruff format --check).API and Usage Example
No external API changes. Users configuring
actor_rollout_ref.rollout.full_determinism=truewill now obtain identical request-to-replica assignments across distributed worker processes and training runs.Design & Code Changes
verl/workers/rollout/router.py:acquire_server: Compute server index usingzlib.crc32(request_id.encode("utf-8")) % len(sorted_servers)oversorted(self._servers.keys()).tests/workers/rollout/test_router_on_cpu.py:TestFullDeterminismRoutingsuite.Checklist Before Submitting
pre-commit install && pre-commit run --all-files --show-diff-on-failure --color=always