You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@bookkeeper.apache.org by GitBox <gi...@apache.org> on 2022/09/09 01:58:34 UTC
[GitHub] [bookkeeper] StevenLuMT commented on a diff in pull request #3214: Issue 3070: Fix bug where checkAllLedgers gets stuck when read throttling is enabled
StevenLuMT commented on code in PR #3214:
URL: https://github.com/apache/bookkeeper/pull/3214#discussion_r933708341
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerChecker.java:
##########
@@ -144,20 +147,30 @@ public LedgerChecker(BookKeeper bkc) {
}
public LedgerChecker(BookieClient client, BookieWatcher watcher) {
- this(client, watcher, -1);
+ this(client, watcher, null);
}
- public LedgerChecker(BookKeeper bkc, int inFlightReadEntryNum) {
- this(bkc.getBookieClient(), bkc.getBookieWatcher(), inFlightReadEntryNum);
+ public LedgerChecker(BookKeeper bkc, ServerConfiguration conf) {
+ this(bkc.getBookieClient(), bkc.getBookieWatcher(), conf);
}
- public LedgerChecker(BookieClient client, BookieWatcher watcher, int inFlightReadEntryNum) {
+ public LedgerChecker(BookieClient client, BookieWatcher watcher, ServerConfiguration conf) {
bookieClient = client;
bookieWatcher = watcher;
- if (inFlightReadEntryNum > 0) {
- semaphore = new Semaphore(inFlightReadEntryNum);
+ if (conf != null) {
+ if (conf.getReadEntryRateInLedgerChecker() > 0) {
+ semaphore = null;
+ rateLimiter = RateLimiter.create(conf.getReadEntryRateInLedgerChecker());
+ } else if (conf.getInFlightReadEntryNumInLedgerChecker() > 0) {
+ semaphore = new Semaphore(conf.getInFlightReadEntryNumInLedgerChecker());
Review Comment:
add
`this.inFlightReadEntryNumInLedgerCheckerEnable=true`
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerChecker.java:
##########
@@ -166,7 +179,9 @@ public LedgerChecker(BookieClient client, BookieWatcher watcher, int inFlightRea
* blocking until all are available.
*/
public void acquirePermit() throws InterruptedException {
- if (null != semaphore) {
+ if (rateLimiter != null) {
Review Comment:
add two switch,Would it be clearer? have a look @massakam @eolivelli
```
if (this.readEntryRateInLedgerCheckerEnable){
rateLimiter.acquire(1);
} else (this.inFlightReadEntryNumInLedgerCheckerEnable) {
semaphore.acquire(1);
}
```
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerChecker.java:
##########
@@ -144,20 +147,30 @@ public LedgerChecker(BookKeeper bkc) {
}
public LedgerChecker(BookieClient client, BookieWatcher watcher) {
- this(client, watcher, -1);
+ this(client, watcher, null);
}
- public LedgerChecker(BookKeeper bkc, int inFlightReadEntryNum) {
- this(bkc.getBookieClient(), bkc.getBookieWatcher(), inFlightReadEntryNum);
+ public LedgerChecker(BookKeeper bkc, ServerConfiguration conf) {
+ this(bkc.getBookieClient(), bkc.getBookieWatcher(), conf);
}
- public LedgerChecker(BookieClient client, BookieWatcher watcher, int inFlightReadEntryNum) {
+ public LedgerChecker(BookieClient client, BookieWatcher watcher, ServerConfiguration conf) {
bookieClient = client;
bookieWatcher = watcher;
- if (inFlightReadEntryNum > 0) {
- semaphore = new Semaphore(inFlightReadEntryNum);
+ if (conf != null) {
+ if (conf.getReadEntryRateInLedgerChecker() > 0) {
+ semaphore = null;
+ rateLimiter = RateLimiter.create(conf.getReadEntryRateInLedgerChecker());
Review Comment:
add
`this.readEntryRateInLedgerCheckerEnable=true`
--
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: commits-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org