[#2198] Fix pending sends count on rolled back send#2199
Conversation
|
@mattrpav - I have asked this several times before, can you please start adding actual information to your PRs and issues? You added a bunch of info to the test itself which explains the problem, some of that needs to be in the description so people can figure out what the point of the PR is without having to look at the test. |
cshannon
left a comment
cshannon
left a comment
There was a problem hiding this comment.
LGTM, this is a good catch, just a couple minor tweaks that I noticed that could be done
mattrpav
force-pushed
the
amq-gh-2198-pending-sends-count
branch
from
049bb42 to
4081252
Compare
jbonofre
left a comment
jbonofre
left a comment
There was a problem hiding this comment.
I suspect that we could have double-decrement pendingSends in XA. The singlePendingSend() uses <= 1 so negative values can pass the test. Neither fireAfterCommit() and fireAfterRollback() has a guard preventing both from running on the same sync list.
That's OK, and we can always improve in a follow-up PR.
|
|
||
| private int getPendingSends(Queue queue) { | ||
| try { | ||
| var f = Queue.class.getDeclaredField("pendingSends"); |
There was a problem hiding this comment.
nit: instead of reflection, we could have added an accessor with @VisibleForTesting in Queue to avoid coupling the test.
3 participants
This was uncovered while looking into a different issue.
The problem here is that pending sends counter (internal, not exposed) does not decrement on a rolled back producer transaction.