You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@bookkeeper.apache.org by GitBox <gi...@apache.org> on 2020/01/25 14:33:04 UTC
[GitHub] [bookkeeper] dmercuriali opened a new pull request #2254: Issue
2218: Write customMetadata on ledgers created by DistributedLog
dmercuriali opened a new pull request #2254: Issue 2218: Write customMetadata on ledgers created by DistributedLog
URL: https://github.com/apache/bookkeeper/pull/2254
### Changes
Enable DistributedLog to attach custom metadata to underlying ledgers when writing logs.
Master Issue: #2218
> - [X] [skip bookkeeper-server bookie tests]: skip testing `org.apache.bookkeeper.bookie` in bookkeeper-server module.
> - [X] [skip bookkeeper-server client tests]: skip testing `org.apache.bookkeeper.client` in bookkeeper-server module.
> - [X] [skip bookkeeper-server replication tests]: skip testing `org.apache.bookkeeper.replication` in bookkeeper-server module.
> - [X] [skip bookkeeper-server tls tests]: skip testing `org.apache.bookkeeper.tls` in bookkeeper-server module.
> - [X] [skip bookkeeper-server remaining tests]: skip testing all other tests in bookkeeper-server module.
> - [ ] [skip integration tests]: skip docker based integration tests. if you make java code changes, you shouldn't skip integration tests.
> - [ ] [skip build java8]: skip build on java8. *ONLY* skip this when *ONLY* changing files under documentation under `site`.
> - [ ] [skip build java11]: skip build on java11. *ONLY* skip this when *ONLY* changing files under documentation under `site`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [bookkeeper] eolivelli commented on a change in pull request #2254:
Issue 2218: Write customMetadata on ledgers created by DistributedLog
Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #2254: Issue 2218: Write customMetadata on ledgers created by DistributedLog
URL: https://github.com/apache/bookkeeper/pull/2254#discussion_r370938668
##########
File path: stream/distributedlog/core/src/main/java/org/apache/distributedlog/api/DistributedLogManager.java
##########
@@ -104,6 +105,17 @@
*/
CompletableFuture<AsyncLogWriter> openAsyncLogWriter();
+ /**
+ * Open async log writer to write records to the log stream.
+ * Provided metadata will be attached to the underlying BookKeeper ledgers.
+ *
+ * @param ledgerMetadata
+ * @return result represents the open result
+ */
+ default CompletableFuture<AsyncLogWriter> openAsyncLogWriter(LedgerMetadata ledgerMetadata) {
+ throw new UnsupportedOperationException();
Review comment:
Return a failed future
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [bookkeeper] dmercuriali commented on issue #2254: Issue 2218:
Write customMetadata on ledgers created by DistributedLog
Posted by GitBox <gi...@apache.org>.
dmercuriali commented on issue #2254: Issue 2218: Write customMetadata on ledgers created by DistributedLog
URL: https://github.com/apache/bookkeeper/pull/2254#issuecomment-581970432
> @dmercuriali the approach is good.
> You have some javac error:
>
> ```
>
> 2020-01-30T17:03:44.4179776Z Downloaded from central: https://repo.maven.apache.org/maven2/org/apache/httpcomponents/httpclient/4.4.1/httpclient-4.4.1.jar (721 kB at 6.3 MB/s)
> 2020-01-30T17:04:01.7976679Z [ERROR] COMPILATION ERROR :
> 2020-01-30T17:04:01.7986913Z [ERROR] /home/runner/work/bookkeeper/bookkeeper/stream/distributedlog/core/src/main/java/org/apache/distributedlog/api/DistributedLogManager.java:[116,33] cannot find symbol
> 2020-01-30T17:04:01.7991275Z symbol: method failedFuture(java.lang.UnsupportedOperationException)
> 2020-01-30T17:04:01.7994806Z location: class java.util.concurrent.CompletableFuture
> 2020-01-30T17:04:01.8023930Z [ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.8.1:compile (default-compile) on project distributedlog-core: Compilation failure
> 2020-01-30T17:04:01.8033444Z [ERROR] /home/runner/work/bookkeeper/bookkeeper/stream/distributedlog/core/src/main/java/org/apache/distributedlog/api/DistributedLogManager.java:[116,33] cannot find symbol
> 2020-01-30T17:04:01.8039627Z [ERROR] symbol: method failedFuture(java.lang.UnsupportedOperationException)
> 2020-01-30T17:04:01.8044182Z [ERROR] location: class java.util.concurrent.CompletableFuture
> 2020-01-30T17:04:01.8045720Z [ERROR]
> ```
Fixed, my bad for building with jdk-11 locally.
Also added some tests.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [bookkeeper] eolivelli commented on issue #2254: Issue 2218: Write
customMetadata on ledgers created by DistributedLog
Posted by GitBox <gi...@apache.org>.
eolivelli commented on issue #2254: Issue 2218: Write customMetadata on ledgers created by DistributedLog
URL: https://github.com/apache/bookkeeper/pull/2254#issuecomment-586983825
merged to master branch, it will be available from 4.11.0 onwards
thank you @dmercuriali
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [bookkeeper] dmercuriali edited a comment on issue #2254: Issue
2218: Write customMetadata on ledgers created by DistributedLog
Posted by GitBox <gi...@apache.org>.
dmercuriali edited a comment on issue #2254: Issue 2218: Write customMetadata on ledgers created by DistributedLog
URL: https://github.com/apache/bookkeeper/pull/2254#issuecomment-582319966
> [ERROR] Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:2.21.0:test (default-test) on project smoke: There was a timeout or other error in the fork -> [Help 1]
Can someone trigger a new run for the failing jobs?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [bookkeeper] dmercuriali commented on issue #2254: Issue 2218:
Write customMetadata on ledgers created by DistributedLog
Posted by GitBox <gi...@apache.org>.
dmercuriali commented on issue #2254: Issue 2218: Write customMetadata on ledgers created by DistributedLog
URL: https://github.com/apache/bookkeeper/pull/2254#issuecomment-582319966
> [ERROR] Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:2.21.0:test (default-test) on project smoke: There was a timeout or other error in the fork -> [Help 1]
Can someone trigger a new run for the failing job?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [bookkeeper] eolivelli commented on a change in pull request #2254:
Issue 2218: Write customMetadata on ledgers created by DistributedLog
Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #2254: Issue 2218: Write customMetadata on ledgers created by DistributedLog
URL: https://github.com/apache/bookkeeper/pull/2254#discussion_r374797331
##########
File path: stream/distributedlog/core/src/main/java/org/apache/distributedlog/bk/LedgerMetadata.java
##########
@@ -0,0 +1,71 @@
+/*
+ * Copyright 2020 The Apache Software Foundation.
+ *
+ * Licensed 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.distributedlog.bk;
+
+import java.nio.charset.StandardCharsets;
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * Ledger metadata.
+ */
+public final class LedgerMetadata {
+
+ private String application;
+ private String component;
+ private Map<String, String> customMetadata;
+
+ public void setApplication(String application) {
+ this.application = application;
+ }
+
+ public void setComponent(String component) {
+ this.component = component;
+ }
+
+ public void addCustomMetadata(String key, String value) {
+ if (key == null || "".equals(key.trim())) {
+ throw new IllegalArgumentException("Metadata key cant be empty");
+ }
+
+ if (customMetadata == null) {
+ customMetadata = new HashMap<>();
+ }
+
+ customMetadata.put(key, value);
+ }
+
+ public Map<String, byte[]> getMetadata() {
+ Map<String, byte[]> meta = new HashMap<>();
+ if (application != null) {
+ meta.put("application", application.getBytes(StandardCharsets.UTF_8));
+ }
+ if (component != null) {
+ meta.put("component", component.getBytes(StandardCharsets.UTF_8));
+ }
+
+ if (customMetadata != null) {
+ for (Map.Entry<String, String> e : customMetadata.entrySet()) {
+ String value = e.getValue();
+ meta.put(e.getKey(), value == null
+ ? null
Review comment:
It doesn't make sense to have a null value here.
We should fail with a null key or value
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [bookkeeper] eolivelli commented on issue #2254: Issue 2218: Write
customMetadata on ledgers created by DistributedLog
Posted by GitBox <gi...@apache.org>.
eolivelli commented on issue #2254: Issue 2218: Write customMetadata on ledgers created by DistributedLog
URL: https://github.com/apache/bookkeeper/pull/2254#issuecomment-586684881
@dmercuriali can you please rebase onto latest master ?
I have fixed the integration tests issue (#2265 )
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [bookkeeper] eolivelli merged pull request #2254: Issue 2218: Write
customMetadata on ledgers created by DistributedLog
Posted by GitBox <gi...@apache.org>.
eolivelli merged pull request #2254: Issue 2218: Write customMetadata on ledgers created by DistributedLog
URL: https://github.com/apache/bookkeeper/pull/2254
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [bookkeeper] eolivelli commented on issue #2254: Issue 2218: Write
customMetadata on ledgers created by DistributedLog
Posted by GitBox <gi...@apache.org>.
eolivelli commented on issue #2254: Issue 2218: Write customMetadata on ledgers created by DistributedLog
URL: https://github.com/apache/bookkeeper/pull/2254#issuecomment-582107687
Some ci job failed PTAL
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [bookkeeper] eolivelli commented on issue #2254: Issue 2218: Write
customMetadata on ledgers created by DistributedLog
Posted by GitBox <gi...@apache.org>.
eolivelli commented on issue #2254: Issue 2218: Write customMetadata on ledgers created by DistributedLog
URL: https://github.com/apache/bookkeeper/pull/2254#issuecomment-586685699
Actually we have to merge this patch as well in order to have a fully working github actions env
https://github.com/apache/bookkeeper/pull/2266
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [bookkeeper] eolivelli commented on issue #2254: Issue 2218: Write
customMetadata on ledgers created by DistributedLog
Posted by GitBox <gi...@apache.org>.
eolivelli commented on issue #2254: Issue 2218: Write customMetadata on ledgers created by DistributedLog
URL: https://github.com/apache/bookkeeper/pull/2254#issuecomment-578414492
@dmercuriali palese writer in the description how the client application can set custom ledger metadata.
Otherwise from the PR it is hard to understand how this change can be used by users
Thanks for working on this
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [bookkeeper] dmercuriali commented on a change in pull request
#2254: Issue 2218: Write customMetadata on ledgers created by DistributedLog
Posted by GitBox <gi...@apache.org>.
dmercuriali commented on a change in pull request #2254: Issue 2218: Write customMetadata on ledgers created by DistributedLog
URL: https://github.com/apache/bookkeeper/pull/2254#discussion_r374894665
##########
File path: stream/distributedlog/core/src/main/java/org/apache/distributedlog/bk/LedgerMetadata.java
##########
@@ -0,0 +1,71 @@
+/*
+ * Copyright 2020 The Apache Software Foundation.
+ *
+ * Licensed 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.distributedlog.bk;
+
+import java.nio.charset.StandardCharsets;
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * Ledger metadata.
+ */
+public final class LedgerMetadata {
+
+ private String application;
+ private String component;
+ private Map<String, String> customMetadata;
+
+ public void setApplication(String application) {
+ this.application = application;
+ }
+
+ public void setComponent(String component) {
+ this.component = component;
+ }
+
+ public void addCustomMetadata(String key, String value) {
+ if (key == null || "".equals(key.trim())) {
+ throw new IllegalArgumentException("Metadata key cant be empty");
+ }
+
+ if (customMetadata == null) {
+ customMetadata = new HashMap<>();
+ }
+
+ customMetadata.put(key, value);
+ }
+
+ public Map<String, byte[]> getMetadata() {
+ Map<String, byte[]> meta = new HashMap<>();
+ if (application != null) {
+ meta.put("application", application.getBytes(StandardCharsets.UTF_8));
+ }
+ if (component != null) {
+ meta.put("component", component.getBytes(StandardCharsets.UTF_8));
+ }
+
+ if (customMetadata != null) {
+ for (Map.Entry<String, String> e : customMetadata.entrySet()) {
+ String value = e.getValue();
+ meta.put(e.getKey(), value == null
+ ? null
Review comment:
added a guard clause in LedgerMetadata#addCustomMetadata like the one for the key
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [bookkeeper] eolivelli commented on issue #2254: Issue 2218: Write
customMetadata on ledgers created by DistributedLog
Posted by GitBox <gi...@apache.org>.
eolivelli commented on issue #2254: Issue 2218: Write customMetadata on ledgers created by DistributedLog
URL: https://github.com/apache/bookkeeper/pull/2254#issuecomment-582007366
@dmercuriali I left one little comment, we have FutureUtils utility in order to create a failed Future
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [bookkeeper] eolivelli commented on issue #2254: Issue 2218: Write
customMetadata on ledgers created by DistributedLog
Posted by GitBox <gi...@apache.org>.
eolivelli commented on issue #2254: Issue 2218: Write customMetadata on ledgers created by DistributedLog
URL: https://github.com/apache/bookkeeper/pull/2254#issuecomment-581655307
@dmercuriali the approach is good.
You have some javac error:
```
2020-01-30T17:03:44.4179776Z Downloaded from central: https://repo.maven.apache.org/maven2/org/apache/httpcomponents/httpclient/4.4.1/httpclient-4.4.1.jar (721 kB at 6.3 MB/s)
2020-01-30T17:04:01.7976679Z [ERROR] COMPILATION ERROR :
2020-01-30T17:04:01.7986913Z [ERROR] /home/runner/work/bookkeeper/bookkeeper/stream/distributedlog/core/src/main/java/org/apache/distributedlog/api/DistributedLogManager.java:[116,33] cannot find symbol
2020-01-30T17:04:01.7991275Z symbol: method failedFuture(java.lang.UnsupportedOperationException)
2020-01-30T17:04:01.7994806Z location: class java.util.concurrent.CompletableFuture
2020-01-30T17:04:01.8023930Z [ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.8.1:compile (default-compile) on project distributedlog-core: Compilation failure
2020-01-30T17:04:01.8033444Z [ERROR] /home/runner/work/bookkeeper/bookkeeper/stream/distributedlog/core/src/main/java/org/apache/distributedlog/api/DistributedLogManager.java:[116,33] cannot find symbol
2020-01-30T17:04:01.8039627Z [ERROR] symbol: method failedFuture(java.lang.UnsupportedOperationException)
2020-01-30T17:04:01.8044182Z [ERROR] location: class java.util.concurrent.CompletableFuture
2020-01-30T17:04:01.8045720Z [ERROR]
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services