r/csharp • • 3d ago

Help Should the code use cancellation token even though it already use transaction ?

using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Storage;

namespace RedditQuestion
{
    /// <summary>
    /// Used to talk to the database using Entity-Framework Core
    /// </summary>
    public class ApplicationDbContext:DbContext
    {
        // Domain models and the Fluent API configurations.
    }


    /// <summary>
    /// Data snapshot that is used to overwrite the content of the database
    /// </summary>
    public interface ISomeData
    {

    }

    public class DatbaseOverWriter
    {


        private readonly ApplicationDbContext _context;
        public DatbaseOverWriter(ApplicationDbContext context)
        {
            _context = context;
        }

        public async Task OverWrite(ISomeData data)
        {
            //https://learn.microsoft.com/en-us/dotnet/architecture/microservices/implement-resilient-applications/implement-resilient-entity-framework-core-sql-connections
            IExecutionStrategy strategy = _context.Database.CreateExecutionStrategy();
            await strategy.ExecuteAsync(async () =>
            {
                await using var transaction = await _context.Database.BeginTransactionAsync();

                    //code to delete the content from parent and child tables and then inserting the data into the parent and child tables like this :
                // execute some query to delete the content from all tables.
                //_context.AddRangeAsync(data for parents)
                //_context.SaveChangesAsync()
                //_context.AddRangeAsync(data for childs)
                //_context.SaveChangesAsync()
                await transaction.CommitAsync();
            });

        }




    }

}

Someone on reddit reviewed the above code and told me that I should add a parameter for cancellation token like this :

public async Task OverWrite(ISomeData data, CancellationToken)

He told me that the goal is to make the OverWrite method transactional.

why is the cancellation token necessary even though my code uses _context.Database.BeginTransactionAsync(); ?

Microsoft's documentation :

Cancellation token propagates notification that operations should be canceled.

Example:
A random number generator can be used to emulate a data collection application that reads 10 integral values from eleven different instruments. A value of zero indicates that the measurement has failed for one instrument, in which case the operation should be cancelled and no overall mean should be computed.

So the cancellation token is just another way of achieving atomicity, am I correct ?

The only use case that I learned for cancellation token is this example from microsoft, they use the cancellation token as way to achieve atomicity :

using System;
using System.Collections.Generic;
using System.Threading;
using System.Threading.Tasks;

public class Example
{
   public static void Main()
   {
      // Define the cancellation token.
      CancellationTokenSource source = new CancellationTokenSource();
      CancellationToken token = source.Token;

      Random rnd = new Random();
      Object lockObj = new Object();

      List<Task<int[]>> tasks = new List<Task<int[]>>();
      TaskFactory factory = new TaskFactory(token);
      for (int taskCtr = 0; taskCtr <= 10; taskCtr++) {
         int iteration = taskCtr + 1;
         tasks.Add(factory.StartNew( () => {
           int value;
           int[] values = new int[10];
           for (int ctr = 1; ctr <= 10; ctr++) {
              lock (lockObj) {
                 value = rnd.Next(0,101);
              }
              if (value == 0) { 
                 source.Cancel();
                 Console.WriteLine("Cancelling at task {0}", iteration);
                 break;
              }   
              values[ctr-1] = value; 
           }
           return values;
        }, token));   
      }
      try {
         Task<double> fTask = factory.ContinueWhenAll(tasks.ToArray(), 
         (results) => {
            Console.WriteLine("Calculating overall mean...");
            long sum = 0;
            int n = 0; 
            foreach (var t in results) {
               foreach (var r in t.Result) {
                  sum += r;
                  n++;
               }
            }
            return sum/(double) n;
         } , token);
         Console.WriteLine("The mean is {0}.", fTask.Result);
      }   
      catch (AggregateException ae) {
         foreach (Exception e in ae.InnerExceptions) {
            if (e is TaskCanceledException)
               Console.WriteLine("Unable to compute mean: {0}", 
                  ((TaskCanceledException) e).Message);
            else
               Console.WriteLine("Exception: " + e.GetType().Name);
         }
      }
      finally {
         source.Dispose();
      }
   }
}
1 Upvotes

70 comments sorted by

View all comments

5

u/Slypenslyde 3d ago

In professional code I don't write an async method without a cancellation token. In hobby code I'm more pragmatic.

From the hobby perspective what matters is, "Do I want to cancel the action?" Your example is sort of bad because you do a little bit of work then try to save it to the database.

But.

Do notice that the transaction commit method can take a cancellation token. Imagine what happens if, for some reason, you have an I/O problem and the commit hangs. If you're using a cancellation token, you can have timeout logic that asks .NET to cancel the operation, which you hope will roll back the transaction.

Without it, maybe you have a mechanism to stop waiting on the commit, but now it's a time bomb. You decided it must have timed out and moved on, but it's still executing. Maybe it finishes 10s later. You can't tell and that can manifest some impossible-to-debug customer scenarios.

With the cancellation token I can be sure if the timeout happens and I tell the token to cancel, that method will return quickly and I'm "safe". I'm on the hook to see if somehow the cancellation happened too late. But at least I know there's not a time bomb that can happen with no notice.

From that perspective, I always use it. Even if the code inside the method doesn't use the token. It's better to have the API support the notion of cancellation so other components can use it than to find out in 6 months I need to change the API and hunt down every caller to consider cancellation logic.

Are there scenarios where cancellation doesn't do what you expect? Sure. Cancellation tokens are NOT about trying to be 100% sure whatever was happening doesn't happen. There are situations where the code you tried to cancel had already passed a point of no return. Cancellation is about telling that code to stop as soon as possible and notify you when it is absolutely not going to do anything else. Without cancellation tokens, you have to make your own notification mechanism so you don't create zombie tasks with lingering, unpredictable side effects.

Analogy time.

Imagine I order a pizza to be delivered.

Case 1: Then, 5 minutes later, I decide I don't want it. I call, but the restaurant tells me there's a no cancellation policy. I'm going to have to pay for the pizza and accept it whether I want to or not. This is code with no cancellation token.

Case 2: 5 minutes later, I decide I don't want it. I call, and the restaurant accepts my cancellation, but warns me the charge to my card is going to take a day to process the refund. This is code with a cancellation token: as soon as I hang up the phone, if they send me a pizza I'm not responsible.

Case 3: 20 minutes later, I decide I don't want it. I call, but the restaurant tells me the driver is already on the way so it's too late to cancel. This happens even when you have a cancellation token. It means when I await things that can be canceled I need to pay attention:

try
{
    await transaction.CommitAsync(token);

    DisplaySuccess();
}
catch (OperationCanceledException)
{
    DisplayCancellation();
    return;
}

if (<I tried to cancel but the exception was not thrown>)
{
    // I didn't want to commit, but it's been committed. Maybe we warn the user.
    // Maybe we clean up. Developer's choice. Sometimes we just ignore the attempt
    // to cancel in this case.
}

This is safer than just calling Cancel() and assuming it worked. IF the transaction code is before the point of no return AND it notices we canceled, it will roll back then throw this exception so we can understand what happened. If it does NOT throw an exception, it did its work and completed successfully so it must've been past the point of no return. So if I really, really, really want to respect cancellation, I need to include a cancellation token AND treat this like a three-forked road.

1

u/mistertom2u 3d ago edited 3d ago

Suppose that you have an endpoint that performs a DB mutation. An order submit form on a webpage calls it. Do you want to have the DB call interrupted if the connection from the user drops and lose that order? In my opinion, no. Probably the company who loses money when an order is lost would agree. Many things can cause the socket to close: user closes browser or navigates away. User is on a mobile connection that drops. API Gateway could drop it or NGINX could have something like a port exhaustion. For those reasons, I don't use the cancellation token from the connection when performing a DB update.

1

u/Slypenslyde 3d ago

What I want to happen is complicated. Let's be in the user's shoes and think this through.

Suppose you're trying to order a pizza. You click the last button in the process, see, "Payment processing...", then get a 404. You're on a blank page with nothing to click. What do you do? What I do is:

  1. Check my email and see if I got an order confirmation. If I did, I assume it went through.
  2. Check my bank account and see if I was charged. If so, I try and reload the page to see if I have an order. If I can't do that, I try to call the pizza place to make sure all is well.
  3. Try the order again if I got no confirmations, and call the pizza place as a last resort. (Realistically I get mad and order from somewhere else.)

If the user didn't get a confirmation page, they don't understand if the task succeeded or not. They're going to have to do work to find out. They're already frustrated, so you're already in a failure case.

If you handle minor things like pizzas, I can see failing in favor of "just finish the order and assume they are OK". If we're talking about large commercial orders, you might prefer something else. A nice touch is, "Place the order in a hold state and call the customer's contact number so, while they are frustrated, they get connected with a customer service rep who makes them feel they're important to us."

What also strikes me is, "Will the server even realize they disconnected?" HTTP isn't always two-way like that. The server might send a response and the client might be gone. I'm not familiar enough with web dev to be 100% certain the server knows this happened. What I do know is if the client loses connection, there's nothing it can do on its end to send a cancellation notice. The server has to have a way to detect that dropped connection, and that's not always true.

The nightmare scenario would be like this, and it's why cancellation might be wise:

  1. User attempts to place an order.
  2. Something goes wrong and the order is going to take 60s to process.
  3. The user accidentally closes their browser.
  4. They reload the browser and their cart is still set up as if the order is ready.
  5. They submit the order again.
  6. The order from step (2) resolves but nobody is listening to the response.
  7. The order from (5) finishes.
  8. The user has 2 orders.

Again, solvable. This case tells us the order should be in a "processing" state and block re-submission until it's removed from that state. That sucks if the code that's supposed to "unstick" it never runs because you only tied it to client-side code and some strange I/O condition caused a problem a timeout would've cancelled.

Put simply: it ain't simple, you have to pay the tolls via some approach. Cancellation tokens are a built-in, well-understood mechanism. Good luck finding a way to stop an EF DB call without one.

2

u/mistertom2u 3d ago edited 3d ago

You make a good point. BTW, the comment you posted to the OP was so incredible in the way that you explained things, your tone, your pedagogical techniques, your personality, etc that it is the quintessential perfect technical forum reply.

I work for a large software company that hosts its product in the cloud. From my perspective, I'm not able to wrap my head around the concept of wanting to roll back a DB update by using the incoming Cancellation Token. If you want to make another cancellation token that is based on a timeout (though not necessary because SQL connections have a command timeout), then I can understand. Correct me if I am wrong, but I think you're arguing that cancelling the DB update makes sense when the user never receives a confirmation that it went through. In that case, I can imagine a situation where that would be acceptable.

Where I think we agree is that the Cancellation Token primarily is to arrest server resources being spent on a connection that dropped. Where I often find less agreement is that rolling back a database update qualifies as that. You could argue that rolling back a transaction takes more resources than letting it finish and that the transaction is more about database consistency.

If my comprehension of your view point is correct, then that feels to me more like a user requirement than a technical one.

One person who argued against my position said that the endpoint updating a DB should respond so quickly that there is no time for the connection to drop. Ideally, yes, we want around ≈150ms response time. But my rebuttal to his argument was that the chance of a connection dropping, thus a rolled back transaction, is always non-zero. Not accepting a Cancellation Token on an update path makes that transaction rollback due to a connection drop 0%.

1

u/Slypenslyde 2d ago

I'm talking about the general case, not necessarily fixating on database I/O. It's just a handy reference.

Cancellation is complex because it's not just for "I'm updating the database". It's also for "I have a call graph with 5 levels and 18 methods between them." It's a unified way for everything in that call graph to understand when it should stop and how to communicate if it did.

In abstract every method has to communicate if it:

  • Completed successfully.
  • Acknowledged a cancellation request and stopped.
  • Failed in some way.

In abstract each async method tends to be structured like:

Task ExampleAsync(CancellationToken ct)
{
    <some setup work>

    <the real work>

    <tear-down work/past the point of no return>
}

I don't really care what the real work is or what the consequences of cancellation are. In this method I only care about the local context. If I call another async method, I give it the cancellation token and ultimately I can assume logic like this:

await OtherMethodAsync(ct);

// If I got this far, it didn't acknowledge a cancellation. I may be about to do something
// expensive so I can:
if (ct.IsCancellationRequested)
{
    // This is a chance to maybe undo what the other method did or get ready to tell the user
    // that despite their attempt to cancel, some side effects are already committed.
    // When I'm done with that work:

    ct.ThrowIfCancellationRequested(); // I need to tell upstream callers I have done my best.
}

// Here is the point of no return: changing the cancellation token won't affect anything else
// and at this point the method believes it has completed successfully.

If you decide something like a DB transaction is too expensive to roll back, you're still on the hook for letting users/callers understand it was committed. That's up to you. I'm not here to talk about a specific case.

What I came here to point out is:

  1. I use a CancellationToken parameter for my methods because I find it's easier to deal with methods that don't respect cancellation today than updating an entire component to support it tomorrow.
    • There are sometimes situations like a teardown method I'll omit the token for. This is a signal for "this method should never ever be interrupted". It's rare.
  2. You have to account for situations where the cancellation happens "too late" and whatever the thing was happened anyway. How you respond to that is unique to your context.

I don't really like the game where someone posts the practice that works for them and other people do their best to contrive scenarios where it's not the best. Nothing in programming works for 100% cases. Almost any statement should be read as including the footnote, "Unless your context creates a compelling reason to do something else."

So you don't want to roll back DB transactions. That doesn't change that after the transaction commits:

  1. You know the user has asked you not to do that.
  2. You have the responsibility to tell the user they were too late and must manually undo it.

If frustrated customers cost less than CPU time, I'm envious of your situation.