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/07/17 17:34:55 UTC

[GitHub] [bookkeeper] odidev opened a new pull request #2379: Updated netty,netty-boringssl and rocksdb

odidev opened a new pull request #2379:
URL: https://github.com/apache/bookkeeper/pull/2379


   ### Descriptions of the changes in this PR:
   
   -Updated netty,netty-boringssl and rocksdb to latest version for aarch64 support
   -Updated deprecated methods of rocksdb to remove compilation warning and resolve build failure of bookkeeper-server on amd64 and aarch64 platforms as Werror flag is enabled.
   
   ### Motivation
   Build of the bookkeeper-server package was failing on amd64 and aarch64 platforms.
   
   ### Changes
   There are many methods that are marked deprecated in rocksdb, but in use by bookkeeper package.
   So updated those methods in bookkeeper package according to current rocksdb implementation.
   Also updated netty,netty-boringssl and rocksdb to latest version as they are having aarch64 support.
   
   Master Issue: #2378 
   


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



[GitHub] [bookkeeper] odidev commented on pull request #2379: Updated netty,netty-boringssl and rocksdb

Posted by GitBox <gi...@apache.org>.
odidev commented on pull request #2379:
URL: https://github.com/apache/bookkeeper/pull/2379#issuecomment-661245588


   @eolivelli , 
   
   > I see this is a major version bump for RocksDB.
   > Is the upgrade of RocksDB automatic? I am talking about files on disk. Do you need manual upgrade of data files?
   > In case of problems, if you rollback rocksdb to previous version, will the bookie work?
   
   Latest RocksDB is supported on aarch64, and its latest version was not being used in bookkeeper. So manually boomed RocksDB in bookkeeper in this PR.If rollback rocksdb to previous version , various tests are failing on aarch64 with JNI error like:
   `
   [ERROR] test(org.apache.bookkeeper.bookie.storage.ldb.ConversionTest)  Time elapsed: 2.506 s  <<< ERROR!
   com.google.common.util.concurrent.UncheckedExecutionException: Failed to load RocksDB JNI library
           at org.apache.bookkeeper.bookie.storage.ldb.ConversionTest.test(ConversionTest.java:118)
   Caused by: java.io.IOException: Failed to load RocksDB JNI library
           at org.apache.bookkeeper.bookie.storage.ldb.ConversionTest.test(ConversionTest.java:118)
   Caused by: java.lang.UnsatisfiedLinkError: /tmp/librocksdbjni4423711401334751285.so: /tmp/librocksdbjni4423711401334751285.so: cannot open shared object file: No such file or directory (Possible cause: can't load AMD 64-bit .so on a AARCH64-bit platform)
           at org.apache.bookkeeper.bookie.storage.ldb.ConversionTest.test(ConversionTest.java:118)
   `
   
   So latest rocksdb should be used.Current “Integration test” check failure does not seems specific to rocksdb or netty version updates.
   


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



[GitHub] [bookkeeper] odidev commented on pull request #2379: Updated netty,netty-boringssl and rocksdb

Posted by GitBox <gi...@apache.org>.
odidev commented on pull request #2379:
URL: https://github.com/apache/bookkeeper/pull/2379#issuecomment-665177186


   Run the tests locally and found that ‘integration test’ failure was due to maven-surefire plugin compatibility with rocksdb version 5.18.4 onwards.


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



[GitHub] [bookkeeper] odidev commented on pull request #2379: Updated netty,netty-boringssl and rocksdb

Posted by GitBox <gi...@apache.org>.
odidev commented on pull request #2379:
URL: https://github.com/apache/bookkeeper/pull/2379#issuecomment-669136303


   Hi @eolivelli ,
   Please have a look at 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] odidev commented on pull request #2379: Updated netty,netty-boringssl and rocksdb

Posted by GitBox <gi...@apache.org>.
odidev commented on pull request #2379:
URL: https://github.com/apache/bookkeeper/pull/2379#issuecomment-661250593


   > Btw my concern is about a possible case in which you upgrade to BK 4.12 and you hit a problem and you have to rollback to 4.11.
   > In this case will RocksDB work?
   
   In this case, Rocksdb should work on AMD, but not on AArch64 platform because 4.11 release is having older version of rocksdb. 


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



[GitHub] [bookkeeper] sijie commented on pull request #2379: Updated netty,netty-boringssl and rocksdb

Posted by GitBox <gi...@apache.org>.
sijie commented on pull request #2379:
URL: https://github.com/apache/bookkeeper/pull/2379#issuecomment-673346147


   @eolivelli can you review it again?


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



[GitHub] [bookkeeper] eolivelli commented on a change in pull request #2379: Updated netty,netty-boringssl and rocksdb

Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #2379:
URL: https://github.com/apache/bookkeeper/pull/2379#discussion_r465674181



##########
File path: pom.xml
##########
@@ -191,7 +191,7 @@
     <maven-javadoc-plugin.version>3.1.1</maven-javadoc-plugin.version>
     <maven-shade-plugin.version>3.2.0</maven-shade-plugin.version>
     <maven-source-plugin.version>2.2.1</maven-source-plugin.version>
-    <maven-surefire-plugin.version>2.21.0</maven-surefire-plugin.version>
+    <maven-surefire-plugin.version>3.0.0-M5</maven-surefire-plugin.version>

Review comment:
       Please use 3.0.0-M4
   Unfortunately M5 has some known problems about interactive development (basically it doesn't flush the output often  enough)  
   
   Can you also point which problem did you see with Surefire?  And how it is related to RocksDB?




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



[GitHub] [bookkeeper] odidev commented on a change in pull request #2379: Updated netty,netty-boringssl and rocksdb

Posted by GitBox <gi...@apache.org>.
odidev commented on a change in pull request #2379:
URL: https://github.com/apache/bookkeeper/pull/2379#discussion_r465834302



##########
File path: pom.xml
##########
@@ -191,7 +191,7 @@
     <maven-javadoc-plugin.version>3.1.1</maven-javadoc-plugin.version>
     <maven-shade-plugin.version>3.2.0</maven-shade-plugin.version>
     <maven-source-plugin.version>2.2.1</maven-source-plugin.version>
-    <maven-surefire-plugin.version>2.21.0</maven-surefire-plugin.version>
+    <maven-surefire-plugin.version>3.0.0-M5</maven-surefire-plugin.version>

Review comment:
       Hi @eolivelli ,
   Using 3.0.0-M4, 'Integration Tests' checks are failing again. It seems that it is not compatible. 
   Refer: https://github.com/apache/bookkeeper/pull/2379/checks?check_run_id=949575926
   Please share your opinion.
   




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



[GitHub] [bookkeeper] sijie merged pull request #2379: Updated netty,netty-boringssl and rocksdb

Posted by GitBox <gi...@apache.org>.
sijie merged pull request #2379:
URL: https://github.com/apache/bookkeeper/pull/2379


   


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



[GitHub] [bookkeeper] sijie merged pull request #2379: Updated netty,netty-boringssl and rocksdb

Posted by GitBox <gi...@apache.org>.
sijie merged pull request #2379:
URL: https://github.com/apache/bookkeeper/pull/2379






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



[GitHub] [bookkeeper] sijie merged pull request #2379: Updated netty,netty-boringssl and rocksdb

Posted by GitBox <gi...@apache.org>.
sijie merged pull request #2379:
URL: https://github.com/apache/bookkeeper/pull/2379






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



[GitHub] [bookkeeper] odidev commented on pull request #2379: Updated netty,netty-boringssl and rocksdb

Posted by GitBox <gi...@apache.org>.
odidev commented on pull request #2379:
URL: https://github.com/apache/bookkeeper/pull/2379#issuecomment-661877468


   @eolivelli 
   Please share your opinion on the "integration test" check failure which seems failing due to timeout.


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



[GitHub] [bookkeeper] eolivelli commented on pull request #2379: Updated netty,netty-boringssl and rocksdb

Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #2379:
URL: https://github.com/apache/bookkeeper/pull/2379#issuecomment-661247920


   Btw my concern is about a possible case in which you upgrade to BK 4.12 and you hit a problem and you have to rollback to 4.11.
   In this case will RocksDB work?


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



[GitHub] [bookkeeper] odidev commented on pull request #2379: Updated netty,netty-boringssl and rocksdb

Posted by GitBox <gi...@apache.org>.
odidev commented on pull request #2379:
URL: https://github.com/apache/bookkeeper/pull/2379#issuecomment-673006747


   Hi @eolivelli ,
   As per review suggestion, we tried to use surefire "3.0.0-M4", but that doesn't seem compatible with these optimized netty versions, and got integration test failures. So we had to use version "3.0.0-M5" again.


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



[GitHub] [bookkeeper] eolivelli commented on pull request #2379: Updated netty,netty-boringssl and rocksdb

Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #2379:
URL: https://github.com/apache/bookkeeper/pull/2379#issuecomment-661247278


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



[GitHub] [bookkeeper] eolivelli commented on pull request #2379: Updated netty,netty-boringssl and rocksdb

Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #2379:
URL: https://github.com/apache/bookkeeper/pull/2379#issuecomment-662862759


   @odidev I have restarted the job.
   If the job fails again you should run the tests locally and perform debug.
   
   You can split this patch in updating Netty + Updating RocksDB, this way it will be clearer which is the culprit


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



[GitHub] [bookkeeper] eolivelli commented on pull request #2379: Updated netty,netty-boringssl and rocksdb

Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #2379:
URL: https://github.com/apache/bookkeeper/pull/2379#issuecomment-664160185


   @odidev I am sorry but on github actions we do not have much log about the error.
   I suggest you to split the patch into:
   - upgrade Netty
   - upgrade RocksDB
   
   I am sure that Netty will run smoothly because we are using that version in production


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