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

simplify CloseReason handling & improve AutorecoveringConnection eventing #1002

Merged
merged 2 commits into from
Jan 19, 2021

Conversation

bollhals
Copy link
Contributor

Proposed Changes

  • Simplifies the handling of the close reason (no locking required)
  • improves how the event recovering is done for AutoRecoveringConnection

Types of Changes

  • Bug fix (non-breaking change which fixes issue #NNNN)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause an observable behavior change in existing systems)
  • Documentation improvements (corrections, new content, etc)
  • Cosmetic change (whitespace, formatting, etc)

Checklist

  • I have read the CONTRIBUTING.md document
  • I have signed the CA (see https://cla.pivotal.io/sign/rabbitmq)
  • All tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)
  • Any dependent changes have been merged and published in related repositories

@michaelklishin
Copy link
Member

I get test failures in

RABBITMQ_RABBITMQCTL_PATH=rabbitmqctl dotnet test projects/Unit --filter "FullyQualifiedName~RabbitMQ.Client.Unit.TestConnectionRecovery"

on this branch but not master.

@michaelklishin
Copy link
Member

One example failure:

  X TestBasicAckEventHandlerRecovery [14s 428ms]
  Error Message:
     waiting on a latch timed out
  Expected: True
  But was:  False

  Stack Trace:
     at RabbitMQ.Client.Unit.IntegrationFixture.Wait(ManualResetEventSlim latch) in rabbitmq_dotnet_client.git/projects/Unit/Fixtures.cs:line 474
   at RabbitMQ.Client.Unit.TestConnectionRecovery.CloseAndWaitForRecovery(AutorecoveringConnection conn) in rabbitmq_dotnet_client.git/projects/Unit/TestConnectionRecovery.cs:line 953
   at RabbitMQ.Client.Unit.TestConnectionRecovery.CloseAndWaitForRecovery() in rabbitmq_dotnet_client.git/projects/Unit/TestConnectionRecovery.cs:line 944
   at RabbitMQ.Client.Unit.TestConnectionRecovery.TestBasicAckEventHandlerRecovery() in rabbitmq_dotnet_client.git/projects/Unit/TestConnectionRecovery.cs:line 114

@bollhals
Copy link
Contributor Author

I had the same locally, but I fixed it. The I was able to run these tests 30x without issues.
Not sure though wether the fix made it into the PR. 🤔 Let me have another look

align CloseReason implementation
- do not init with default value
@bollhals
Copy link
Contributor Author

I had the same locally, but I fixed it. The I was able to run these tests 30x without issues.
Not sure though wether the fix made it into the PR. 🤔 Let me have another look

yes, was still in the stash, sorry for that.

@michaelklishin michaelklishin merged commit ca65015 into rabbitmq:master Jan 19, 2021
@michaelklishin
Copy link
Member

Thank you!

@bollhals bollhals deleted the feature/eventing2 branch January 19, 2021 22:59
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.

2 participants