Support for DO shutdown hooks - #7041
Conversation
| // preShutdown is reserved by the Durable Objects implementation, but only when the target | ||
| // worker has opted in via the compatibility flag, since existing classes may already export | ||
| // an RPC method with this name. | ||
| if (name == "preShutdown" && FeatureFlags::get(js).getDurableObjectPreShutdown()) { |
There was a problem hiding this comment.
Switch to [shutdown] and remove this reserved name logic/tests?
There was a problem hiding this comment.
And then the compatibility date can be ripped out too.
There was a problem hiding this comment.
Still being discussed internally
466e0b0 to
8219cf3
Compare
| // Unlike the alarm timeout, we do NOT abort the IoContext: teardown is already committed | ||
| // and proceeding, and the caller still drains storage writes the handler managed to issue. | ||
| // We just stop waiting for the handler. The abandoned handler continuation may keep | ||
| // running until that final storage drain completes, after which all of its pending |
There was a problem hiding this comment.
What if the abandoned continuation schedules more storage operations? Can that be prevented?
There was a problem hiding this comment.
Based on the usage of onNoPendingFlush in the relevant implementation, those later storage operations are not waited on.
Which is good in the sense that it prevents shutdown from being delayed, but does it cause problems for being able to immediately reuse the on-disk sqlite instance if some in-flight writes to it may get aborted? It's not clear how big of an issue this is in practice.
There was a problem hiding this comment.
Not fully preventable without aborting the IoContext at timeout, which we deliberately don't do so the drain can run.
Writes issued after onNoPendingFlush aren't waited on, but they were never confirmed to anyone (the output gate never released them), so no observer can have depended on them; teardown aborts them atomically at the storage layer. The SRS on-disk-reuse question is one to handle in edgeworker.
a-robinson
changed the title
Adds the API surface for the preShutdown() lifecycle hook on Durable Object classes, gated by the new experimental durable_object_pre_shutdown compatibility flag. The hook is invoked on a best-effort basis before planned, storage-healthy shutdowns. The reason list is extensible so further planned reasons (such as code-update resets or system-initiated shutdowns) can be delivered later without another flag. The actual implementation of the functionality is in the following commits.
Adds the machinery that actually invokes the preShutdown() Durable Object lifecycle hook, and wires it into workerd's graceful eviction paths for local-dev parity. This is modeled after runAlarm. Some notes on the decisions made: * If the wall-clock budget is exceeded, we don't abort the IoContext, so that writes issued by the handler can still succeed. * There is no per-request AsyncLocalStorage. * Exceptions don't propagate anywhere other than invocation logs. * This code path takes pains to avoid Worker::Actor::addRef() to avoid creating a RequestTracker and marking the DO as active. * After the handler completes, we wait on all storage writes to flush. * In the case where there were other callbacks active in the DO, they're allowed to run up until the point when the handler completes but not any more after that point. * This complicated the DO inactivty shutdown code in server.c++ a bit since now there are more possible race conditions there (sorry!).
Adds ActorObserver::preShutdownFinished(), called from Worker::Actor::runPreShutdownImpl() with the outcome of every hook run, including the early-exit case where the IoContext is already gone. Shutdowns that skip the hook synchronously (no applicable handler) are deliberately not reported, so observers only see actors that participate in the hook. The default implementation is a no-op; embedders override it to count outcomes.
01f27fa to
111d0d3
Compare
|
Squashed the commits down for easier review. This now LGTM and is ready for review, with the only real caveat being the naming question ( |
2 participants
No description provided.