[GitHub] [bookkeeper] mauricebarnum commented on pull request #2686: Rocksdb tombstones

2022-03-15 Thread GitBox
mauricebarnum commented on pull request #2686: URL: https://github.com/apache/bookkeeper/pull/2686#issuecomment-1068539002 The motivation to call `compactRange` was to quickly drop all of the rocksdb entries in a range when deleting a bunch of ledgers in GC so that seeking wouldn't run

[GitHub] [bookkeeper] dlg99 commented on pull request #3103: add async or sync for :entry location index's rocksdb write

2022-03-15 Thread GitBox
dlg99 commented on pull request #3103: URL: https://github.com/apache/bookkeeper/pull/3103#issuecomment-1068564975 @hangc0276 @merlimat please take a look -- 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

[GitHub] [bookkeeper] dlg99 commented on pull request #3097: bump lombok from 1.18.20 to 1.18.22 to support java17 compile

2022-03-15 Thread GitBox
dlg99 commented on pull request #3097: URL: https://github.com/apache/bookkeeper/pull/3097#issuecomment-1068563406 @Shoothzj merged -- 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

[GitHub] [bookkeeper] dlg99 merged pull request #3097: bump lombok from 1.18.20 to 1.18.22 to support java17 compile

2022-03-15 Thread GitBox
dlg99 merged pull request #3097: URL: https://github.com/apache/bookkeeper/pull/3097 -- 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:

[GitHub] [bookkeeper] dlg99 merged pull request #3101: Bump testcontainers version to 1.16.3

2022-03-15 Thread GitBox
dlg99 merged pull request #3101: URL: https://github.com/apache/bookkeeper/pull/3101 -- 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:

[GitHub] [bookkeeper] hangc0276 commented on pull request #2686: Rocksdb tombstones

2022-03-15 Thread GitBox
hangc0276 commented on pull request #2686: URL: https://github.com/apache/bookkeeper/pull/2686#issuecomment-1068631708 For this issue, IMO we'd better divide into two steps: 1. remove the compactRange logic in during checkpoint 2. Figure out smarter solution for deleted entry

[GitHub] [bookkeeper] hangc0276 commented on pull request #2932: direct-io: add support for bypassing operating system I/O cache when logging entries

2022-03-15 Thread GitBox
hangc0276 commented on pull request #2932: URL: https://github.com/apache/bookkeeper/pull/2932#issuecomment-1068634837 @mauricebarnum Would you please send a proposal discuss into d...@bookkeeper.apache.org mail list ? -- This is an automated message from the Apache Git Service. To

[GitHub] [bookkeeper] Nicklee007 opened a new pull request #3113: Fix the pid occupied check when use bookkeeper-daemon.sh start or stop

2022-03-15 Thread GitBox
Nicklee007 opened a new pull request #3113: URL: https://github.com/apache/bookkeeper/pull/3113 Master Issue: #3112 ### Motivation Fix the failed pid occupied check. we'll fail when use bookkeeper-daemon.sh start or stop bookie, after the last time we exit the bookie direct

[GitHub] [bookkeeper] Nicklee007 opened a new issue #3112: Can't start bookie by bookkeeper-daemon.sh, because the last pid occupied by other progress’s thread

2022-03-15 Thread GitBox
Nicklee007 opened a new issue #3112: URL: https://github.com/apache/bookkeeper/issues/3112 **BUG REPORT** ***Describe the bug*** After bookie progress is killed or occur some non-normal exit,can't start bookie by bin/bookkeeper-daemon.sh, because the last pid occupied by other

[GitHub] [bookkeeper] wuzhanpeng opened a new pull request #3111: BP-49: Support read ahead async

2022-03-15 Thread GitBox
wuzhanpeng opened a new pull request #3111: URL: https://github.com/apache/bookkeeper/pull/3111 Master issue: #3085 -- 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

[GitHub] [bookkeeper] hangc0276 commented on a change in pull request #3110: Fix ledgerEntryImpl reuse problem

2022-03-15 Thread GitBox
hangc0276 commented on a change in pull request #3110: URL: https://github.com/apache/bookkeeper/pull/3110#discussion_r826620612 ## File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/client/PendingReadOp.java ## @@ -563,6 +565,10 @@ public long

[GitHub] [bookkeeper] lhotari commented on pull request #3107: Run protobuf code generation automatically in IntelliJ and fix config

2022-03-15 Thread GitBox
lhotari commented on pull request #3107: URL: https://github.com/apache/bookkeeper/pull/3107#issuecomment-1067597235 rerun failure checks -- 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

[GitHub] [bookkeeper] congbobo184 commented on pull request #3106: Revert "Issue #1791: Read Submission should bypass OSE Threads"

2022-03-15 Thread GitBox
congbobo184 commented on pull request #3106: URL: https://github.com/apache/bookkeeper/pull/3106#issuecomment-1067609690 https://github.com/apache/bookkeeper/pull/3110 will fix it -- This is an automated message from the Apache Git Service. To respond to the message, please log on to

[GitHub] [bookkeeper] lhotari commented on pull request #3106: Revert "Issue #1791: Read Submission should bypass OSE Threads"

2022-03-15 Thread GitBox
lhotari commented on pull request #3106: URL: https://github.com/apache/bookkeeper/pull/3106#issuecomment-1067610773 > You could always have outstanding read responses after erroring out. But even if that happens, what exactly the error/corruption that is visible to the client?

[GitHub] [bookkeeper] codelipenghui commented on a change in pull request #3110: Fix ledgerEntryImpl reuse problem

2022-03-15 Thread GitBox
codelipenghui commented on a change in pull request #3110: URL: https://github.com/apache/bookkeeper/pull/3110#discussion_r826617880 ## File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/client/PendingReadOp.java ## @@ -563,6 +565,10 @@ public long

[GitHub] [bookkeeper] lhotari commented on pull request #3110: Fix ledgerEntryImpl reuse problem

2022-03-15 Thread GitBox
lhotari commented on pull request #3110: URL: https://github.com/apache/bookkeeper/pull/3110#issuecomment-1067608248 Thanks @congbobo184 for making this PR. I'm just wondering that `close` should get called once as a result of the guard by

[GitHub] [bookkeeper] eolivelli closed pull request #3106: Revert "Issue #1791: Read Submission should bypass OSE Threads"

2022-03-15 Thread GitBox
eolivelli closed pull request #3106: URL: https://github.com/apache/bookkeeper/pull/3106 -- 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:

[GitHub] [bookkeeper] eolivelli commented on pull request #3106: Revert "Issue #1791: Read Submission should bypass OSE Threads"

2022-03-15 Thread GitBox
eolivelli commented on pull request #3106: URL: https://github.com/apache/bookkeeper/pull/3106#issuecomment-1067627998 Closing for now -- 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

[GitHub] [bookkeeper] eolivelli merged pull request #3082: fix reduce unnecessary expansions

2022-03-15 Thread GitBox
eolivelli merged pull request #3082: URL: https://github.com/apache/bookkeeper/pull/3082 -- 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:

[GitHub] [bookkeeper] congbobo184 commented on pull request #3110: Fix ledgerEntryImpl reuse problem

2022-03-15 Thread GitBox
congbobo184 commented on pull request #3110: URL: https://github.com/apache/bookkeeper/pull/3110#issuecomment-1067719275 rerun failure checks -- 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

[GitHub] [bookkeeper] hangc0276 commented on a change in pull request #2932: direct-io: add support for bypassing operating system I/O cache when logging entries

2022-03-15 Thread GitBox
hangc0276 commented on a change in pull request #2932: URL: https://github.com/apache/bookkeeper/pull/2932#discussion_r826749675 ## File path: bookkeeper-slogger/slf4j/src/main/java/org/apache/bookkeeper/slogger/slf4j/Slf4jSlogger.java ## @@ -0,0 +1,108 @@ +/* + * Licensed to

[GitHub] [bookkeeper] hangc0276 commented on pull request #3110: Fix ledgerEntryImpl reuse problem

2022-03-15 Thread GitBox
hangc0276 commented on pull request #3110: URL: https://github.com/apache/bookkeeper/pull/3110#issuecomment-1067827283 > close @lhotari For example 1. there is a read request for ledger 1, entryId request range is [1, 100]. When entryId [1, 10] read requests has been sent

[GitHub] [bookkeeper] congbobo184 commented on pull request #3110: Fix ledgerEntryImpl reuse problem

2022-03-15 Thread GitBox
congbobo184 commented on pull request #3110: URL: https://github.com/apache/bookkeeper/pull/3110#issuecomment-1067780207 rerun failure checks -- 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

[GitHub] [bookkeeper] lhotari edited a comment on pull request #3110: Fix ledgerEntryImpl reuse problem

2022-03-15 Thread GitBox
lhotari edited a comment on pull request #3110: URL: https://github.com/apache/bookkeeper/pull/3110#issuecomment-1067626623 > @lhotari The `if (!complete.compareAndSet(false, true)) {` in PendingReadOp#submitCallback just change the complete flag in PendingReadOp instance. > > But

[GitHub] [bookkeeper] eolivelli commented on issue #3085: BP-49: Support reading ahead in async mode

2022-03-15 Thread GitBox
eolivelli commented on issue #3085: URL: https://github.com/apache/bookkeeper/issues/3085#issuecomment-1067692477 next step is to start the discussion on dev@bookkeeper it looks like a nice work ! well done (I have already taken a quick look to the PR) -- This is an automated

[GitHub] [bookkeeper] eolivelli merged pull request #3092: add unit tests for reduce unnecessary expansions

2022-03-15 Thread GitBox
eolivelli merged pull request #3092: URL: https://github.com/apache/bookkeeper/pull/3092 -- 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:

[GitHub] [bookkeeper] hangc0276 commented on pull request #3110: Fix ledgerEntryImpl reuse problem

2022-03-15 Thread GitBox
hangc0276 commented on pull request #3110: URL: https://github.com/apache/bookkeeper/pull/3110#issuecomment-1067634348 > Why isn't the `if (!complete.compareAndSet(false, true)) {` sufficient in PendingReadOp#submitCallback? @lhotari The `if (!complete.compareAndSet(false, true))

[GitHub] [bookkeeper] lhotari commented on pull request #3110: Fix ledgerEntryImpl reuse problem

2022-03-15 Thread GitBox
lhotari commented on pull request #3110: URL: https://github.com/apache/bookkeeper/pull/3110#issuecomment-1067642520 > Change the `complete` flag in LedgerEntryRequest. They are different instances. yes, I have noticed that there's 2 separate `complete` fields:

[GitHub] [bookkeeper] eolivelli commented on pull request #3110: Fix ledgerEntryImpl reuse problem

2022-03-15 Thread GitBox
eolivelli commented on pull request #3110: URL: https://github.com/apache/bookkeeper/pull/3110#issuecomment-1067842791 @hangc0276 in your last explanation your are not citing that there is a read error. My understanding is that this problem may happen only when there is a read

[GitHub] [bookkeeper] hangc0276 commented on pull request #3110: Fix ledgerEntryImpl reuse problem

2022-03-15 Thread GitBox
hangc0276 commented on pull request #3110: URL: https://github.com/apache/bookkeeper/pull/3110#issuecomment-1067618955 > Thanks @congbobo184 for making this PR. > > I'm just wondering that `close` should get called once as a result of the guard by

[GitHub] [bookkeeper] lhotari commented on pull request #3110: Fix ledgerEntryImpl reuse problem

2022-03-15 Thread GitBox
lhotari commented on pull request #3110: URL: https://github.com/apache/bookkeeper/pull/3110#issuecomment-1067626623 > > Thanks @congbobo184 for making this PR. > > I'm just wondering that `close` should get called once as a result of the guard by

[GitHub] [bookkeeper] hangc0276 commented on pull request #3110: Fix ledgerEntryImpl reuse problem

2022-03-15 Thread GitBox
hangc0276 commented on pull request #3110: URL: https://github.com/apache/bookkeeper/pull/3110#issuecomment-1067891759 > @hangc0276 in your last explanation your are not citing that there is a read error. My understanding is that this problem may happen only when there is a read error, in

[GitHub] [bookkeeper] hangc0276 edited a comment on pull request #3110: Fix ledgerEntryImpl reuse problem

2022-03-15 Thread GitBox
hangc0276 edited a comment on pull request #3110: URL: https://github.com/apache/bookkeeper/pull/3110#issuecomment-1067827283 > close @lhotari For example 1. there is a read request for ledger 1, entryId request range is [1, 100]. When entryId [1, 10] read requests has been

[GitHub] [bookkeeper] eolivelli commented on pull request #3110: Fix ledgerEntryImpl reuse problem

2022-03-15 Thread GitBox
eolivelli commented on pull request #3110: URL: https://github.com/apache/bookkeeper/pull/3110#issuecomment-1068005727 thank you @hangc0276 for your clarification. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and

[GitHub] [bookkeeper] lhotari commented on pull request #3107: Run protobuf code generation automatically in IntelliJ and fix config

2022-03-15 Thread GitBox
lhotari commented on pull request #3107: URL: https://github.com/apache/bookkeeper/pull/3107#issuecomment-1068055432 rerun failure checks -- 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

[GitHub] [bookkeeper] lhotari commented on a change in pull request #3110: PendingReadOp: Fix ledgerEntryImpl reuse problem

2022-03-15 Thread GitBox
lhotari commented on a change in pull request #3110: URL: https://github.com/apache/bookkeeper/pull/3110#discussion_r827065870 ## File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/client/PendingReadOp.java ## @@ -110,6 +110,7 @@ @Override

[GitHub] [bookkeeper] eolivelli merged pull request #3107: Run protobuf code generation automatically in IntelliJ and fix config

2022-03-15 Thread GitBox
eolivelli merged pull request #3107: URL: https://github.com/apache/bookkeeper/pull/3107 -- 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:

[GitHub] [bookkeeper] lhotari commented on pull request #3110: PendingReadOp: Fix ledgerEntryImpl reuse problem

2022-03-15 Thread GitBox
lhotari commented on pull request #3110: URL: https://github.com/apache/bookkeeper/pull/3110#issuecomment-1068099040 Thanks for the explanation @hangc0276 . I now understood how the race condition happens in `org.apache.bookkeeper.client.PendingReadOp#readEntryComplete` method and it

[GitHub] [bookkeeper] lhotari commented on issue #3104: Recycled LedgerEntryImpl instances are corrupted due to invalid state handling in BK client

2022-03-15 Thread GitBox
lhotari commented on issue #3104: URL: https://github.com/apache/bookkeeper/issues/3104#issuecomment-1068108570 This will be fixed by #3110 -- 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

[GitHub] [bookkeeper] lhotari edited a comment on pull request #3110: PendingReadOp: Fix ledgerEntryImpl reuse problem

2022-03-15 Thread GitBox
lhotari edited a comment on pull request #3110: URL: https://github.com/apache/bookkeeper/pull/3110#issuecomment-1068099040 Thanks for the explanation @hangc0276 . I now understood how the race condition happens in `org.apache.bookkeeper.client.PendingReadOp#readEntryComplete` method, and

[GitHub] [bookkeeper] lhotari commented on pull request #3107: Run protobuf code generation automatically in IntelliJ and fix config

2022-03-15 Thread GitBox
lhotari commented on pull request #3107: URL: https://github.com/apache/bookkeeper/pull/3107#issuecomment-1068164325 rerun failure checks -- 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

[GitHub] [bookkeeper] lhotari commented on pull request #1792: Issue #1791: Read Submission should bypass OSE Threads

2022-03-15 Thread GitBox
lhotari commented on pull request #1792: URL: https://github.com/apache/bookkeeper/pull/1792#issuecomment-1068111747 #3104 turned out to be a clear state handling issue in PendingReadOp class (and included embedded classes), fix is #3110 -- This is an automated message from the Apache

[GitHub] [bookkeeper] lhotari commented on pull request #3110: PendingReadOp: Fix ledgerEntryImpl reuse problem

2022-03-15 Thread GitBox
lhotari commented on pull request #3110: URL: https://github.com/apache/bookkeeper/pull/3110#issuecomment-1068136249 I now see the reason why Congbo chose to implement it using ```java public void close() { complete.set(true); entryImpl.close();

[GitHub] [bookkeeper] lhotari edited a comment on pull request #3110: PendingReadOp: Fix ledgerEntryImpl reuse problem

2022-03-15 Thread GitBox
lhotari edited a comment on pull request #3110: URL: https://github.com/apache/bookkeeper/pull/3110#issuecomment-1068136249 I now see the reason why Congbo chose to implement it using ```java public void close() { complete.set(true);

[GitHub] [bookkeeper] eolivelli commented on pull request #3110: PendingReadOp: Fix ledgerEntryImpl reuse problem

2022-03-15 Thread GitBox
eolivelli commented on pull request #3110: URL: https://github.com/apache/bookkeeper/pull/3110#issuecomment-1068364436 Just to clarify: I don't think that not recycling a object is a memory leak, as the jvm will discard it, but we are losing the chanceto save a allocation. BTW I