Skip to content

ARTEMIS-6031 Handle credit starvation affecting Core bridge#6387

Open
AntonRoskvist wants to merge 1 commit intoapache:mainfrom
AntonRoskvist:ARTEMIS-6031
Open

ARTEMIS-6031 Handle credit starvation affecting Core bridge#6387
AntonRoskvist wants to merge 1 commit intoapache:mainfrom
AntonRoskvist:ARTEMIS-6031

Conversation

@AntonRoskvist
Copy link
Copy Markdown
Contributor

This relates to #6020

The fix in that case is still valid but turns out there was a similar thing that could happen in AbstractProducerCreditsImpl as well.

Big thanks to @mbengtsson for co-authoring this PR!

return null;
}).when(mockClientSession).sendProducerCreditsMessage(Mockito.anyInt(), Mockito.any());

AsynchronousProducerCreditsImpl producerCredits = new AsynchronousProducerCreditsImpl(mockClientSession, null, producerWindowSize, bridgeFlowCallback(blocked));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Instead of having the private static method I think this is a bit more clear:

      AsynchronousProducerCreditsImpl producerCredits = new AsynchronousProducerCreditsImpl(mockClientSession, null, producerWindowSize, new ClientProducerFlowCallback() {
         @Override
         public void onCreditsFlow(boolean isBlocked, ClientProducerCredits credits) {
            blocked.set(isBlocked);
         }

         @Override
         public void onCreditsFail(ClientProducerCredits credits) {
         }
      });

}
} else {
if (logger.isTraceEnabled()) {
logger.trace("CheckCredits did not need it, balance={}, arriving={}, needed={}, getbalance + arriving < needed={}", getBalance(), arriving, needed, (boolean)(getBalance() + arriving < needed));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This log message should be updated to reflect the new comparison logic.

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