[jira] [Work logged] (ARTEMIS-4003) FlowControl on BridgeImpl is broken for Large Messages

["ASF GitHub Bot (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/ARTEMIS-4003?focusedWorklogId=810053&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-810053 ]

ASF GitHub Bot logged work on ARTEMIS-4003:
-------------------------------------------

                Author: ASF GitHub Bot
            Created on: 19/Sep/22 14:27
            Start Date: 19/Sep/22 14:27
    Worklog Time Spent: 10m 
      Work Description: clebertsuconic commented on code in PR #4224:
URL: https://github.com/apache/activemq-artemis/pull/4224#discussion_r974320869


##########
artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/impl/AbstractProducerCreditsImpl.java:
##########
@@ -144,24 +144,46 @@ public synchronized int decrementRefCount() {
 
    protected void checkCredits(final int credits) {
       int needed = Math.max(credits, windowSize);
+      if (logger.isTraceEnabled()) {
+         logger.tracef("CheckCredits %s on address %s, needed=%s, credits=%s, window=%s", credits, address, needed, credits, windowSize);
+      }
 
       int toRequest = -1;
 
       synchronized (this) {
          if (getBalance() + arriving < needed) {
             toRequest = needed - arriving;
 
-            pendingCredits += toRequest;
-            arriving += toRequest;
+            if (logger.isTraceEnabled()) {
+               logger.tracef("CheckCredits on Address %s, requesting=%s, arriving=%s, balance=%s", address, toRequest, arriving, getBalance());
+            }
+         } else {
+            if (logger.isTraceEnabled()) {
+               logger.tracef("CheckCredits did not need it, balance=%s, arriving=%s,  needed=%s, getbalance + arriving < needed=%s", getBalance(), arriving, needed, (boolean)(getBalance() + arriving < needed));
+            }
          }
       }
 
-      if (toRequest != -1) {
+      if (toRequest > 0) {
+         if (logger.isDebugEnabled()) {
+            logger.debugf("Requesting %s credits on address %s, needed = %s, arriving = %s", toRequest, address, needed, arriving);
+         }
          requestCredits(toRequest);
+      } else {
+         logger.debugf("not asking for %s credits on %s", toRequest, address);
       }
    }
 
-   private void requestCredits(final int credits) {
+   @Override
+   public String toString() {

Review Comment:
   hmmm... I did not mean to merge this line.. I will remove it.





Issue Time Tracking
-------------------

            Worklog Id:     (was: 810053)
    Remaining Estimate: 0h
            Time Spent: 10m

> FlowControl on BridgeImpl is broken for Large Messages
> ------------------------------------------------------
>
>                 Key: ARTEMIS-4003
>                 URL: https://issues.apache.org/jira/browse/ARTEMIS-4003
>             Project: ActiveMQ Artemis
>          Issue Type: Bug
>            Reporter: Clebert Suconic
>            Assignee: Clebert Suconic
>            Priority: Major
>          Time Spent: 10m
>  Remaining Estimate: 0h
>
> LargeMessages over BridgeImpl (including Clustering) will request more credits eventually than what they are entitled. This is because we don't flow control the large message producer on the Bridge (at least not yet), and the sender could get to a window where it requested more credits than it was entitled. The flow control on the Bridge is limited to handle returning busy or not (and that should be enough really).
> as a Result, the flow control will never refill and the bridge would freeze and get stuck.
> To fix this situation we should check for the starve at the end of the receive credits and do the proper refill.
> I am writing a test validating this issue, it will be under smokeTests and it will be named org.apache.activemq.artemis.tests.smoke.clusteredLargeMessage.ClusteredLargeMessageTest



--
This message was sent by Atlassian Jira
(v8.20.10#820010)