gemmellr commented on code in PR #5498:
URL: https://github.com/apache/activemq-artemis/pull/5498#discussion_r1960259057
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/impl/PagingStoreImpl.java:
##########
@@ -205,17 +206,23 @@ public PagingStoreImpl(final SimpleString address,
this.syncNonTransactional = syncNonTransactional;
- if (scheduledExecutor != null && syncTimeout > 0) {
- this.syncTimer = new PageSyncTimer(this, scheduledExecutor,
ioExecutor, syncTimeout);
- } else {
- this.syncTimer = null;
- }
+ this.timedWriter = createPageTimedWriter(scheduledExecutor, syncTimeout);
this.cursorProvider = storeFactory.newCursorProvider(this,
this.storageManager, addressSettings, executor);
this.usingGlobalMaxSize = pagingManager.isUsingGlobalSize();
}
+ protected PageTimedWriter createPageTimedWriter(ScheduledExecutorService
scheduledExecutor, long syncTimeout) {
+ if (scheduledExecutor != null && syncTimeout > 0) {
+ PageTimedWriter localWriter = new PageTimedWriter(pageSize,
storageManager, this, scheduledExecutor, executor, syncNonTransactional,
syncTimeout);
Review Comment:
_scheduledExecutor_ and _syncTimeout_ are passed in from the constructor,
but others like _executor_ are not and instead accessed from the fields, would
be nice to make it consistent if possible.
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/impl/PagingStoreImpl.java:
##########
@@ -85,7 +86,8 @@ public class PagingStoreImpl implements PagingStore {
private final PageCache usedPages = new PageCache(this);
- //it's being guarded by lock.writeLock().lock() and never read concurrently
+ // This is updated and read by the PageTimedWriter's executor thread
+ // or by a writeLock if the timedWriter is null
Review Comment:
It looks like 3 methods can update the value. 2 of them always hold the
write lock, but 1 of them only does if timedWriter is null. If timedWriter is
not null, and it doesnt hold the write lock as it seems, then wouldn't it be
possible this could be updated concurrently?
--
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
