Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Design a stop mechanism into async tasks #60

Open
mhofman opened this issue Feb 7, 2022 · 5 comments
Open

Design a stop mechanism into async tasks #60

mhofman opened this issue Feb 7, 2022 · 5 comments
Assignees

Comments

@mhofman
Copy link
Member

mhofman commented Feb 7, 2022

While #40 was mitigated by addressing known places of hang, and adding explicit timeouts at those locations, an overall timeout to prevent runaways would still be valuable. The correct way to do this is to modify the async task helpers to thread a stop mechanism (maybe in the form of a promise) to downstream tasks signaling that they should exit immediately, and add a top-level timeout to the stage to make sure it never goes longer than anticipated. Threading an abort mechanism in async tasks is fairly complex, especially when it should accommodate a somewhat clean shutdown (finalization steps for each task). @kriskowal has thoughts (e.g. a context object which allows to also probe the stopped state).

@kriskowal
Copy link
Member

At this point, I’m bullish on just using promises and promise combinators for this.

The design is that you thread a cancelled promise as an argument that will be rejected if and when the caller loses interest in the result.

To create a cancellation context:

const { reject: cancel, promise: cancelled } = makePromiseKit();

To check whether cancellation has already occurred before conducting an expensive operation:

await Promise.race([cancelled, undefined]);

To create a context bounded timeout:

const { reject: cancelChild, promise: childCancelled } = makePromiseKit();
cancelled.catch(cancelChild);
const handle = setTimeout(() => {
  cancelChild(new Error(`Timed out after ${timeoutMs}ms`));
}, timeoutMs);
cancelled.catch(() => clearTimeout(handle))

The alternative is to create something like an AbortController and AbortSignal pair that has a synchronously observable cancelled property. The advantage of that is that it wouldn’t require interleaving a microtask to check whether the context has been cancelled already. But, that hardly rationalizes building a new suite of combinators that suspiciously resembles promises.

If we continue with “cancellation promises” as a framework to solve this problem, I believe the solution will grow nicely into explicit context propagation when we have space to consider that problem holistically. Systems like Open Tracing will benefit from threading an “opaque token” around for adding context to logs and measurements. Cancellation context and trace spans tend to travel together, and where we’re threading one thing, I find it’s best to thread exactly one thing. That one thing could be both the opaque token and the cancellation promise together. The opaque token and a chain of parents (possibly the prototype chain) should be suitable for use as WeakMap keys when creating side tables. Using the prototype chain has the benefit that a child context could in some cases share the cancellation promise with a parent through prototype inheritance.

The context should also encapsulate the “deadline” for the context, and our Eventual Send layer would probably benefit from being privy to that, e.g., E(reference, {context}). This is important because deadlines should be absolute in memory and relative on the wire, since phase locked clocks are a pie in the sky. So, the idiomatic ceremony for establishing a deadline couples the ceremony I propose above to some ceremony on this context object.

I believe this is a compelling argument, but @dtribble has disagreed in the past.

@dtribble
Copy link
Member

dtribble commented Feb 8, 2022

For specific purposes here, this should be fine. For a general API, there are many systemic and architecture reasons to have a first class API for cancelation. I think that's outside the scope of this ticket though :)

@mhofman
Copy link
Member Author

mhofman commented Feb 8, 2022

For reference, the goal is very much to build something specific to this loadgen runner use case, at least at first. There are unique stacking (next task is abstract) and cleanup (perform actions once next task is done or aborted) requirements that I want to preserve. Threading a stop signal is pretty straightforward. Making it easy to use while preserving the requirements is more difficult.

@mhofman
Copy link
Member Author

mhofman commented Feb 8, 2022

Also, this is in the icebox for a reason ;)

@dtribble
Copy link
Member

dtribble commented Feb 8, 2022

Yep. A promise for this limited use case makes perfect sense to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants