[GitHub] [bookkeeper] lhotari opened a new issue #3105: Improve performance of OrderedExecutor by switching to a more performant BlockingQueue implementation
lhotari opened a new issue #3105: URL: https://github.com/apache/bookkeeper/issues/3105 **FEATURE REQUEST** [OrderedExecutor uses `LinkedBlockingQueue`](https://github.com/apache/bookkeeper/blob/3db4de9da1447e2a5135ac9330d4347b9d220a0d/bookkeeper-common/src/main/java/org/apache/bookkeeper/common/util/OrderedExecutor.java#L307-L308) (when `enableBusyWait`=false). This is not optimal. It looks like @merlimat introduced [GrowableArrayBlockingQueue](https://github.com/apache/bookkeeper/blob/master/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/collections/GrowableArrayBlockingQueue.java) with the intention to replace `LinkedBlockingQueue` usage with `GrowableArrayBlockingQueue`. However the swtich hasn't happened. > In multiple places, (eg: journal, ordered executor, etc..), we are using LinkedBlockingQueue instances to pass objects between threads. See #153 ### Additional context The proposal of this issue is to make this change since it would be an alternative mitigation for #1791 . The changes in #1792 will most likely need to be reverted because the change introduced thread safety issues such as #3104. By improving the queue implementation, there would be better performance without breaking thread safety. -- 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: issues-unsubscr...@bookkeeper.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [bookkeeper] lhotari opened a new issue #3104: Recycled LedgerEntryImpl instances are corrupted due to a thread safety issue in BK client
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 safety issue. It looks like it is caused by the changes in PR #1792. The change breaks thread safety for PendingReadOp handling which doesn't have any synchronization on it's own. The design seems to rely on the fact that the request is created by the same thread that handles the response. Before PR #1792 that was the case. A network request-response call doesn't ensure happens-before and a separate solution for ensuring that would be needed unless PR#1792 is reverted. Happens-before visibility guarantee is needed since there's no synchronization in the PendingReadOp handling. ***To Reproduce*** Steps to reproduce the behavior: Follow instructions in https://github.com/apache/pulsar/issues/14436 ***Expected behavior*** BK client should not contain thread safety issues. -- 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: issues-unsubscr...@bookkeeper.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [bookkeeper] lhotari commented on pull request #1792: Issue #1791: Read Submission should bypass OSE Threads
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 URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@bookkeeper.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [bookkeeper] eolivelli edited a comment on issue #3104: Recycled LedgerEntryImpl instances are corrupted due to a thread safety issue in BK client
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 on master branch and on other active branches and possibly cut a new release -- 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: issues-unsubscr...@bookkeeper.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [bookkeeper] eolivelli commented on pull request #3106: Revert "Issue #1791: Read Submission should bypass OSE Threads"
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 log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@bookkeeper.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [bookkeeper] leizhiyuan commented on pull request #3099: feat: use Runtime.getRuntime().availableProcessors() as thread pool size
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 all clusters have different cpu cores, we need to provider some different configs ,but it can be set by CPU cores automatically -- 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: issues-unsubscr...@bookkeeper.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [bookkeeper] eolivelli commented on issue #3104: Recycled LedgerEntryImpl instances are corrupted due to a thread safety issue in BK client
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 respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@bookkeeper.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [bookkeeper] eolivelli edited a comment on issue #3104: Recycled LedgerEntryImpl instances are corrupted due to a thread safety issue in BK client
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 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: issues-unsubscr...@bookkeeper.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [bookkeeper] eolivelli opened a new pull request #3106: Revert "Issue #1791: Read Submission should bypass OSE Threads"
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" ### Motivation See https://github.com/apache/bookkeeper/issues/3104 -- 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: issues-unsubscr...@bookkeeper.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [bookkeeper] eolivelli commented on issue #3104: Recycled LedgerEntryImpl instances are corrupted due to a thread safety issue in BK client
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 master branch and on other active branches and possibly cut a new release -- 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: issues-unsubscr...@bookkeeper.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [bookkeeper] lhotari commented on pull request #3106: Revert "Issue #1791: Read Submission should bypass OSE Threads"
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 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: issues-unsubscr...@bookkeeper.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [bookkeeper] lhotari opened a new pull request #3107: Run protobuf code generation automatically in IntelliJ and fix config
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 project to IntelliJ
IDEA.
### Changes
- Set generated code directory for protobuf gradle plugin to
`${projectDir}/src/generated` so that code gets generated to a path that isn't
ignored in IntelliJ
- Apply the 'idea' plugin before 'com.google.protobuf' plugin since the
protobuf plugin will only setup source code paths for the generated code if
idea plugin is present
- [Use the `org.jetbrains.gradle.plugin.idea-ext`
plugin](https://github.com/JetBrains/gradle-idea-ext-plugin/wiki#gradle-tasks-triggers-settings)
to run the `generateProto` tasks automatically after importing / refreshing
the Gradle project in IntelliJ.
--
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: issues-unsubscr...@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [bookkeeper] lordcheng10 commented on pull request #3092: add unit tests for reduce unnecessary expansions
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 the specific comment. To unsubscribe, e-mail: issues-unsubscr...@bookkeeper.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [bookkeeper] lordcheng10 commented on pull request #3082: fix reduce unnecessary expansions
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 log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@bookkeeper.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [bookkeeper] eolivelli merged pull request #3090: [website] update website every time there is a change
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: issues-unsubscr...@bookkeeper.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [bookkeeper] gaozhangmin opened a new pull request #3109: Fix getMetadataServiceUri
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 ` instead of `getString` -- 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: issues-unsubscr...@bookkeeper.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [bookkeeper] gaozhangmin commented on a change in pull request #3109: Fix getMetadataServiceUri
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() {
this.conf.setZkLedgersRootPath("/path/to/ledgers");
}
+@Test
+public void testSetGetServiceUri() throws Exception {
+this.conf.setMetadataServiceUri("zk1:2181,zk2:2181/test");
Review comment:
Pulsar broker set serviceUrl like this: `metadata-store:zk:my-zk-1:2181`
--
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: issues-unsubscr...@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [bookkeeper] lhotari commented on pull request #3107: Run protobuf code generation automatically in IntelliJ and fix config
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 ':bookkeeper-dist-server:distZip' to ensure correctness due to the following reasons: > - Gradle detected a problem with the following location: '/Users/enrico.olivelli/dev/bookkeeper/bookkeeper-common-allocator/build/libs/bookkeeper-common-allocator.jar'. Reason: Task ':bookkeeper-dist-server:distZip' uses this output of task ':bookkeeper-common-allocator:jar' without declaring an explicit or implicit dependency. This can lead to incorrect results being produced, depending on what order the tasks are executed. Please refer to https://docs.gradle.org/7.3.3/userguide/validation_problems.html#implicit_dependency for more details about this problem. > - Gradle detected a problem with the following location: '/Users/enrico.olivelli/dev/bookkeeper/bookkeeper-common/build/libs/bookkeeper-common.jar'. Reason: Task ':bookkeeper-dist-server:distZip' uses this output of task ':bookkeeper-common:jar' without declaring an explicit or implicit dependency. This can lead to incorrect results being produced, depending on what order the tasks are executed. Please refer to https://docs.gradle.org/7.3.3/userguide/validation_problems.html#implicit_dependency for more details about this problem. > - Gradle detected a problem with the following location: '/Users/enrico.olivelli/dev/bookkeeper/bookkeeper-http/vertx-http-server/build/libs/vertx-http-server.jar'. Reason: Task ':bookkeeper-dist-server:distZip' uses this output of task ':bookkeeper-http:vertx-http-server:jar' without declaring an explicit or implicit dependency. This can lead to incorrect results being produced, depending on what order the tasks are executed. Please refer to https://docs.gradle.org/7.3.3/userguide/validation_problems.html#implicit_dependency for more details about this problem. > ``` Yes this is an existing problem which isn't caused by this PR. -- 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: issues-unsubscr...@bookkeeper.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [bookkeeper] eolivelli commented on pull request #3109: Fix getMetadataServiceUri
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 the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@bookkeeper.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [bookkeeper] gaozhangmin commented on pull request #3109: Fix getMetadataServiceUri
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. @eolivelli -- 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: issues-unsubscr...@bookkeeper.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [bookkeeper] lhotari edited a comment on pull request #3107: Run protobuf code generation automatically in IntelliJ and fix config
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 ':bookkeeper-dist-server:distZip' to ensure correctness due to the following reasons: > - Gradle detected a problem with the following location: '/Users/enrico.olivelli/dev/bookkeeper/bookkeeper-common-allocator/build/libs/bookkeeper-common-allocator.jar'. Reason: Task ':bookkeeper-dist-server:distZip' uses this output of task ':bookkeeper-common-allocator:jar' without declaring an explicit or implicit dependency. This can lead to incorrect results being produced, depending on what order the tasks are executed. Please refer to https://docs.gradle.org/7.3.3/userguide/validation_problems.html#implicit_dependency for more details about this problem. > - Gradle detected a problem with the following location: '/Users/enrico.olivelli/dev/bookkeeper/bookkeeper-common/build/libs/bookkeeper-common.jar'. Reason: Task ':bookkeeper-dist-server:distZip' uses this output of task ':bookkeeper-common:jar' without declaring an explicit or implicit dependency. This can lead to incorrect results being produced, depending on what order the tasks are executed. Please refer to https://docs.gradle.org/7.3.3/userguide/validation_problems.html#implicit_dependency for more details about this problem. > - Gradle detected a problem with the following location: '/Users/enrico.olivelli/dev/bookkeeper/bookkeeper-http/vertx-http-server/build/libs/vertx-http-server.jar'. Reason: Task ':bookkeeper-dist-server:distZip' uses this output of task ':bookkeeper-http:vertx-http-server:jar' without declaring an explicit or implicit dependency. This can lead to incorrect results being produced, depending on what order the tasks are executed. Please refer to https://docs.gradle.org/7.3.3/userguide/validation_problems.html#implicit_dependency for more details about this problem. > ``` Yes this is an existing problem which isn't caused by this PR. -- 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: issues-unsubscr...@bookkeeper.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [bookkeeper] eolivelli commented on a change in pull request #3109: Fix getMetadataServiceUri
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 getMetadataServiceUriUnchecked() throws
UncheckedConfigurationExce
* @throws ConfigurationException if the metadata service uri is invalid.
*/
public String getMetadataServiceUri() throws ConfigurationException {
-String serviceUri = getString(METADATA_SERVICE_URI);
+List servers = getList(METADATA_SERVICE_URI, null);
Review comment:
This is not correct, METADATA_SERVICE_URI is a URI, it is not a list of
servers
for instance it should start with zk://
##
File path:
bookkeeper-server/src/test/java/org/apache/bookkeeper/conf/AbstractConfigurationTest.java
##
@@ -56,6 +56,12 @@ public void setup() {
this.conf.setZkLedgersRootPath("/path/to/ledgers");
}
+@Test
+public void testSetGetServiceUri() throws Exception {
+this.conf.setMetadataServiceUri("zk1:2181,zk2:2181/test");
Review comment:
this is not a valid URI
it should be something like `zk://zk1:2181,zk2:2181/test` or
`zk+null://zk1:2181,zk2:2181/test`
--
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: issues-unsubscr...@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [bookkeeper] eolivelli commented on pull request #3107: Run protobuf code generation automatically in IntelliJ and fix config
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 respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@bookkeeper.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [bookkeeper] gaozhangmin commented on a change in pull request #3109: Fix getMetadataServiceUri
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() {
this.conf.setZkLedgersRootPath("/path/to/ledgers");
}
+@Test
+public void testSetGetServiceUri() throws Exception {
+this.conf.setMetadataServiceUri("zk1:2181,zk2:2181/test");
Review comment:
https://github.com/apache/pulsar/blob/d02bd7378acd1f202d8bcf50aa6999749e61b20b/pulsar-broker/src/main/java/org/apache/pulsar/PulsarClusterMetadataSetup.java#L246-L248
--
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: issues-unsubscr...@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [bookkeeper] diegosalvi commented on pull request #3106: Revert "Issue #1791: Read Submission should bypass OSE Threads"
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 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: issues-unsubscr...@bookkeeper.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [bookkeeper] lhotari removed a comment on issue #3105: Improve performance of OrderedExecutor by switching to a more performant BlockingQueue implementation
lhotari removed a comment on issue #3105: URL: https://github.com/apache/bookkeeper/issues/3105#issuecomment-1066544555 Pulsar contains GrowableArrayBlockingQueue too, https://github.com/apache/pulsar/blob/master/pulsar-common/src/main/java/org/apache/pulsar/common/util/collections/GrowableArrayBlockingQueue.java . I picked one fix to the BK version of GrowableArrayBlockingQueue when comparing the classes. -- 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: issues-unsubscr...@bookkeeper.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [bookkeeper] lhotari commented on a change in pull request #3108: Issue #3105: Optimize OrderedExecutor performance by using GrowableArrayBlockingQueue
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 poll() {
try {
if (SIZE_UPDATER.get(this) > 0) {
T item = data[headIndex.value];
+data[headIndex.value] = null;
Review comment:
Pulsar contains GrowableArrayBlockingQueue too,
https://github.com/apache/pulsar/blob/master/pulsar-common/src/main/java/org/apache/pulsar/common/util/collections/GrowableArrayBlockingQueue.java
. I picked this fix to the BK version of GrowableArrayBlockingQueue when
comparing the classes.
I didn't see an explicit test.
--
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: issues-unsubscr...@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [bookkeeper] lhotari commented on a change in pull request #3108: Issue #3105: Optimize OrderedExecutor performance by using GrowableArrayBlockingQueue
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 poll() {
try {
if (SIZE_UPDATER.get(this) > 0) {
T item = data[headIndex.value];
+data[headIndex.value] = null;
Review comment:
I'll look more into it.
--
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: issues-unsubscr...@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [bookkeeper] lhotari commented on issue #3105: Improve performance of OrderedExecutor by switching to a more performant BlockingQueue implementation
lhotari commented on issue #3105: URL: https://github.com/apache/bookkeeper/issues/3105#issuecomment-1066544555 Pulsar contains GrowableArrayBlockingQueue too, https://github.com/apache/pulsar/blob/master/pulsar-common/src/main/java/org/apache/pulsar/common/util/collections/GrowableArrayBlockingQueue.java . I picked one fix to the BK version of GrowableArrayBlockingQueue when comparing the classes. -- 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: issues-unsubscr...@bookkeeper.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [bookkeeper] lhotari opened a new pull request #3108: Issue #3105: Optimize OrderedExecutor performance by using GrowableArrayBlockingQueue
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 object allocations each time new items are added to the queue. More details in #153 where GrowableArrayBlockingQueue was added. ### Changes - Use GrowableArrayBlockingQueue introduced in #153 -- 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: issues-unsubscr...@bookkeeper.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [bookkeeper] eolivelli commented on a change in pull request #3108: Issue #3105: Optimize OrderedExecutor performance by using GrowableArrayBlockingQueue
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 T poll() {
try {
if (SIZE_UPDATER.get(this) > 0) {
T item = data[headIndex.value];
+data[headIndex.value] = null;
Review comment:
is there a test that cover this change ?
--
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: issues-unsubscr...@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [bookkeeper] lhotari commented on pull request #3107: Run protobuf code generation automatically in IntelliJ and fix config
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 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: issues-unsubscr...@bookkeeper.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [bookkeeper] eolivelli commented on pull request #3107: Run protobuf code generation automatically in IntelliJ and fix config
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 task ':bookkeeper-dist:distZip' to ensure correctness due to the following reasons: - Gradle detected a problem with the following location: '/Users/enrico.olivelli/dev/bookkeeper'. Reason: Task ':bookkeeper-dist:distZip' uses this output of task ':bookkeeper-proto:generateProto' without declaring an explicit or implicit dependency. This can lead to incorrect results being produced, depending on what order the tasks are executed. Please refer to https://docs.gradle.org/7.3.3/userguide/validation_problems.html#implicit_dependency for more details about this problem. ``` -- 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: issues-unsubscr...@bookkeeper.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [bookkeeper] lhotari commented on a change in pull request #3108: Issue #3105: Optimize OrderedExecutor performance by using GrowableArrayBlockingQueue
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 poll() {
try {
if (SIZE_UPDATER.get(this) > 0) {
T item = data[headIndex.value];
+data[headIndex.value] = null;
Review comment:
https://github.com/apache/pulsar/pull/1292/files#diff-013ecf9234e6aaa381510e81420c91664149af6d7f16866f1a36b5b3eb71aadcR87
is where the fix was made. It seems that there's not an explicit test for it.
--
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: issues-unsubscr...@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [bookkeeper] lhotari commented on pull request #3108: Issue #3105: Optimize OrderedExecutor performance by using GrowableArrayBlockingQueue
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 GrowableArrayBlockingQueue too, https://github.com/apache/pulsar/blob/master/pulsar-common/src/main/java/org/apache/pulsar/common/util/collections/GrowableArrayBlockingQueue.java . I picked this fix to the BK version of GrowableArrayBlockingQueue when comparing the classes. The Pulsar version contains also a few other features such as `remove` methods, `forEach` and `toList`. I didn't copy those over. https://github.com/apache/pulsar/pull/1292/files#diff-013ecf9234e6aaa381510e81420c91664149af6d7f16866f1a36b5b3eb71aadcR87 is where the fix was made. It seems that there's not an explicit test for the issue that was fixed there. I guess one way to test for it would be it inspect the internal state? -- 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: issues-unsubscr...@bookkeeper.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [bookkeeper] lhotari commented on pull request #3107: Run protobuf code generation automatically in IntelliJ and fix config
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, but `./gradlew build -x test` fails with `Cannot perform signing task ':bookkeeper-dist:signDistTar' because it has no configured signatory`, I guess there's something else I'd have to do.) -- 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: issues-unsubscr...@bookkeeper.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [bookkeeper] lhotari edited a comment on pull request #3108: Issue #3105: Optimize OrderedExecutor performance by using GrowableArrayBlockingQueue
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 GrowableArrayBlockingQueue too, https://github.com/apache/pulsar/blob/master/pulsar-common/src/main/java/org/apache/pulsar/common/util/collections/GrowableArrayBlockingQueue.java . I picked this fix to the BK version of GrowableArrayBlockingQueue when comparing the classes. The Pulsar version contains also a few other features such as `remove` methods, `forEach` and `toList`. I didn't copy those over. https://github.com/apache/pulsar/pull/1292/files#diff-013ecf9234e6aaa381510e81420c91664149af6d7f16866f1a36b5b3eb71aadcR87 is where the fix was made. It seems that there's not an explicit test for the issue that was fixed there. I guess one way to test for it would be it inspect the internal state? @merlimat Do you have suggestions how to handle the testing for the fix? -- 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: issues-unsubscr...@bookkeeper.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [bookkeeper] hangc0276 commented on pull request #2932: direct-io: add support for bypassing operating system I/O cache when logging entries
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 bookkeeper-slogger-api.jar unaccounted for in LICENSE bookkeeper-slogger-slf4j.jar unaccounted for in LICENSE org.slf4j-slf4j-api-1.7.36.jar unaccounted for in LICENSE org.slf4j-slf4j-api-1.7.32.jar mentioned in LICENSE, but not bundled ``` -- 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: issues-unsubscr...@bookkeeper.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [bookkeeper] jvrao commented on pull request #3106: Revert "Issue #1791: Read Submission should bypass OSE Threads"
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 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? -- 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: issues-unsubscr...@bookkeeper.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[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
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 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+package org.apache.bookkeeper.bookie.storage.directentrylogger;
+
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkState;
+import static org.apache.bookkeeper.common.util.ExceptionMessageHelper.exMsg;
+
+import io.netty.buffer.ByteBuf;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Iterator;
+import java.util.List;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Future;
+
+import org.apache.bookkeeper.common.util.nativeio.NativeIO;
+import org.apache.bookkeeper.common.util.nativeio.NativeIOException;
+import org.apache.bookkeeper.slogger.Slogger;
+import org.apache.commons.lang3.SystemUtils;
+
+class DirectWriter implements LogWriter {
+final NativeIO nativeIO;
+final int fd;
+final int id;
+final String filename;
+final BufferPool bufferPool;
+final ExecutorService writeExecutor;
+final Object bufferLock = new Object();
+final List> outstandingWrites = new ArrayList>();
+Buffer nativeBuffer;
+long offset;
+private static volatile boolean useFallocate = true;
+
+DirectWriter(int id,
+ String filename,
+ long maxFileSize,
+ ExecutorService writeExecutor,
+ BufferPool bufferPool,
+ NativeIO nativeIO, Slogger slog) throws IOException {
+checkArgument(maxFileSize > 0, "Max file size (%d) must be positive");
+this.id = id;
+this.filename = filename;
+this.writeExecutor = writeExecutor;
+this.nativeIO = nativeIO;
+
+offset = 0;
+
+try {
+fd = nativeIO.open(filename,
+ NativeIO.O_CREAT | NativeIO.O_WRONLY |
NativeIO.O_DIRECT,
+ 00755);
+checkState(fd >= 0, "Open should have thrown exception, fd is
invalid : %d", fd);
+} catch (NativeIOException ne) {
+throw new IOException(exMsg(ne.getMessage()).kv("file", filename)
+ .kv("errno", ne.getErrno()).toString(), ne);
+}
+
+if (useFallocate) {
+if (!SystemUtils.IS_OS_LINUX) {
+useFallocate = false;
+slog.warn(Events.FALLOCATE_NOT_AVAILABLE);
+} else {
+try {
+int ret = nativeIO.fallocate(fd,
NativeIO.FALLOC_FL_ZERO_RANGE, 0, maxFileSize);
+checkState(ret == 0, "Exception should have been thrown on
non-zero ret: %d", ret);
+} catch (NativeIOException ex) {
+// fallocate(2) is not supported on all filesystems.
Since this is an optimization, disable
+// subsequent usage instead of failing the operation.
+useFallocate = false;
+slog.kv("message", ex.getMessage())
+.kv("file", filename)
+.kv("errno", ex.getErrno())
+.warn(Events.FALLOCATE_NOT_AVAILABLE);
+}
+}
+}
+
+this.bufferPool = bufferPool;
+this.nativeBuffer = bufferPool.acquire();
+}
+
+@Override
+public int logId() {
+return id;
+}
+
+@Override
+public void writeAt(long offset, ByteBuf buf) throws IOException {
+checkArgument(Buffer.isAligned(offset),
+ "Offset to writeAt must be aligned to %d: %d is not",
Buffer.ALIGNMENT, offset);
+checkArgument(Buffer.isAligned(buf.readableBytes()),
+ "Buffer must write multiple of alignment bytes (%d), %d
is not",
+ Buffer.ALIGNMENT, buf.readableBytes());
+Buffer tmpBuffer = bufferPool.acquire();
+int bytesToWrite = buf.readableBytes();
+
[GitHub] [bookkeeper] gaozhangmin closed pull request #3109: Fix getMetadataServiceUri
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: issues-unsubscr...@bookkeeper.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [bookkeeper] congbobo184 commented on pull request #3110: Fix ledgerEntryImpl reuse problem
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 specific comment. To unsubscribe, e-mail: issues-unsubscr...@bookkeeper.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [bookkeeper] congbobo184 opened a new pull request #3110: Fix ledgerEntryImpl reuse problem
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 `LedgerEntryRequest` for example in PendingReadOp(1) has two LedgerEntryRequest (entryId = 1, entryId = 2) another PendingReadOp(2) has two LedgerEntryRequest (entryId = 3, entryId = 4) 1. When `LedgerEntryRequest (entryId = 1)` read fail will close all `LedgerEntryRequest (entryId = 1, entryId = 2)` in `PendingReadOp(1)`. 2. `LedgerEntryImpl` in `LedgerEntryRequest (entryId = 2)` will be recycle. 3. `LedgerEntryRequest(entryId = 3`) will reuse this `LedgerEntryImpl` in another `PendingReadOp(2)` because the `LedgerEntryImpl` in `LedgerEntryRequest (entryId = 2)` has been recycle. 4. Another `LedgerEntryRequest(entryId = 3)` readComplete will set the ByteBuf to this `LedgerEntryImpl` 5. `LedgerEntryRequest(entryId = 2)` readComplete also use this `LedgerEntryImpl` and set the ByteBuf to this `LedgerEntryImpl`. 6. Now the `LedgerEntryImpl` the entryId is 3 but the ByteBuf is entryId = 2. ### Motivation fix this problem ### Changes when `LedgerEntryRequest(entryId = 2)` reponse come can't change the `LedgerEntryImpl` ByteBuf when LedgerEntryRequest change the complete to true, the response will not change the ByteBuf. because `LedgerEntryRequest(entryId = 2)` readComplete will check complete the same Ledger read will in the same thread. -- 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: issues-unsubscr...@bookkeeper.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [bookkeeper] lordcheng10 commented on pull request #3092: add unit tests for reduce unnecessary expansions
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 the specific comment. To unsubscribe, e-mail: issues-unsubscr...@bookkeeper.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [bookkeeper] lordcheng10 commented on pull request #3082: fix reduce unnecessary expansions
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 the specific comment. To unsubscribe, e-mail: issues-unsubscr...@bookkeeper.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [bookkeeper] lordcheng10 removed a comment on pull request #3082: fix reduce unnecessary expansions
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 go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@bookkeeper.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [bookkeeper] dlg99 commented on pull request #3106: Revert "Issue #1791: Read Submission should bypass OSE Threads"
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 comment. To unsubscribe, e-mail: issues-unsubscr...@bookkeeper.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[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
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 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+package org.apache.bookkeeper.bookie.storage.directentrylogger;
+
+import static com.google.common.base.Preconditions.checkArgument;
+import static
org.apache.bookkeeper.bookie.TransactionalEntryLogCompactor.COMPACTING_SUFFIX;
+import static org.apache.bookkeeper.common.util.ExceptionMessageHelper.exMsg;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.cache.Cache;
+import com.google.common.cache.CacheBuilder;
+import com.google.common.cache.RemovalListener;
+
+import io.netty.buffer.ByteBuf;
+import io.netty.buffer.ByteBufAllocator;
+import java.io.EOFException;
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+import java.util.Set;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+import java.util.regex.Matcher;
+import java.util.stream.Collectors;
+
+import org.apache.bookkeeper.bookie.AbstractLogCompactor;
+import org.apache.bookkeeper.bookie.Bookie.NoEntryException;
+import org.apache.bookkeeper.bookie.EntryLogMetadata;
+import org.apache.bookkeeper.bookie.storage.CompactionEntryLog;
+import org.apache.bookkeeper.bookie.storage.EntryLogIds;
+import org.apache.bookkeeper.bookie.storage.EntryLogIdsImpl;
+import org.apache.bookkeeper.bookie.storage.EntryLogScanner;
+import org.apache.bookkeeper.bookie.storage.EntryLoggerIface;
+import org.apache.bookkeeper.common.util.nativeio.NativeIO;
+import org.apache.bookkeeper.slogger.Slogger;
+import org.apache.bookkeeper.stats.StatsLogger;
+
+/**
+ * DirectEntryLogger.
+ */
+public class DirectEntryLogger implements EntryLoggerIface {
+private static final String LOGFILE_SUFFIX = ".log";
+private final Slogger slog;
+private final File ledgerDir;
+private final EntryLogIds ids;
+private final ExecutorService writeExecutor;
+private final ExecutorService flushExecutor;
+private final long maxFileSize;
+private final DirectEntryLoggerStats stats;
+private final ByteBufAllocator allocator;
+private final BufferPool writeBuffers;
+private final int readBufferSize;
+private final int maxSaneEntrySize;
+private final Set unflushedLogs;
+
+private WriterWithMetadata curWriter;
+
+private List> pendingFlushes;
+private final NativeIO nativeIO;
+private final List> allCaches = new CopyOnWriteArrayList<>();
+private final ThreadLocal> caches;
+
+private static final int NUMBER_OF_WRITE_BUFFERS = 8;
+
+public DirectEntryLogger(File ledgerDir,
+ EntryLogIds ids,
+ NativeIO nativeIO,
+ ByteBufAllocator allocator,
+ ExecutorService writeExecutor,
+ ExecutorService flushExecutor,
+ long maxFileSize,
+ int maxSaneEntrySize,
+ long totalWriteBufferSize,
+ long totalReadBufferSize,
+ int readBufferSize,
+ int numReadThreads,
+ int maxFdCacheTimeSeconds,
+ Slogger slogParent,
+ StatsLogger stats) throws IOException {
+this.ledgerDir = ledgerDir;
+this.flushExecutor = flushExecutor;
+this.writeExecutor = writeExecutor;
+this.pendingFlushes = new ArrayList<>();
+this.nativeIO = nativeIO;
+this.unflushedLogs = ConcurrentHashMap.newKeySet();
+
+
[GitHub] [bookkeeper] hangc0276 commented on pull request #2932: direct-io: add support for bypassing operating system I/O cache when logging entries
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 and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@bookkeeper.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
