This is an automated email from the ASF dual-hosted git repository.

He-Pin pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/pekko.git


The following commit(s) were added to refs/heads/main by this push:
     new 5b2f4b468b Ignore stale classic remoting ACKs #3126 (#3158)
5b2f4b468b is described below

commit 5b2f4b468b4ad3289e5427804e188a96555fcf12
Author: He-Pin(kerr) <[email protected]>
AuthorDate: Wed Jun 24 13:01:28 2026 +0800

    Ignore stale classic remoting ACKs #3126 (#3158)
    
    Stale system-message acknowledgements can arrive after classic remoting 
reconnects and the sender has reset its resend buffer for the current UID. 
Treating a cumulative ACK newer than the sender buffer as a protocol violation 
gates/quarantines an otherwise valid association.
    
    Modification:
    
    Make AckedSendBuffer ignore cumulative ACKs beyond the highest sequence 
number it has seen, log and skip those ACKs in ReliableDeliverySupervisor, and 
add regression coverage for empty/non-empty buffers, stale ACKs with NACKs, and 
valid ACK processing after a stale ACK.
    
    Result:
    
    Late ACKs from an old receive buffer no longer trigger 
HopelessAssociation/quarantine, while real unfulfillable NACKs still fail 
through the existing ResendUnfulfillableException path.
    
    Tests:
    
    - sbt "remote / Test / testOnly org.apache.pekko.remote.AckedDeliverySpec" 
/ passed: 13 tests
    
    - sbt checkCodeStyle / passed
    
    - scalafmt --list --mode diff-ref=origin/main / passed: no files listed
    
    - git diff --check / passed: no output
    
    - subAgent review / FINAL PASS
    
    - qodercli review (non-interactive, log: /tmp/tmo-qoder-review.log) / PASS
    
    References:
    
    Fixes #3126
---
 .../org/apache/pekko/remote/AckedDelivery.scala    | 26 +++++++++--------
 .../scala/org/apache/pekko/remote/Endpoint.scala   | 34 +++++++++++++---------
 .../apache/pekko/remote/AckedDeliverySpec.scala    | 11 +++++++
 3 files changed, 46 insertions(+), 25 deletions(-)

diff --git a/remote/src/main/scala/org/apache/pekko/remote/AckedDelivery.scala 
b/remote/src/main/scala/org/apache/pekko/remote/AckedDelivery.scala
index f84c2c7916..16fc083272 100644
--- a/remote/src/main/scala/org/apache/pekko/remote/AckedDelivery.scala
+++ b/remote/src/main/scala/org/apache/pekko/remote/AckedDelivery.scala
@@ -116,23 +116,25 @@ final case class AckedSendBuffer[T <: HasSequenceNumber](
 
   /**
    * Processes an incoming acknowledgement and returns a new buffer with only 
unacknowledged elements remaining.
+   * Acknowledgements for sequence numbers newer than this buffer has seen are 
stale and return this buffer unchanged.
    * @param ack The received acknowledgement
    * @return An updated buffer containing the remaining unacknowledged messages
    */
   def acknowledge(ack: Ack): AckedSendBuffer[T] = {
-    if (ack.cumulativeAck > maxSeq)
-      throw new IllegalArgumentException(s"Highest SEQ so far was $maxSeq but 
cumulative ACK is ${ack.cumulativeAck}")
-    val newNacked =
-      if (ack.nacks.isEmpty) Vector.empty
+    if (ack.cumulativeAck > maxSeq) this
+    else {
+      val newNacked =
+        if (ack.nacks.isEmpty) Vector.empty
+        else
+          (nacked ++ nonAcked).filter { m =>
+            ack.nacks(m.seq)
+          }
+      if (newNacked.size < ack.nacks.size) throw new 
ResendUnfulfillableException
       else
-        (nacked ++ nonAcked).filter { m =>
-          ack.nacks(m.seq)
-        }
-    if (newNacked.size < ack.nacks.size) throw new ResendUnfulfillableException
-    else
-      this.copy(nonAcked = nonAcked.filter { m =>
-          m.seq > ack.cumulativeAck
-        }, nacked = newNacked)
+        this.copy(nonAcked = nonAcked.filter { m =>
+            m.seq > ack.cumulativeAck
+          }, nacked = newNacked)
+    }
   }
 
   /**
diff --git a/remote/src/main/scala/org/apache/pekko/remote/Endpoint.scala 
b/remote/src/main/scala/org/apache/pekko/remote/Endpoint.scala
index 88e68e5270..65eae47858 100644
--- a/remote/src/main/scala/org/apache/pekko/remote/Endpoint.scala
+++ b/remote/src/main/scala/org/apache/pekko/remote/Endpoint.scala
@@ -366,20 +366,28 @@ private[remote] class ReliableDeliverySupervisor(
     case ack: Ack =>
       // If we are not sure about the UID just ignore the ack. Ignoring is 
fine.
       if (uidConfirmed) {
-        try resendBuffer = resendBuffer.acknowledge(ack)
-        catch {
-          case NonFatal(e) =>
-            throw new HopelessAssociation(
-              localAddress,
-              remoteAddress,
-              uid,
-              new IllegalStateException(
-                s"Error encountered while processing system message " +
-                s"acknowledgement buffer: $resendBuffer ack: $ack",
-                e))
-        }
+        if (ack.cumulativeAck > resendBuffer.maxSeq)
+          log.warning(
+            "Ignoring stale system message acknowledgement from remote system 
[{}]. Highest SEQ so far was [{}] but cumulative ACK is [{}].",
+            remoteAddress,
+            resendBuffer.maxSeq,
+            ack.cumulativeAck)
+        else {
+          try resendBuffer = resendBuffer.acknowledge(ack)
+          catch {
+            case NonFatal(e) =>
+              throw new HopelessAssociation(
+                localAddress,
+                remoteAddress,
+                uid,
+                new IllegalStateException(
+                  s"Error encountered while processing system message " +
+                  s"acknowledgement buffer: $resendBuffer ack: $ack",
+                  e))
+          }
 
-        resendNacked()
+          resendNacked()
+        }
       }
     case AttemptSysMsgRedelivery =>
       if (uidConfirmed) resendAll()
diff --git 
a/remote/src/test/scala/org/apache/pekko/remote/AckedDeliverySpec.scala 
b/remote/src/test/scala/org/apache/pekko/remote/AckedDeliverySpec.scala
index ecc09f2e06..dbe63282b4 100644
--- a/remote/src/test/scala/org/apache/pekko/remote/AckedDeliverySpec.scala
+++ b/remote/src/test/scala/org/apache/pekko/remote/AckedDeliverySpec.scala
@@ -150,6 +150,17 @@ class AckedDeliverySpec extends PekkoSpec {
 
     }
 
+    "ignore stale cumulative ack newer than the send buffer" in {
+      val emptyBuffer = new AckedSendBuffer[Sequenced](10)
+      emptyBuffer.acknowledge(Ack(SeqNo(2))) should ===(emptyBuffer)
+      emptyBuffer.acknowledge(Ack(SeqNo(2), nacks = Set(SeqNo(0)))) should 
===(emptyBuffer)
+
+      val buffer = emptyBuffer.buffer(msg(0)).buffer(msg(1))
+      buffer.acknowledge(Ack(SeqNo(2))) should ===(buffer)
+      buffer.acknowledge(Ack(SeqNo(2), nacks = Set(SeqNo(0)))) should 
===(buffer)
+      buffer.acknowledge(Ack(SeqNo(2))).acknowledge(Ack(SeqNo(0))).nonAcked 
should ===(Vector(msg(1)))
+    }
+
     "keep NACKed messages in buffer if selective nacks are received" in {
       val b0 = new AckedSendBuffer[Sequenced](10)
       val msg0 = msg(0)


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to