-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Akka.Delivery: fix ProducerControllerImpl<T>
state bug
#7034
Conversation
|
I think this is the key line:
In this state, I don't believe the Producer can do a redelivery of any un-ACKed messages, so the states are getting messed up somewhere between the producer and consumer contorllers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Detailed my changes
newRequestedSeqNr, newRequestedSeqNr2, stateAfterAck.CurrentSeqNr); | ||
|
||
if (newRequestedSeqNr > CurrentState.RequestedSeqNr) | ||
if (newRequestedSeqNr2 > CurrentState.RequestedSeqNr) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was the bug - we need to evaluate newRequestedSeqNr2
here for when the producer believes that the consumer has lost data (and re-delivery isn't enabled). This is what was causing this test to get stuck and probably some of the others in this suite.
ProducerControllerImpl<T>
state bug
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Changes
Fixes #7033
Working on troubleshooting the issues with ReliableDeliveryRandomSpecs
Checklist
For significant changes, please ensure that the following have been completed (delete if not relevant):