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.

Sign up for GitHub

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

Jump to bottom

Better error handling for streams #3176

Merged
merged 7 commits into from
Jul 27, 2021
Merged

Better error handling for streams #3176

merged 7 commits into from
Jul 27, 2021

Conversation

dcorbacho
Copy link
Contributor

Proposed Changes

  • Capture delete_stream errors and return them as protocol errors: clients get the reason of the failure, instead of a generic internal error
  • Avoid double spawn on coordinator and link spawned processes: If phases are not linked, they can continue running after the coordinator has been stopped. This could (or not) lead to unexpected behaviours, but for sure it makes debugging harder

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

@dcorbacho dcorbacho requested a review from kjnilsson July 6, 2021 10:50
dcorbacho added 5 commits July 14, 2021 17:17
Capture delete_stream errors and return them as protocol errors
0103ab9 
Clients get the reason of the failure, instead of a generic internal error
Avoid double spawn on coordinator and link spawned processes
e58ee79 
If phases are not linked, they can continue running *after* the coordinator has
been stopped. This could (or not) lead to unexpected behaviours,
but for sure it makes debugging harder.
Include queue name on log message
6610c34
Split cluster_size_3_parallel in two groups
6052ecd 
Faster to test locally the flaky tests and isolate them
Fix delete_replica bug
e65ba83 
It caused a lot of flakiness on the rabbit_stream_queue_SUITE, both on `delete_replica`
and `delete_last_replica` test cases.
pjk25
pjk25 approved these changes Jul 26, 2021
View reviewed changes
Copy link
Contributor

@pjk25 pjk25 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

@pjk25 pjk25 added this to the 3.9.1 milestone Jul 26, 2021
@michaelklishin michaelklishin merged commit 2efc3d2 into master Jul 27, 2021
@michaelklishin michaelklishin deleted the stream-error-handling branch July 27, 2021 19:25
michaelklishin added a commit that referenced this pull request Jul 27, 2021
@michaelklishin
Merge pull request #3176 from rabbitmq/stream-error-handling
23ea29a 
Better error handling for streams

(cherry picked from commit 2efc3d2)
@michaelklishin
Copy link
Member

Backported to v3.9.x.

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

@pjk25 pjk25 pjk25 approved these changes

@kjnilsson kjnilsson Awaiting requested review from kjnilsson

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
3.9.1
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@dcorbacho @michaelklishin @pjk25