[GitHub] [bookkeeper] lhotari opened a new issue #3105: Improve performance of OrderedExecutor by switching to a more performant BlockingQueue implementation

2022-03-14 Thread GitBox
lhotari opened a new issue #3105: URL: https://github.com/apache/bookkeeper/issues/3105 **FEATURE REQUEST** [OrderedExecutor uses

[GitHub] [bookkeeper] lhotari opened a new issue #3104: Recycled LedgerEntryImpl instances are corrupted due to a thread safety issue in BK client

2022-03-14 Thread GitBox
lhotari opened a new issue #3104: URL: https://github.com/apache/bookkeeper/issues/3104 **BUG REPORT** ***Describe the bug*** See https://github.com/apache/pulsar/issues/14436#issuecomment-1064758710 for a detailed analysis by @congbobo184 about the symptoms of the thread

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

2022-03-14 Thread GitBox
lhotari commented on pull request #1792: URL: https://github.com/apache/bookkeeper/pull/1792#issuecomment-1066419068 I created #3104 about the thread safety issue. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the

[GitHub] [bookkeeper] eolivelli edited a comment on issue #3104: Recycled LedgerEntryImpl instances are corrupted due to a thread safety issue in BK client

2022-03-14 Thread GitBox
eolivelli edited a comment on issue #3104: URL: https://github.com/apache/bookkeeper/issues/3104#issuecomment-1066472346 Makes sense to me. I am fine with reverting #1792 data corruption is a bad problem, "being faster" is only an enhancement. we should revert that change

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

2022-03-14 Thread GitBox
eolivelli commented on pull request #3106: URL: https://github.com/apache/bookkeeper/pull/3106#issuecomment-1066476085 @diegosalvi this change is also important for HerdDB and DistributedLog -- This is an automated message from the Apache Git Service. To respond to the message, please

[GitHub] [bookkeeper] leizhiyuan commented on pull request #3099: feat: use Runtime.getRuntime().availableProcessors() as thread pool size

2022-03-14 Thread GitBox
leizhiyuan commented on pull request #3099: URL: https://github.com/apache/bookkeeper/pull/3099#issuecomment-1066476507 > Thanks for your contribution > > Can you explain more about the motivation for this change? Thanks for your reply , now, we have different bk clusters, if

[GitHub] [bookkeeper] eolivelli commented on issue #3104: Recycled LedgerEntryImpl instances are corrupted due to a thread safety issue in BK client

2022-03-14 Thread GitBox
eolivelli commented on issue #3104: URL: https://github.com/apache/bookkeeper/issues/3104#issuecomment-1066474503 PR for the revert: https://github.com/apache/bookkeeper/pull/3106 great work @lhotari @congbo1984 -- This is an automated message from the Apache Git Service. To

[GitHub] [bookkeeper] eolivelli edited a comment on issue #3104: Recycled LedgerEntryImpl instances are corrupted due to a thread safety issue in BK client

2022-03-14 Thread GitBox
eolivelli edited a comment on issue #3104: URL: https://github.com/apache/bookkeeper/issues/3104#issuecomment-1066474503 PR for the revert: https://github.com/apache/bookkeeper/pull/3106 great work @lhotari @congbobo1984 -- This is an automated message from the Apache Git

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

2022-03-14 Thread GitBox
eolivelli opened a new pull request #3106: URL: https://github.com/apache/bookkeeper/pull/3106 This reverts commit 6b99ff73278d4e655509f61ea44284e54716f5a8. Descriptions of the changes in this PR: Revert #1792 "Issue #1791: Read Submission should bypass OSE Threads" ###

[GitHub] [bookkeeper] eolivelli commented on issue #3104: Recycled LedgerEntryImpl instances are corrupted due to a thread safety issue in BK client

2022-03-14 Thread GitBox
eolivelli commented on issue #3104: URL: https://github.com/apache/bookkeeper/issues/3104#issuecomment-1066472346 Makes sense to me. I am fine with reverting #3105 data corruption is a bad problem, "being faster" is only an enhancement. we should revert that change on

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

2022-03-14 Thread GitBox
lhotari commented on pull request #3106: URL: https://github.com/apache/bookkeeper/pull/3106#issuecomment-1066476568 There's #3105 as a possible solution for mitigating the performance issue which was the original motivation for the PR which is reverted by this PR. -- This is an

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

2022-03-14 Thread GitBox
lhotari opened a new pull request #3107: URL: https://github.com/apache/bookkeeper/pull/3107 Descriptions of the changes in this PR: ### Motivation IntelliJ shows compilation errors for BookKeeper code that references Protobuf generated classes after importing the BookKeeper

[GitHub] [bookkeeper] lordcheng10 commented on pull request #3092: add unit tests for reduce unnecessary expansions

2022-03-14 Thread GitBox
lordcheng10 commented on pull request #3092: URL: https://github.com/apache/bookkeeper/pull/3092#issuecomment-1066592756 @eolivelli PTAL,thanks! -- 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

[GitHub] [bookkeeper] lordcheng10 commented on pull request #3082: fix reduce unnecessary expansions

2022-03-14 Thread GitBox
lordcheng10 commented on pull request #3082: URL: https://github.com/apache/bookkeeper/pull/3082#issuecomment-1066593244 @eolivelli If there is no problem, can you help me merge? Thank you! -- This is an automated message from the Apache Git Service. To respond to the message, please

[GitHub] [bookkeeper] eolivelli merged pull request #3090: [website] update website every time there is a change

2022-03-14 Thread GitBox
eolivelli merged pull request #3090: URL: https://github.com/apache/bookkeeper/pull/3090 -- 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] gaozhangmin opened a new pull request #3109: Fix getMetadataServiceUri

2022-03-14 Thread GitBox
gaozhangmin opened a new pull request #3109: URL: https://github.com/apache/bookkeeper/pull/3109 ### Motivation when config metadataStoreUri like `zk1:2181,zk2:2181/test`, `getMetadataServiceUri ` will only return `zk1:2181`. it's incorrect. ### Changes Use `getList

[GitHub] [bookkeeper] gaozhangmin commented on a change in pull request #3109: Fix getMetadataServiceUri

2022-03-14 Thread GitBox
gaozhangmin commented on a change in pull request #3109: URL: https://github.com/apache/bookkeeper/pull/3109#discussion_r825909610 ## File path: bookkeeper-server/src/test/java/org/apache/bookkeeper/conf/AbstractConfigurationTest.java ## @@ -56,6 +56,12 @@ public void setup()

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

2022-03-14 Thread GitBox
lhotari commented on pull request #3107: URL: https://github.com/apache/bookkeeper/pull/3107#issuecomment-1066824623 > LGTM > > I am still seeing other warnings, not a blocker for this patch > > ``` > Execution optimizations have been disabled for task

[GitHub] [bookkeeper] eolivelli commented on pull request #3109: Fix getMetadataServiceUri

2022-03-14 Thread GitBox
eolivelli commented on pull request #3109: URL: https://github.com/apache/bookkeeper/pull/3109#issuecomment-1066725156 I believe you should use the semi colon to separate zk servers: `zk1:2181;zk2:2181/test` -- This is an automated message from the Apache Git Service. To respond to

[GitHub] [bookkeeper] gaozhangmin commented on pull request #3109: Fix getMetadataServiceUri

2022-03-14 Thread GitBox
gaozhangmin commented on pull request #3109: URL: https://github.com/apache/bookkeeper/pull/3109#issuecomment-1066731314 > I believe you should use the semi colon to separate zk servers: `zk1:2181;zk2:2181/test` `zk1:2181;zk2:2181/test` is a incorrect connection string, I tried.

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

2022-03-14 Thread GitBox
lhotari edited a comment on pull request #3107: URL: https://github.com/apache/bookkeeper/pull/3107#issuecomment-1066824623 > I am still seeing other warnings, not a blocker for this patch > > ``` > Execution optimizations have been disabled for task

[GitHub] [bookkeeper] eolivelli commented on a change in pull request #3109: Fix getMetadataServiceUri

2022-03-14 Thread GitBox
eolivelli commented on a change in pull request #3109: URL: https://github.com/apache/bookkeeper/pull/3109#discussion_r825907041 ## File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/AbstractConfiguration.java ## @@ -268,7 +268,11 @@ public String

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

2022-03-14 Thread GitBox
eolivelli commented on pull request #3107: URL: https://github.com/apache/bookkeeper/pull/3107#issuecomment-1066746716 I am adding "-x signDistTar" btw with the latest commit I don't see the warning anymore, thanks -- This is an automated message from the Apache Git Service. To

[GitHub] [bookkeeper] gaozhangmin commented on a change in pull request #3109: Fix getMetadataServiceUri

2022-03-14 Thread GitBox
gaozhangmin commented on a change in pull request #3109: URL: https://github.com/apache/bookkeeper/pull/3109#discussion_r825911160 ## File path: bookkeeper-server/src/test/java/org/apache/bookkeeper/conf/AbstractConfigurationTest.java ## @@ -56,6 +56,12 @@ public void setup()

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

2022-03-14 Thread GitBox
diegosalvi commented on pull request #3106: URL: https://github.com/apache/bookkeeper/pull/3106#issuecomment-1066524689 @eolivelli interesting, thank you. We'll test disabling netty recycler as stated in https://github.com/apache/pulsar/issues/14436 for now -- This is an automated

[GitHub] [bookkeeper] lhotari removed a comment on issue #3105: Improve performance of OrderedExecutor by switching to a more performant BlockingQueue implementation

2022-03-14 Thread GitBox
lhotari removed a comment on issue #3105: URL: https://github.com/apache/bookkeeper/issues/3105#issuecomment-1066544555 Pulsar contains GrowableArrayBlockingQueue too,

[GitHub] [bookkeeper] lhotari commented on a change in pull request #3108: Issue #3105: Optimize OrderedExecutor performance by using GrowableArrayBlockingQueue

2022-03-14 Thread GitBox
lhotari commented on a change in pull request #3108: URL: https://github.com/apache/bookkeeper/pull/3108#discussion_r825729833 ## File path: bookkeeper-common/src/main/java/org/apache/bookkeeper/common/collections/GrowableArrayBlockingQueue.java ## @@ -83,6 +82,7 @@ public T

[GitHub] [bookkeeper] lhotari commented on a change in pull request #3108: Issue #3105: Optimize OrderedExecutor performance by using GrowableArrayBlockingQueue

2022-03-14 Thread GitBox
lhotari commented on a change in pull request #3108: URL: https://github.com/apache/bookkeeper/pull/3108#discussion_r825730096 ## File path: bookkeeper-common/src/main/java/org/apache/bookkeeper/common/collections/GrowableArrayBlockingQueue.java ## @@ -83,6 +82,7 @@ public T

[GitHub] [bookkeeper] lhotari commented on issue #3105: Improve performance of OrderedExecutor by switching to a more performant BlockingQueue implementation

2022-03-14 Thread GitBox
lhotari commented on issue #3105: URL: https://github.com/apache/bookkeeper/issues/3105#issuecomment-1066544555 Pulsar contains GrowableArrayBlockingQueue too,

[GitHub] [bookkeeper] lhotari opened a new pull request #3108: Issue #3105: Optimize OrderedExecutor performance by using GrowableArrayBlockingQueue

2022-03-14 Thread GitBox
lhotari opened a new pull request #3108: URL: https://github.com/apache/bookkeeper/pull/3108 Fixes #3105 ### Motivation - See #3105 for details. OrderedExecutor uses LinkedBlockingQueue which isn't the most performant BlockingQueue implementation. For example, it causes

[GitHub] [bookkeeper] eolivelli commented on a change in pull request #3108: Issue #3105: Optimize OrderedExecutor performance by using GrowableArrayBlockingQueue

2022-03-14 Thread GitBox
eolivelli commented on a change in pull request #3108: URL: https://github.com/apache/bookkeeper/pull/3108#discussion_r825723429 ## File path: bookkeeper-common/src/main/java/org/apache/bookkeeper/common/collections/GrowableArrayBlockingQueue.java ## @@ -83,6 +82,7 @@ public

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

2022-03-14 Thread GitBox
lhotari commented on pull request #3107: URL: https://github.com/apache/bookkeeper/pull/3107#issuecomment-1066571457 @eolivelli I pushed 222e6bb as an attempt to fix the warning. Can you check if it goes away? (unfortunately I have the signing error message so I couldn't test) -- This

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

2022-03-14 Thread GitBox
eolivelli commented on pull request #3107: URL: https://github.com/apache/bookkeeper/pull/3107#issuecomment-1066537617 If I run this command I get the warning below: `./gradlew build -x test` ``` > Task :bookkeeper-dist:distZip Execution optimizations have been disabled for

[GitHub] [bookkeeper] lhotari commented on a change in pull request #3108: Issue #3105: Optimize OrderedExecutor performance by using GrowableArrayBlockingQueue

2022-03-14 Thread GitBox
lhotari commented on a change in pull request #3108: URL: https://github.com/apache/bookkeeper/pull/3108#discussion_r825732865 ## File path: bookkeeper-common/src/main/java/org/apache/bookkeeper/common/collections/GrowableArrayBlockingQueue.java ## @@ -83,6 +82,7 @@ public T

[GitHub] [bookkeeper] lhotari commented on pull request #3108: Issue #3105: Optimize OrderedExecutor performance by using GrowableArrayBlockingQueue

2022-03-14 Thread GitBox
lhotari commented on pull request #3108: URL: https://github.com/apache/bookkeeper/pull/3108#issuecomment-1066553933 > it looks like you are also fixing a bug in GrowableArrayBlockingQueue > > is it possible to cover the fix with a test case please ? Pulsar contains

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

2022-03-14 Thread GitBox
lhotari commented on pull request #3107: URL: https://github.com/apache/bookkeeper/pull/3107#issuecomment-1066565457 > If I run this command I get the warning below: `./gradlew build -x test` @eolivelli Does this warning also appear without the changes in this PR? (I tried checking,

[GitHub] [bookkeeper] lhotari edited a comment on pull request #3108: Issue #3105: Optimize OrderedExecutor performance by using GrowableArrayBlockingQueue

2022-03-14 Thread GitBox
lhotari edited a comment on pull request #3108: URL: https://github.com/apache/bookkeeper/pull/3108#issuecomment-1066553933 > it looks like you are also fixing a bug in GrowableArrayBlockingQueue > > is it possible to cover the fix with a test case please ? Pulsar contains

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

2022-03-14 Thread GitBox
hangc0276 commented on pull request #2932: URL: https://github.com/apache/bookkeeper/pull/2932#issuecomment-1066927092 Would you please address the the CI? ``` + dev/check-binary-license dev/../bookkeeper-dist/server/build/distributions/bookkeeper-server-4.15.0-SNAPSHOT-bin.tar.gz

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

2022-03-14 Thread GitBox
jvrao commented on pull request #3106: URL: https://github.com/apache/bookkeeper/pull/3106#issuecomment-1067316430 > +1 on @dlg99 's comment. I could not understand where exactly the problem and how it could get addressed by making create and recycle run in the same thread. You

[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-14 Thread GitBox
hangc0276 commented on a change in pull request #2932: URL: https://github.com/apache/bookkeeper/pull/2932#discussion_r826557707 ## File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/directentrylogger/DirectWriter.java ## @@ -0,0 +1,318 @@ +/** +

[GitHub] [bookkeeper] gaozhangmin closed pull request #3109: Fix getMetadataServiceUri

2022-03-14 Thread GitBox
gaozhangmin closed pull request #3109: URL: https://github.com/apache/bookkeeper/pull/3109 -- 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-14 Thread GitBox
congbobo184 commented on pull request #3110: URL: https://github.com/apache/bookkeeper/pull/3110#issuecomment-1067572688 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 opened a new pull request #3110: Fix ledgerEntryImpl reuse problem

2022-03-14 Thread GitBox
congbobo184 opened a new pull request #3110: URL: https://github.com/apache/bookkeeper/pull/3110 Descriptions of the changes in this PR: In bookeeper client the ledgerEntryImpl wrongly recycle cause data parsing errors. `org.apache.bookkeeper.client.PendingReadOp` have more

[GitHub] [bookkeeper] lordcheng10 commented on pull request #3092: add unit tests for reduce unnecessary expansions

2022-03-14 Thread GitBox
lordcheng10 commented on pull request #3092: URL: https://github.com/apache/bookkeeper/pull/3092#issuecomment-1067044969 @merlimat PTAL,thanks! -- 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

[GitHub] [bookkeeper] lordcheng10 commented on pull request #3082: fix reduce unnecessary expansions

2022-03-14 Thread GitBox
lordcheng10 commented on pull request #3082: URL: https://github.com/apache/bookkeeper/pull/3082#issuecomment-1067045284 @merlimat PTAL,thanks! -- 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

[GitHub] [bookkeeper] lordcheng10 removed a comment on pull request #3082: fix reduce unnecessary expansions

2022-03-14 Thread GitBox
lordcheng10 removed a comment on pull request #3082: URL: https://github.com/apache/bookkeeper/pull/3082#issuecomment-1067045284 @merlimat PTAL,thanks! -- 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

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

2022-03-14 Thread GitBox
dlg99 commented on pull request #3106: URL: https://github.com/apache/bookkeeper/pull/3106#issuecomment-1066918627 @Ghatage FYI -- 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] 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-14 Thread GitBox
hangc0276 commented on a change in pull request #2932: URL: https://github.com/apache/bookkeeper/pull/2932#discussion_r826054869 ## File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/directentrylogger/DirectEntryLogger.java ## @@ -0,0 +1,508 @@

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

2022-03-14 Thread GitBox
hangc0276 commented on pull request #2932: URL: https://github.com/apache/bookkeeper/pull/2932#issuecomment-1066974392 ping @eolivelli , Would you please help review this PR? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub