#848 Adopt bulkman 2.0.1 - pin >=2.0.0,<3 and verify shutdown semantics change

closed medium dependencies Created 2026-08-03 20:48 · Updated 2026-08-03 21:23

Description

Edit
EARS SPEC: - The Highway engine shall pin bulkman to >=2.0.0,<3 so the major-version adoption is explicit rather than resolved by accident from an open >=1.2.0 pin. - When a worker shuts down while tasks are still in flight, the Highway engine shall not tear down shared resources (database pool, services) while bulkhead threads are still executing. - The Highway engine shall classify a post-shutdown execute() rejection as a retryable condition (leave the task for redelivery), not as the tasks own permanent failure. - The Highway engine test suite shall pass against the INSTALLED bulkman wheel, not merely against the pin. CONTEXT: bulkman sent two fix-notices on the agent-mail bus (2.0.0 thread thr-758e7795438d42b1a953, 2.0.1 thread thr-2e54acf6a65f40278b57). Our docker/highway-core/pyproject.toml pins bulkman>=1.2.0, so a lock refresh resolves to the new major. VERIFIED ALREADY: the installed wheel in the running image IS 2.0.1 - the open pin already resolved to it during todays rebuilds. Every test run today therefore executed against 2.0.1. THEIR CLAIM 4 IS WRONG ABOUT US - report back. bulkman wrote: "Your code paths I read dont call shutdown() - if thats accurate, no impact." We call bulkhead.shutdown(wait=True, timeout=5.0) in THREE places: docker/highway-core/engine/cli/worker/worker_core.py:305 docker/highway-core/engine/services/activity_worker/worker.py:202 enterprise/services/activity_worker.py:620 <- in the enterprise repo, which they cannot see They read only the engine repo, so the third was invisible to them; the first two they missed. IMPACT ANALYSIS (why it is still probably benign, and where it is not): - Both call sites already drain in-flight work THEMSELVES before calling shutdown (worker_core waits up to 30s for get_active_task_count()==0; the activity worker checks _active_futures), and both wrap the call in try/except Exception. So in the normal path shutdown is a no-op cleanup and the honored-timeout change is invisible. - THE REAL CHANGE IS IN THE UNHAPPY PATH: if tasks are STILL running after our own 30s drain, pre-2.0.0 shutdown(wait=True) blocked until every task finished; 2.0.0 returns after timeout=5.0. The process then proceeds to _cleanup_services() and closes the database pool WHILE those threads are still executing. That is a new window for "connection closed underneath a running task". - Terminal shutdown (execute after shutdown raises) is low risk here: our shutdown call is the last thing in the teardown path, with no execute() after it. - The 2.0.1 typed BulkheadShutdownError is caught by our existing except Exception, so no code change is required to remain correct - but we should classify it explicitly rather than swallowing it as a generic warning.

Comments

Loading comments...

Context

Loading context...

Audit History

View All
Loading audit history...