The `ManagementService` is used by the broker to register and
unregister components for management as well as send notifications.
When the broker is busy dealing with new sessions and auto-creating
queues, addresses, etc. there is a lot of contention.
To reduce synchronization and improve the service overall this commit
does the following:
- Remove `synchronized` from most methods. In most cases it's
completely unnecessary because the methods are already using a
thread-safe data-structure (e.g. `ConcurrentHashMap`) or more
specific synchronization is already in place (e.g. on
`mbeanServer`).
- Adds new & clarifies existing logging.
- Synchronizes `start` & `stop` methods and adds gates via `started`.
- Simplifies the `sendNotification` method by synchronizing once rather
than twice and performing legitimacy checks sooner.
- Removing an unnecessary overload of the `registereQueue` method.
To be clear, there are no tests included with this commit as there
should be no semantic changes. Existing tests should be sufficient to
identify any regressions.
This commit fixes 4 distinct issues with divert configuration encoding:
- The encoding size was calculated incorrectly in three ways:
- Using 0 instead of `DataConstants.SIZE_NULL` when no transformer is
defined resulting in a calculated size discrepancy of -1.
- Using `DataConstants.INT` instead of `DataConstants.SIZE_INT` for
the number of transformer properties resulting in a calculated size
discrepancy of +2 when using a configuration with a transformer.
- Using `BufferHelper.sizeOfNullableBoolean` instead of
`DataConstants.SIZE_BOOLEAN` for the `exclusive` property resulting
in a calculated discrepancy of +1.
- Encoding was using `writeString` instead of `writeNullableString` for
the name of the transformer class resulting in an actual buffer size
discrepancy of -1.
Aside from these fixes this commit also:
- Updates the divert storage test to force a reload from disk which
will also trigger this problem.
- Adds new tests specifically for encoding & decoding include a test to
verify known bytes during encoding.
Lastly, it's worth noting that this *won't fix* bad data that was
already stored to disk by an older broker or bad data that comes over
the wire from an older broker (e.g. if a older primary broker paired
with a newer backup).
When a bridge is stopped it doesn't wait for pending send
acknowledgements to arrive. However, when a bridge is paused it does
wait. The behavior should be consistent and more importantly
configurable. This commit implements these improvements and generally
refactors BridgeImpl to clarify and simplify the code. In total, this
commit includes the follow changes:
- Removes the hard-coded 60-second timeout for pending acks when
pausing the bridge and adds a new config parameter (i.e.
"pending-ack-timeout").
- Applies the new pending-ack-timeout when the bridge is stopped.
- Updates existing and adds new logging messages for clarity.
- De-duplicates code for sending bridge-related notifications.
- Avoids converting bridge name to/from SimpleString.
- Removes unnecessary comments.
- Renames variables & functions for clarity.
- Replaces the `started`, `stopping`, & `active` booleans with a
single `state` variable which is an enum.
- Adds `final` to a few variables that were functionally final.
- Synchronizes `stop` & `pause` methods to add safety when invoked
concurrently with `handle` (since both deal with `state` and execute
runnables on the ordered executor).
- Reorganizes and removes a few methods for clarity.
- Relocates `connect` method directly into `ConnectRunnable` (mirroring
the structure of the `StopRunnable` and `PauseRunnable`).
- Eliminates unnecessary variables in `ConnectRunnable` and
`ScheduledConnectRunnable`.
- Adds test to verify pending ack timeout works as expected with both
`stop` & `pause` with both regular and large messages.
Updates artemis-log-annotation-processor to use artemis-project so that
artemis-pom can reference artemis-log-annotation-processor without cycle.
Split out its tests to their own module to faciltate, also exercising the
profile mechanism to enable the processor usage with trigger file.
Simplify disabling processing in the module using maven.compiler.proc prop
available since maven-compiler-plugin 3.13.0
Uses a dummy non-processor path at root to 'disable' processsing on JDK < 23,
accounting for Maven 3 not being able to unset maven.compiler.proc from a
parent, and JDKs < 21 requiring newest builds to support -proc:full value
needed otherwise to reenable processing once explicitly disabled.
This commit uses lambdas or method references wherever possible. There
are still a handful of places that appear like they could be changed but
couldn't mainly because they use "this" and the meaning of "this"
changes when using a lambda.
Currently, with 500K+ queues, the cleanup step of TempQueueCleanerUpper
requires invoking WildcardAddressManager#getDirectBindings, which is
O(k) in the number of queues.
From method profiling, this can consume up to 95% of our CPU time when
needing to clean up many of these.
Add a new map to keep track of the direct bindings, and add a test
assertion that fails if we don't properly remove it.
This commit does the following:
- deprecate all QueueConfiguration ctors
- add `of` static factory methods for all the deprecated ctors
- replace any uses of the normal ctors with the `of` counterparts
This makes the code more concise and readable.
This commit does the following:
- deprecate the verbosely named `toSimpleString` static factory
methods
- add `of` static factory methods for all the ctors
- replace any uses of the normal ctors with the `of` counterparts
This makes the code more concise and readable.
this is fixing PagingTest::testPagingStoreDestroyed(db=derby) (or any other DB).
This could eventually also fail on journal.
The cleanup could act on a destroyed folder, and issue an IOException stopping the server with this race.
You would need the server active cleaning messages while the queue is being removed.
I'm just adding some context to the code here. I have had to explain these variables a few times to different people,
I guess it's time to make that little explanation as part of the code now.
We observed this assert error in the netty collection used for acks:
java.lang.AssertionError: null is not a legitimate internal value. Concurrent Modification?
at io.netty.util.collection.IntObjectHashMap.toExternal(IntObjectHashMap.java:103) ~[netty-common-4.1.109.Final.jar:4.1.109.Final]
at io.netty.util.collection.IntObjectHashMap.access$900(IntObjectHashMap.java:37) ~[netty-common-4.1.109.Final.jar:4.1.109.Final]
at io.netty.util.collection.IntObjectHashMap$PrimitiveIterator.value(IntObjectHashMap.java:650) ~[netty-common-4.1.109.Final.jar:4.1.109.Final]
at io.netty.util.collection.IntObjectHashMap$2$1.next(IntObjectHashMap.java:234) ~[netty-common-4.1.109.Final.jar:4.1.109.Final]
this will avoid the cleanup and rely on GC for the cleanup.
PagingLeakTest is being added to make sure the cleanup is actually not needed.
This commit includes the following changes:
- Management operations to get sucess & failure counts for authn and
authz along with the corresponding audit logging.
- Export the aforementioned authn & authz metrics.
- Export metrics for the underlying authn & authz caches including the
ability to enable/disable them.
- Update metrics tests to validate tags in addition to keys and values.
- Update documentation to explain new functionality and clarify
existing metric tags.
The original commit (1ee3e884b7) for this
issue wasn't completely correct. This commit fixes those issues so that
both the messageCount and scheduledMessageCount are accurate now when
a scheduled message is removed by its ID.
Justin Bertram
Clebert Suconic
Robbie Gemmell
Gary Tully
Josh Byster
Domenico Francesco Bruscino
iliya