lhotari commented on code in PR #24596:
URL: https://github.com/apache/pulsar/pull/24596#discussion_r2251167322
##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerFactoryImpl.java:
##########
@@ -231,7 +231,7 @@ private ManagedLedgerFactoryImpl(MetadataStoreExtended
metadataStore,
this.entryCacheManager = new RangeEntryCacheManagerImpl(this,
scheduledExecutor, openTelemetry);
this.statsTask =
scheduledExecutor.scheduleWithFixedDelay(catchingAndLoggingThrowables(this::refreshStats),
0, config.getStatsPeriodSeconds(), TimeUnit.SECONDS);
- this.flushCursorsTask =
scheduledExecutor.scheduleAtFixedRate(catchingAndLoggingThrowables(this::flushCursors),
+ this.flushCursorsTask =
scheduledExecutor.scheduleWithFixedDelay(catchingAndLoggingThrowables(this::flushCursors),
config.getCursorPositionFlushSeconds(),
config.getCursorPositionFlushSeconds(), TimeUnit.SECONDS);
Review Comment:
Changing from `scheduleAtFixedRate` to `scheduleWithFixedDelay` could have a
significant impact to behavior. Let's say if cursorPositionFlushSeconds is set
to 1 and flushing takes 300ms. After the change, cursors would get flushed
every 1300 ms instead of every 1000 ms.
This is why I think that we should consider each case closely where we are
going to make a change from `scheduleAtFixedRate` to `scheduleWithFixedDelay`.
One possible alternative solution would be to keep on using
`scheduleAtFixedRate` and implement the wrapper in `Runnables` that I explained
in a separate comment.
--
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]
