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/03/23 08:13:18 UTC

[GitHub] [bookkeeper] dmercuriali opened a new pull request #2296: #2288: add metadata to StreamStorage ledgers

dmercuriali opened a new pull request #2296:  #2288: add metadata to StreamStorage ledgers
URL: https://github.com/apache/bookkeeper/pull/2296
 
 
   Descriptions of the changes in this PR:
   
   Added 'application' and 'component' metadata to StreamStorage ledgers.
   application: bk-stream-storage-service
   component: state-store / checkpoint-store
   
   Master Issue: #2288 
   
   > - [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 #2296: #2288: add metadata to StreamStorage ledgers

Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #2296:  #2288: add metadata to StreamStorage ledgers
URL: https://github.com/apache/bookkeeper/pull/2296#discussion_r396318361
 
 

 ##########
 File path: stream/statelib/src/main/java/org/apache/bookkeeper/statelib/impl/rocksdb/checkpoint/dlog/DLCheckpointStore.java
 ##########
 @@ -98,6 +100,9 @@ public OutputStream openOutputStream(String filePath) throws IOException {
         try {
             DistributedLogManager dlm = namespace.openLog(
                 filePath);
+            LedgerMetadata metadata = new LedgerMetadata();
+            metadata.setApplication(Constants.LEDGER_METADATA_APPLICATION_STREAM_STORAGE);
+            metadata.setComponent("checkpoint-store");
 
 Review comment:
   ok

----------------------------------------------------------------
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 #2296: #2288: add metadata to StreamStorage ledgers

Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #2296:  #2288: add metadata to StreamStorage ledgers
URL: https://github.com/apache/bookkeeper/pull/2296#discussion_r396293015
 
 

 ##########
 File path: stream/statelib/src/main/java/org/apache/bookkeeper/statelib/impl/rocksdb/checkpoint/dlog/DLCheckpointStore.java
 ##########
 @@ -98,6 +100,9 @@ public OutputStream openOutputStream(String filePath) throws IOException {
         try {
             DistributedLogManager dlm = namespace.openLog(
                 filePath);
+            LedgerMetadata metadata = new LedgerMetadata();
+            metadata.setApplication(Constants.LEDGER_METADATA_APPLICATION_STREAM_STORAGE);
+            metadata.setComponent("checkpoint-store");
 
 Review comment:
   this looks like a dead store,
   you are not using this variable

----------------------------------------------------------------
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] jiazhai merged pull request #2296: #2288: add metadata to StreamStorage ledgers

Posted by GitBox <gi...@apache.org>.
jiazhai merged pull request #2296:  #2288: add metadata to StreamStorage ledgers
URL: https://github.com/apache/bookkeeper/pull/2296
 
 
   

----------------------------------------------------------------
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 #2296: #2288: add metadata to StreamStorage ledgers

Posted by GitBox <gi...@apache.org>.
eolivelli commented on issue #2296:  #2288: add metadata to StreamStorage ledgers
URL: https://github.com/apache/bookkeeper/pull/2296#issuecomment-602464451
 
 
   @mino181295 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] mino181295 commented on issue #2296: #2288: add metadata to StreamStorage ledgers

Posted by GitBox <gi...@apache.org>.
mino181295 commented on issue #2296:  #2288: add metadata to StreamStorage ledgers
URL: https://github.com/apache/bookkeeper/pull/2296#issuecomment-602532995
 
 
   Looks good to me

----------------------------------------------------------------
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 #2296: #2288: add metadata to StreamStorage ledgers

Posted by GitBox <gi...@apache.org>.
eolivelli commented on issue #2296:  #2288: add metadata to StreamStorage ledgers
URL: https://github.com/apache/bookkeeper/pull/2296#issuecomment-602484590
 
 
   @sijie all tests are passing.
   can you please take a look ?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above 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 #2296: #2288: add metadata to StreamStorage ledgers

Posted by GitBox <gi...@apache.org>.
dmercuriali commented on a change in pull request #2296:  #2288: add metadata to StreamStorage ledgers
URL: https://github.com/apache/bookkeeper/pull/2296#discussion_r396309472
 
 

 ##########
 File path: stream/statelib/src/main/java/org/apache/bookkeeper/statelib/impl/rocksdb/checkpoint/dlog/DLCheckpointStore.java
 ##########
 @@ -98,6 +100,9 @@ public OutputStream openOutputStream(String filePath) throws IOException {
         try {
             DistributedLogManager dlm = namespace.openLog(
                 filePath);
+            LedgerMetadata metadata = new LedgerMetadata();
+            metadata.setApplication(Constants.LEDGER_METADATA_APPLICATION_STREAM_STORAGE);
+            metadata.setComponent("checkpoint-store");
 
 Review comment:
   passed to the openAsyncLogWriter call

----------------------------------------------------------------
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