You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by re...@apache.org on 2026/10/01 06:41:42 UTC
(tomcat) 19/25: Compare type and action as well as name when de-duplicating DeltaRequest actions so that distinct changes sharing a name, such as an attribute and a note, are all replicated, and recycle the evicted action back to the pool
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit dd3554c534b46201746a742145a8f3933a8a7bc8
Author: opencode <re...@apache.org>
AuthorDate: Wed Sep 30 23:07:29 2026 +0200
Compare type and action as well as name when de-duplicating DeltaRequest actions so that distinct changes sharing a name, such as an attribute and a note, are all replicated, and recycle the evicted action back to the pool
---
.../apache/catalina/ha/session/DeltaRequest.java | 25 ++++++++++++++++------
.../catalina/ha/session/TestDeltaRequest.java | 25 ++++++++++++++++++++++
2 files changed, 44 insertions(+), 6 deletions(-)
diff --git a/java/org/apache/catalina/ha/session/DeltaRequest.java b/java/org/apache/catalina/ha/session/DeltaRequest.java
index 5a04057888..80f3491339 100644
--- a/java/org/apache/catalina/ha/session/DeltaRequest.java
+++ b/java/org/apache/catalina/ha/session/DeltaRequest.java
@@ -23,6 +23,7 @@ import java.io.ObjectOutputStream;
import java.security.Principal;
import java.util.ArrayDeque;
import java.util.Deque;
+import java.util.Iterator;
import org.apache.catalina.SessionListener;
import org.apache.catalina.realm.GenericPrincipal;
@@ -279,10 +280,16 @@ public class DeltaRequest implements Externalizable {
// if we have already done something to this attribute, make sure
// we don't send multiple actions across the wire
if (!recordAllActions) {
- try {
- actions.remove(info);
- } catch (java.util.NoSuchElementException x) {
- // do nothing, we wanted to remove it anyway
+ Iterator<AttributeInfo> iterator = actions.iterator();
+ while (iterator.hasNext()) {
+ AttributeInfo existing = iterator.next();
+ if (existing.equals(info)) {
+ iterator.remove();
+ // Return the evicted action to the pool
+ existing.recycle();
+ actionPool.addLast(existing);
+ break;
+ }
}
}
// add the action
@@ -522,7 +529,9 @@ public class DeltaRequest implements Externalizable {
@Override
public int hashCode() {
- return name.hashCode();
+ // Must be consistent with equals() which compares name, type and
+ // action
+ return (name.hashCode() * 31 + type) * 31 + action;
}
public String getName() {
@@ -541,7 +550,11 @@ public class DeltaRequest implements Externalizable {
if (!(o instanceof AttributeInfo other)) {
return false;
}
- return other.getName().equals(this.getName());
+ // Attributes and notes are different stores and setting and
+ // removing are different changes, so a name match alone is not
+ // enough to consider two actions as the same change
+ return other.getType() == this.getType() && other.getAction() == this.getAction() &&
+ other.getName().equals(this.getName());
}
@Override
diff --git a/test/org/apache/catalina/ha/session/TestDeltaRequest.java b/test/org/apache/catalina/ha/session/TestDeltaRequest.java
index 17bf8672f4..b2e0db59ff 100644
--- a/test/org/apache/catalina/ha/session/TestDeltaRequest.java
+++ b/test/org/apache/catalina/ha/session/TestDeltaRequest.java
@@ -70,4 +70,29 @@ public class TestDeltaRequest {
Assert.assertEquals("One", copySession.getAttribute("A"));
Assert.assertEquals("Two", copySession.getAttribute("B"));
}
+
+ /*
+ * Attributes and notes are separate stores. Setting an attribute and setting a note with the same name are two
+ * distinct changes and both must be replicated.
+ */
+ @Test
+ public void testAttributeAndNoteWithSameNameBothRecorded() {
+ DeltaRequest request = new DeltaRequest();
+ request.setSessionId("1234");
+ request.setAttribute("name", "value");
+ request.setNote("name", Integer.valueOf(1));
+ Assert.assertEquals(2, request.getSize());
+ }
+
+ /*
+ * Setting the same attribute twice should be recorded as a single action.
+ */
+ @Test
+ public void testRepeatedSetAttributeRecordedOnce() {
+ DeltaRequest request = new DeltaRequest();
+ request.setSessionId("1234");
+ request.setAttribute("name", "value1");
+ request.setAttribute("name", "value2");
+ Assert.assertEquals(1, request.getSize());
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: (tomcat) 19/25: Compare type and action as well as name when de-duplicating DeltaRequest actions so that distinct changes sharing a name, such as an attribute and a note, are all replicated, and recycle the evicted action back to the pool
Posted by Mark Thomas <ma...@apache.org>.
On 01/10/2026 07:41, remm@apache.org wrote:
> This is an automated email from the ASF dual-hosted git repository.
>
> rmaucher pushed a commit to branch main
> in repository https://gitbox.apache.org/repos/asf/tomcat.git
>
> commit dd3554c534b46201746a742145a8f3933a8a7bc8
> Author: opencode <re...@apache.org>
> AuthorDate: Wed Sep 30 23:07:29 2026 +0200
>
> Compare type and action as well as name when de-duplicating DeltaRequest actions so that distinct changes sharing a name, such as an attribute and a note, are all replicated, and recycle the evicted action back to the pool
I don't think you want to include action when de-duplicating. Now a set
then remove sequence sends both actions rather than just the remove.
Mark
> ---
> .../apache/catalina/ha/session/DeltaRequest.java | 25 ++++++++++++++++------
> .../catalina/ha/session/TestDeltaRequest.java | 25 ++++++++++++++++++++++
> 2 files changed, 44 insertions(+), 6 deletions(-)
>
> diff --git a/java/org/apache/catalina/ha/session/DeltaRequest.java b/java/org/apache/catalina/ha/session/DeltaRequest.java
> index 5a04057888..80f3491339 100644
> --- a/java/org/apache/catalina/ha/session/DeltaRequest.java
> +++ b/java/org/apache/catalina/ha/session/DeltaRequest.java
> @@ -23,6 +23,7 @@ import java.io.ObjectOutputStream;
> import java.security.Principal;
> import java.util.ArrayDeque;
> import java.util.Deque;
> +import java.util.Iterator;
>
> import org.apache.catalina.SessionListener;
> import org.apache.catalina.realm.GenericPrincipal;
> @@ -279,10 +280,16 @@ public class DeltaRequest implements Externalizable {
> // if we have already done something to this attribute, make sure
> // we don't send multiple actions across the wire
> if (!recordAllActions) {
> - try {
> - actions.remove(info);
> - } catch (java.util.NoSuchElementException x) {
> - // do nothing, we wanted to remove it anyway
> + Iterator<AttributeInfo> iterator = actions.iterator();
> + while (iterator.hasNext()) {
> + AttributeInfo existing = iterator.next();
> + if (existing.equals(info)) {
> + iterator.remove();
> + // Return the evicted action to the pool
> + existing.recycle();
> + actionPool.addLast(existing);
> + break;
> + }
> }
> }
> // add the action
> @@ -522,7 +529,9 @@ public class DeltaRequest implements Externalizable {
>
> @Override
> public int hashCode() {
> - return name.hashCode();
> + // Must be consistent with equals() which compares name, type and
> + // action
> + return (name.hashCode() * 31 + type) * 31 + action;
> }
>
> public String getName() {
> @@ -541,7 +550,11 @@ public class DeltaRequest implements Externalizable {
> if (!(o instanceof AttributeInfo other)) {
> return false;
> }
> - return other.getName().equals(this.getName());
> + // Attributes and notes are different stores and setting and
> + // removing are different changes, so a name match alone is not
> + // enough to consider two actions as the same change
> + return other.getType() == this.getType() && other.getAction() == this.getAction() &&
> + other.getName().equals(this.getName());
> }
>
> @Override
> diff --git a/test/org/apache/catalina/ha/session/TestDeltaRequest.java b/test/org/apache/catalina/ha/session/TestDeltaRequest.java
> index 17bf8672f4..b2e0db59ff 100644
> --- a/test/org/apache/catalina/ha/session/TestDeltaRequest.java
> +++ b/test/org/apache/catalina/ha/session/TestDeltaRequest.java
> @@ -70,4 +70,29 @@ public class TestDeltaRequest {
> Assert.assertEquals("One", copySession.getAttribute("A"));
> Assert.assertEquals("Two", copySession.getAttribute("B"));
> }
> +
> + /*
> + * Attributes and notes are separate stores. Setting an attribute and setting a note with the same name are two
> + * distinct changes and both must be replicated.
> + */
> + @Test
> + public void testAttributeAndNoteWithSameNameBothRecorded() {
> + DeltaRequest request = new DeltaRequest();
> + request.setSessionId("1234");
> + request.setAttribute("name", "value");
> + request.setNote("name", Integer.valueOf(1));
> + Assert.assertEquals(2, request.getSize());
> + }
> +
> + /*
> + * Setting the same attribute twice should be recorded as a single action.
> + */
> + @Test
> + public void testRepeatedSetAttributeRecordedOnce() {
> + DeltaRequest request = new DeltaRequest();
> + request.setSessionId("1234");
> + request.setAttribute("name", "value1");
> + request.setAttribute("name", "value2");
> + Assert.assertEquals(1, request.getSize());
> + }
> }
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org