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

Outbox Sweeper effectively ignores the distributed lock due to the sweeping being done on a background thread #3321

Closed
preardon opened this issue Sep 24, 2024 · 3 comments
Assignees
Labels
2 - In Progress Bug .NET Pull requests that update .net code v10 Allocal to a v10 release

Comments

@preardon
Copy link
Member

Describe the bug

In the timed outbox sweeper we use a Locking provider

            var lockId = _distributedLock.ObtainLockAsync(LockingResourceName, CancellationToken.None).Result; 
            if (lockId != null)
            {
                s_logger.LogInformation("Outbox Sweeper looking for unsent messages");

                var scope = _serviceScopeFactory.CreateScope();
                try
                {
                    IAmACommandProcessor commandProcessor = scope.ServiceProvider.GetService<IAmACommandProcessor>();

                    var outBoxSweeper = new OutboxSweeper(
                        millisecondsSinceSent: _options.MinimumMessageAge,
                        commandProcessor: commandProcessor,
                        _options.BatchSize,
                        _options.UseBulk,
                        _options.Args);

                    if (_options.UseBulk)
                        outBoxSweeper.SweepAsyncOutbox();
                    else
                        outBoxSweeper.Sweep();
                }
                catch (Exception e)
                {
                    s_logger.LogError(e, "Error while sweeping the outbox.");
                    throw;
                }
                finally
                {
                    _distributedLock.ReleaseLockAsync(LockingResourceName, lockId, CancellationToken.None).Wait();
                    scope.Dispose();
                }
            }
...

however inside of the External bus Service we do the following

if (useAsync)
            {
                if (!HasAsyncOutbox())
                    throw new InvalidOperationException("No async outbox defined.");
                
                Task.Run(() => BackgroundDispatchUsingAsync(amountToClear, minimumAge, useBulk, args), CancellationToken.None);
            }

            else
            {
                if (!HasOutbox())
                    throw new InvalidOperationException("No outbox defined.");
                
                Task.Run(() => BackgroundDispatchUsingSync(amountToClear, minimumAge, args));
            }
...

meaning that we release the Lock inside of the Timed Sweeper service without any assurances that the Sweep is finished

@preardon
Copy link
Member Author

@iancooper I've got 2 PRs here that both solve this issue, do you have a preference on which one to take forward?

@iancooper iancooper added 2 - In Progress Bug v10 Allocal to a v10 release v9 Required for v9 release .NET Pull requests that update .net code labels Oct 2, 2024
@iancooper
Copy link
Member

@preardon Did we finish this one?

@preardon
Copy link
Member Author

I think I need to transpose this into v10

@iancooper iancooper removed the v9 Required for v9 release label Nov 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 - In Progress Bug .NET Pull requests that update .net code v10 Allocal to a v10 release
Projects
None yet
Development

No branches or pull requests

2 participants