fix: make provider transfer termination async #4766
Conversation
| .processor(processConsumerTransfersInState(RESUMING, this::processConsumerResuming)) | ||
| .processor(processTransfersInState(COMPLETING, this::processCompleting)) | ||
| .processor(processTransfersInState(TERMINATING, this::processTerminating)) | ||
| .processor(processTransfersInState(TERMINATING_REQUESTED, this::processTerminating)) |
There was a problem hiding this comment.
If the state TERMINATING_REQUESTED is handled by processTerminating, wouldn't that mean that a TransferTerminationMessage is sent again after one has already been sent by the counter-party? Afaik, only one party should send this message, right?
There was a problem hiding this comment.
yes, that's why there's a condition into the sendTransferTerminationMessage.
I thought to bring the condition to the upper lever, but it would had complicated the code much more, because of the nested RetryProcess used. I can take a look if it could refactored into a better shape.
EDIT: it's also described in this class tests :)
There was a problem hiding this comment.
You're right, I completely missed that. While it reads a bit intransparent at first, if the alternative is refactoring it all in a more complex way, this would be fine with me.
ronjaquensel
dismissed
their stale review
misclicked approve instead of comment
jimmarino
left a comment
jimmarino
left a comment
There was a problem hiding this comment.
Let's see if the condition mentioned in the comments can be cleaned up in a subsequent PR
3 participants
What this PR changes/adds
Apply transfer termination in asynchronous manner.
Why it does that
as stated in the related DR
Further notes
TERMINATING_REQUESTEDhas been added (best name that came to mind 😅 ). For this status there will be no message dispatch, because the message was already sent from the counter party. If this proposal goes through I will update the documentation accordingly.Who will sponsor this feature?
Please @-mention the committer that will sponsor your feature.
Linked Issue(s)
Part of #4592
Please be sure to take a look at the contributing guidelines and our etiquette for pull requests.