You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by "void-ptr974 (via GitHub)" <gi...@apache.org> on 2026/05/01 10:08:50 UTC
[PR] [fix][broker] ConcurrentLongHashMap throw ArrayIndexOutOfBoundsException [pulsar]
void-ptr974 opened a new pull request, #25644:
URL: https://github.com/apache/pulsar/pull/25644
Fixes #25643
### Motivation
### Modifications
Bundle keys / values / capacity into one immutable inner class and publish via a single volatile reference
### Verifying this change
This change added tests and can be verified as follows:
### Does this pull request potentially affect one of the following parts:
<!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
*If the box was checked, please highlight the changes*
- [ ] Dependencies (add or upgrade a dependency)
- [ ] The public API
- [ ] The schema
- [ ] The default values of configurations
- [ ] The threading model
- [ ] The binary protocol
- [ ] The REST endpoints
- [ ] The admin CLI options
- [ ] The metrics
- [ ] Anything that affects deployment
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [fix][broker] ConcurrentLongHashMap throw ArrayIndexOutOfBoundsException [pulsar]
Posted by "void-ptr974 (via GitHub)" <gi...@apache.org>.
void-ptr974 commented on code in PR #25644:
URL: https://github.com/apache/pulsar/pull/25644#discussion_r3176276591
##########
pulsar-common/src/main/java/org/apache/pulsar/common/util/collections/ConcurrentLongHashMap.java:
##########
@@ -288,11 +289,27 @@ public interface EntryProcessor<V> {
void accept(long key, V value);
}
- // A section is a portion of the hash map that is covered by a single
+ // A section is a portion of the hash map that is covered by a single lock. The keys, values
+ // and capacity arrays are bundled into an immutable Table snapshot so that readers always see
+ // a consistent (key, value, length) triple, eliminating the partial-publish race that the
+ // previous design had to paper over with Math.min(keys.length, values.length).
@SuppressWarnings("serial")
private static final class Section<V> extends StampedLock {
- private volatile long[] keys;
- private volatile V[] values;
+ private static final class Table<V> {
Review Comment:
Done — converted to a record. All field accesses now use the generated accessors.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [fix][broker] ConcurrentLongHashMap throw ArrayIndexOutOfBoundsException [pulsar]
Posted by "void-ptr974 (via GitHub)" <gi...@apache.org>.
void-ptr974 commented on PR #25644:
URL: https://github.com/apache/pulsar/pull/25644#issuecomment-4363218898
Thanks @merlimat — both addressed in the latest commit. PTAL.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [fix][broker] ConcurrentLongHashMap throw ArrayIndexOutOfBoundsException [pulsar]
Posted by "void-ptr974 (via GitHub)" <gi...@apache.org>.
void-ptr974 commented on code in PR #25644:
URL: https://github.com/apache/pulsar/pull/25644#discussion_r3176276820
##########
pulsar-common/src/main/java/org/apache/pulsar/common/util/collections/ConcurrentLongHashMap.java:
##########
@@ -288,11 +289,27 @@ public interface EntryProcessor<V> {
void accept(long key, V value);
}
- // A section is a portion of the hash map that is covered by a single
+ // A section is a portion of the hash map that is covered by a single lock. The keys, values
+ // and capacity arrays are bundled into an immutable Table snapshot so that readers always see
+ // a consistent (key, value, length) triple, eliminating the partial-publish race that the
+ // previous design had to paper over with Math.min(keys.length, values.length).
@SuppressWarnings("serial")
private static final class Section<V> extends StampedLock {
- private volatile long[] keys;
- private volatile V[] values;
+ private static final class Table<V> {
+ final long[] keys;
+ final V[] values;
+ final int capacity;
+
+ Table(long[] keys, V[] values, int capacity) {
+ this.keys = keys;
+ this.values = values;
+ this.capacity = capacity;
+ }
+ }
+
+ // Section is Serializable only by inheritance from StampedLock; never actually serialized.
+ @SuppressFBWarnings("SE_BAD_FIELD")
+ private volatile Table<V> table;
private volatile int capacity;
Review Comment:
Done — removed. Outer `capacity()` now reads `s.table.capacity()`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [fix][broker] ConcurrentLongHashMap throw ArrayIndexOutOfBoundsException [pulsar]
Posted by "merlimat (via GitHub)" <gi...@apache.org>.
merlimat commented on code in PR #25644:
URL: https://github.com/apache/pulsar/pull/25644#discussion_r3174050744
##########
pulsar-common/src/main/java/org/apache/pulsar/common/util/collections/ConcurrentLongHashMap.java:
##########
@@ -288,11 +289,27 @@ public interface EntryProcessor<V> {
void accept(long key, V value);
}
- // A section is a portion of the hash map that is covered by a single
+ // A section is a portion of the hash map that is covered by a single lock. The keys, values
+ // and capacity arrays are bundled into an immutable Table snapshot so that readers always see
+ // a consistent (key, value, length) triple, eliminating the partial-publish race that the
+ // previous design had to paper over with Math.min(keys.length, values.length).
@SuppressWarnings("serial")
private static final class Section<V> extends StampedLock {
- private volatile long[] keys;
- private volatile V[] values;
+ private static final class Table<V> {
Review Comment:
This could be converted to a Java record
##########
pulsar-common/src/main/java/org/apache/pulsar/common/util/collections/ConcurrentLongHashMap.java:
##########
@@ -288,11 +289,27 @@ public interface EntryProcessor<V> {
void accept(long key, V value);
}
- // A section is a portion of the hash map that is covered by a single
+ // A section is a portion of the hash map that is covered by a single lock. The keys, values
+ // and capacity arrays are bundled into an immutable Table snapshot so that readers always see
+ // a consistent (key, value, length) triple, eliminating the partial-publish race that the
+ // previous design had to paper over with Math.min(keys.length, values.length).
@SuppressWarnings("serial")
private static final class Section<V> extends StampedLock {
- private volatile long[] keys;
- private volatile V[] values;
+ private static final class Table<V> {
+ final long[] keys;
+ final V[] values;
+ final int capacity;
+
+ Table(long[] keys, V[] values, int capacity) {
+ this.keys = keys;
+ this.values = values;
+ this.capacity = capacity;
+ }
+ }
+
+ // Section is Serializable only by inheritance from StampedLock; never actually serialized.
+ @SuppressFBWarnings("SE_BAD_FIELD")
+ private volatile Table<V> table;
private volatile int capacity;
Review Comment:
shouldn't this get removed now?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [fix][broker] ConcurrentLongHashMap throw ArrayIndexOutOfBoundsException [pulsar]
Posted by "merlimat (via GitHub)" <gi...@apache.org>.
merlimat merged PR #25644:
URL: https://github.com/apache/pulsar/pull/25644
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org