[
https://issues.apache.org/jira/browse/ARTEMIS-5305?focusedWorklogId=957611&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-957611
]
ASF GitHub Bot logged work on ARTEMIS-5305:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 18/Feb/25 18:06
Start Date: 18/Feb/25 18:06
Worklog Time Spent: 10m
Work Description: 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?
Issue Time Tracking
-------------------
Worklog Id: (was: 957611)
Time Spent: 2h 20m (was: 2h 10m)
> Improve performance on paging for multiple producers and optimize locking
> -------------------------------------------------------------------------
>
> Key: ARTEMIS-5305
> URL: https://issues.apache.org/jira/browse/ARTEMIS-5305
> Project: ActiveMQ Artemis
> Issue Type: Bug
> Affects Versions: 2.39.0
> Reporter: Clebert Suconic
> Assignee: Clebert Suconic
> Priority: Major
> Labels: pull-request-available
> Fix For: 2.40.0
>
> Time Spent: 2h 20m
> Remaining Estimate: 0h
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information, visit: https://activemq.apache.org/contact
