gemmellr commented on code in PR #5173:
URL: https://github.com/apache/activemq-artemis/pull/5173#discussion_r1733123545
##########
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/connect/mirror/AckManager.java:
##########
@@ -77,9 +75,15 @@ public AckManager(ActiveMQServer server) {
this.server = server;
this.configuration = server.getConfiguration();
this.ioCriticalErrorListener = server.getIoCriticalErrorListener();
- this.journal = server.getStorageManager().getMessageJournal();
this.sequenceGenerator = server.getStorageManager()::generateID;
- journalHashMapProvider = new JournalHashMapProvider<>(sequenceGenerator,
journal, AckRetry.getPersister(), JournalRecordIds.ACK_RETRY,
OperationContextImpl::getContext, server.getPostOffice()::findQueue,
server.getIoCriticalErrorListener());
+
+
//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
Review Comment:
Don't think we need 182 comment openings. Something the width of the comment
would be more than sufficient. TBH omitting it would seem fine too.
##########
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/connect/mirror/AckManager.java:
##########
@@ -77,9 +75,15 @@ public AckManager(ActiveMQServer server) {
this.server = server;
this.configuration = server.getConfiguration();
this.ioCriticalErrorListener = server.getIoCriticalErrorListener();
- this.journal = server.getStorageManager().getMessageJournal();
this.sequenceGenerator = server.getStorageManager()::generateID;
- journalHashMapProvider = new JournalHashMapProvider<>(sequenceGenerator,
journal, AckRetry.getPersister(), JournalRecordIds.ACK_RETRY,
OperationContextImpl::getContext, server.getPostOffice()::findQueue,
server.getIoCriticalErrorListener());
+
+
//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
+ // We have to use the StorageManager here
+ // if we used the journal directly, we would not capture a switch
between local journal and replicated journal that happens
+ // inside the storageManager
+ //
+ // also the switch has to be done while holding a read lock to avoid
missing the current record as well.
Review Comment:
These 4 lines would probably be a nicer as a single 2 or 3 line paragraph
--
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: gitbox-unsubscr...@activemq.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For additional commands, e-mail: gitbox-h...@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
