Skip to content

Conversation

mgravell
Copy link
Collaborator

@mgravell mgravell commented Jul 14, 2025

After consideration, I have decided not to proceed with this PR; there is significant complexity and confusion and potential performance impact, when a simpler version of things can already be achieved, at least on .NET 6+, with the inbuilt .WaitAsync(...) method family:

var timeout = TimeSpan.FromSeconds(5);
var cancellationToken = ...;
await db.StringSetAsync(key, value).WaitAsync(timeout);
var val = await db.StringGetAsync(key).WaitAsync(cancellationToken);

This approach also has additional flexibility advantages. The downside is that we don't withdraw the command from the unsent queue. I think, on reflection, that I can live with that for the simplicity.

A separate PR will be issued with guidance on using this approach.


(status: in progress; first draft is via generative codegen, to be reviewed, validated and expanded)

Implement async cancellation (also encompasses per-command cancellation).

Design approach here is to use ambient async state (i.e. AsyncLocal<T>) via wrapper layers:

  • exposing directly on individual methods is horrible due to API explosion / maintenance burden
  • decorator wrapper isn't really viable because there would be no proper way for the decorated API to pass the value down

Final API approach can be summarized as:

using (database.WithCancellation(someToken))
// alternative: using (database.WithTimeout(timeout))
{
    await database.DoTheThingAsync(...);
}

The return from WithCancellation here is an IDisposable that simply restores the previous state; it is not a decorated database. I view this as a positive, as it avoids the fat-fingered error (if it was that style of API) that is too common with the transaction API:

using var withTimeout = database.WithCancellation(someToken);
await database.DoTheThingAsync(...);
// ^^^ ERROR: should be await withTimeout.DoTheThingAsync(...);

This currently only applies to async operations, but it is an interesting question as to whether this also work for synchronous operations. It ... isn't impossible.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant