dlg99 commented on a change in pull request #2973:
URL: https://github.com/apache/bookkeeper/pull/2973#discussion_r779216068
##########
File path:
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerChecker.java
##########
@@ -51,6 +51,14 @@
public final BookieClient bookieClient;
public final BookieWatcher bookieWatcher;
+ private static int averageEntrySize;
+
+ private static final int INITIAL_AVERAGE_ENTRY_SIZE = 1024;
Review comment:
consider simplifying by using number of entries in flight (using
Semaphore, releasing when processed) instead of guessing avg sizes and rate
limiting. Or rate limit by number of entries.
This also removes need in synchronization.
One ledger may have entries of 512K, next one of the 1K, third one is mixed.
I don't see how tracking avg size significantly helps in this case,
especially if the backpressure is not enabled.
##########
File path:
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerChecker.java
##########
@@ -76,6 +84,11 @@
@Override
public void readEntryComplete(int rc, long ledgerId, long entryId,
ByteBuf buffer, Object ctx) {
+ if (readThrottle != null && buffer != null) {
+ int readSize = buffer.readableBytes();
+ averageEntrySize = (int) (averageEntrySize *
AVERAGE_ENTRY_SIZE_RATIO
Review comment:
async code, concurrent reads, plus updates of non-volatile field/later
reads from non-volatile field.
You'll need to add synchronization around use of this field.
--
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]
