lhotari commented on code in PR #25681:
URL: https://github.com/apache/pulsar/pull/25681#discussion_r3207261826
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryDelayedDeliveryTracker.java:
##########
@@ -122,21 +122,29 @@ private static long trimLowerBit(long timestamp, int
bits) {
}
@Override
- public boolean addMessage(long ledgerId, long entryId, long deliverAt) {
+ public synchronized boolean addMessage(long ledgerId, long entryId, long
deliverAt) {
Review Comment:
To keep the performance characteristics of the previous implementation, it
would be necessary to use
`synchronized (dispatcher) { ... }` blocks instead of making the methods
`synchronized`. Java runtime will make that a no-op when the thread already
holds the dispatcher's lock which is the current intended design.
A future improvement could be to make this pattern handled by a base class,
which would extend `AbstractDelayedDeliveryTracker`, so that the public methods
would marked final and the implementing class would implement a protected
method with a different name, for example `internalAddMessage`. This could be
handled in another PR so that the scope of this PR doesn't go beyond the bug
fix.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
