Add asynchronous workers#338
Conversation
dc4a20f to
f96a37e
Compare
dedd0c1 to
5dde6d5
Compare
| iteration. [`add_workers`](@ref) submits each worker as an individual | ||
| scheduler allocation, so on a busy cluster the scheduler can start them | ||
| independently as resources free up rather than needing to place every worker | ||
| at once. Each worker adds itself to a global pool once it has started and | ||
| loaded its code, and forward model runs are dispatched to workers as they | ||
| become available. Use [`@worker_setup`](@ref) instead of `@everywhere` to load | ||
| model code so that workers joining later are initialized correctly. Because |
There was a problem hiding this comment.
For this note, I would prefer it talks about the differences between the backends. A lot of content in this note is talking about the HPC Cluster Backends and the relevant functions. Could this be moved into the section for "HPC Cluster Backends"? Maybe it is worth another note about talking about the difference between @everywhere and @worker_setup.
| while !all(id -> id in pool.workers, ids) | ||
| (time() - tstart) > timeout && | ||
| error("Workers did not initialize within $(timeout)s") | ||
| sleep(0.5) |
There was a problem hiding this comment.
Is there a better solution than busy waiting? I don't think it need to be addressed in this PR, but there is probably a better solution.
| on any worker that joins later. Use this instead of `@everywhere` to load model | ||
| code for an asynchronous [`WorkerBackend`](@ref) calibration, where workers may | ||
| join after the calibration has started. |
There was a problem hiding this comment.
Can you add more emphasis or use an admonition? I don't think this emphasize the importance of using this macro (e.g. what can happen if you don't?).
| `using`/`import` statements run on the master first (to precompile once), and | ||
| the current source path is propagated so relative `include` works on workers. | ||
| As with `@everywhere`, local variables must be interpolated with `\$`. |
There was a problem hiding this comment.
Maybe I am not too familiar with the teminology, but I don't understand what master is here. Is it the same as the main process or is this specific to the Distributed.jl?
| # Run `cmd`, discarding its output. | ||
| _run_quiet(cmd) = run(pipeline(cmd; stdout = devnull, stderr = devnull)) |
There was a problem hiding this comment.
When this run, is the output of each of the forward model still saved in the logs?
|
|
||
| # Workers that have connected but are still loading code: present in `workers()` | ||
| # but not yet schedulable. Keeps `default_worker_pool` from pooling them early. | ||
| const INITIALIZING = Set{Int}() |
There was a problem hiding this comment.
This is a nit pick, but I don't like this variable name being a verb or action.
| # Generous bound (seconds) on how long a calibration will wait on an empty | ||
| # worker pool before erroring, so an asynchronous calibration cannot hang | ||
| # forever when no workers ever start. | ||
| const EMPTY_POOL_TIMEOUT = 7200 |
There was a problem hiding this comment.
Should this be configurable by the user?
| function default_worker_pool() | ||
| lock(POOL_LOCK) do | ||
| for id in workers() | ||
| id == 1 && continue |
There was a problem hiding this comment.
Can you add a comment about the id == 1 case? I think this is the ID of the main worker, but I am not confident.
| deregisters (e.g. walltime expiry or crash). Any stale entry left in the pool's | ||
| channel is filtered out at `take!` time. |
There was a problem hiding this comment.
Can you clarify the last sentence? I am not sure what context this is referring to.
| throw( | ||
| ErrorException( | ||
| "No workers available for $(round(Int, t_empty))s \ |
There was a problem hiding this comment.
Is this better than using error(msg)?
This PR makes workers individually submitted and self-registering. Each worker is its own scheduler allocation, adds itself to a process-wide pool once it has started and loaded model code, and the scheduler loop dispatches forward-model runs to workers as they become available. Closes #296.
Changes
add_workersto be asynchronous. This is the new intended way to use theWorkerBackend, thoughaddprocsstill works.GLOBAL_WORKER_POOL. This pool is guarded byPOOL_LOCKDistributed.launchnow submitsnseparate single-process allocations instead of one job array.poll_files_for_worker_startupnow polls all worker signal files, since they spin up separately now.@worker_setupto replace@everywhere, which required all workers to be already registered. The new macro records setup expressions on the main process and replays them on every worker as it joins.run_iterationnow starts with a possibly empty pool and waits for workers. AddedEMPTY_POOL_TIMEOUT = 7200so that jobs don't just hang indefinitely, maybe we can make this configurable.cancel_worker_jobsto cancel all workers. This now runs in anatexithook.