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

Learning transport throws exceptions when there are competing consumers #5309

Merged
merged 7 commits into from Dec 10, 2018

Conversation

Projects
None yet
6 participants
@bording
Copy link
Member

bording commented Dec 5, 2018

While working on Particular/MonitoringDemo#51 to let the monitoring demo run on the learning transport, several problems were encountered:

This PR fixes these problems by guarding against the exceptions that can be thrown.
It also prevents a critical error from being raised for a case that can actually be recovered from instead.

Who's affected

Users using the Learning Transport (which is not supported in production) with multiple consumers on the same queue.

Symptoms

The endpoint fails to start with the following exception:

Unhandled Exception: System.IO.IOException: The process cannot access the file '{RANDOM-GUID}.metadata.out' because it is being used by another process.

@bording bording requested a review from Particular/nservicebus-maintainers as a code owner Dec 5, 2018

{
if (log.IsDebugEnabled)
{
log.Debug($"Completing processing for {filePath}({transaction.FileToProcess}).");

This comment has been minimized.

@andreasohlund

andreasohlund Dec 6, 2018

Member

Should we log with a higher severity to make sure that users reports issues to us?

This comment has been minimized.

@bording

bording Dec 6, 2018

Member

@andreasohlund This was a pre-existing log message, and this isn't one that indicates a problem. Are you thinking about a different message?

Also, this PR introduces several other debug-level messages. Are you thinking we'd want to revise all of them?

This comment has been minimized.

@andreasohlund

andreasohlund Dec 7, 2018

Member

Sorry for the confusion, I meant in general. Will talk to @danielmarbach and PR against develop if needed so no need to hold up this PR

@timbussmann
Copy link
Member

timbussmann left a comment

Looks good to me. I'm trying to understand why you removed the critical error part and I assume it's also related to handling concurrent processing attempts on the same message, is that correct?

@bording

This comment has been minimized.

Copy link
Member

bording commented Dec 6, 2018

@timbussmann See #5299 (comment)

There were scenarios where raising a critical error was too aggressive when the failure was just something that could be resolved by trying again.

@timbussmann

This comment has been minimized.

Copy link
Member

timbussmann commented Dec 7, 2018

can you update the issue title and description before we go ahead and merge this?

@bording bording changed the title Hotfix 7.1.6 Learning transport throws exceptions when there are competing consumers Dec 7, 2018

@bording

This comment has been minimized.

Copy link
Member

bording commented Dec 7, 2018

@timbussmann I've updated both. Do you think that's good enough?

@timbussmann timbussmann merged commit 1ca43e2 into master Dec 10, 2018

5 checks passed

Compile Finished TeamCity Build NServiceBus :: Core :: 1. Compile : Running
Details
Inspections Finished TeamCity Build NServiceBus :: Core :: 2. Inspections : Running
Details
Test .NET Core on Linux Finished TeamCity Build NServiceBus :: Core :: 3.3 Test (.NET Core on Linux) : Tests passed: 1130, ignored: 43
Details
Test .NET Core on Windows Finished TeamCity Build NServiceBus :: Core :: 3.2 Test (.NET Core on Windows) : Tests passed: 1130, ignored: 43
Details
Test .NET Framework on Windows Finished TeamCity Build NServiceBus :: Core :: 3.1 Test (Framework on Windows) : Tests passed: 1363, ignored: 96
Details

@timbussmann timbussmann deleted the hotfix-7.1.6 branch Dec 10, 2018

@bording bording restored the hotfix-7.1.6 branch Dec 10, 2018

@DavidBoike DavidBoike added the Bug label Dec 10, 2018

@DavidBoike DavidBoike added this to the 7.1.6 milestone Dec 10, 2018

@andreasohlund andreasohlund deleted the hotfix-7.1.6 branch Dec 11, 2018

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