Skip to content

HDDS-5208. bump rocksdb version to 6.20.3#2235

Merged
adoroszlai merged 4 commits intoapache:masterfrom
JacksonYao287:HDDS-5208
May 14, 2021
Merged

HDDS-5208. bump rocksdb version to 6.20.3#2235
adoroszlai merged 4 commits intoapache:masterfrom
JacksonYao287:HDDS-5208

Conversation

@JacksonYao287
Copy link
Contributor

What changes were proposed in this pull request?

now , the rocksdb version used in ozone is 6.8.1, which is build on Apr 16 , 2020.
this PR bumps rockdsb to 6.20.3, which is build on May 5, 2021, and includes many new bug-fix

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-5208

How was this patch tested?

no need to add additional tests

@JacksonYao287
Copy link
Contributor Author

@elek please take a look

@JacksonYao287
Copy link
Contributor Author

@elek hi, in my local test , integration(ozone) test is passed, but it fail here. is it a flaky case?

@adoroszlai
Copy link
Contributor

in my local test , integration(ozone) test is passed, but it fail here. is it a flaky case?

TestOzoneManagerRocksDBLogging has not been flaky recently, plus it looks related. I guess the problem could be that output may be changed in the new version.

(BTW, please trigger CI with empty commit instead of closing+reopening the PR. This makes it easier to see previous CI results.)

@JacksonYao287
Copy link
Contributor Author

TestOzoneManagerRocksDBLogging has not been flaky recently, plus it looks related. I guess the problem could be that output may be changed in the new version.

ok, i will fix this

(BTW, please trigger CI with empty commit instead of closing+reopening the PR. This makes it easier to see previous CI results.)

thanks, will take care of this in the future

@JacksonYao287
Copy link
Contributor Author

@adoroszlai i make the wait time longer ,and CI is passed. it seems we should wait longer for the log if we use the rocksdb of this version.

Copy link
Member

@elek elek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks the patch @JacksonYao287

Retriggered the build to be sure that the (possible) intermittency is disappeared. If it passes, I am +1 to merge it.

@adoroszlai
Copy link
Contributor

i make the wait time longer ,and CI is passed. it seems we should wait longer for the log if we use the rocksdb of this version.

Thanks for trying that.

The test expects the output db_impl.cc, which first appears when DUMPING STATS:

... [Thread-302] INFO  rocksdb.RocksDB (DBStoreBuilder.java:log(304)) - [/db_impl/db_impl.cc:932] ------- DUMPING STATS -------

Now with the new version there is a 15-sec delay. @avijayanhwx Would it make sense to check for rocksdb.RocksDB instead? I think it would appear in all lines coming from RocksDB:

public static final Logger ROCKS_DB_LOGGER =
LoggerFactory.getLogger(RocksDB.class);

@avijayanhwx
Copy link
Contributor

i make the wait time longer ,and CI is passed. it seems we should wait longer for the log if we use the rocksdb of this version.

Thanks for trying that.

The test expects the output db_impl.cc, which first appears when DUMPING STATS:

... [Thread-302] INFO  rocksdb.RocksDB (DBStoreBuilder.java:log(304)) - [/db_impl/db_impl.cc:932] ------- DUMPING STATS -------

Now with the new version there is a 15-sec delay. @avijayanhwx Would it make sense to check for rocksdb.RocksDB instead? I think it would appear in all lines coming from RocksDB:

public static final Logger ROCKS_DB_LOGGER =
LoggerFactory.getLogger(RocksDB.class);

Yes, I am ok with that.

@JacksonYao287
Copy link
Contributor Author

JacksonYao287 commented May 14, 2021

Retriggered the build to be sure that the (possible) intermittency is disappeared. If it passes, I am +1 to merge it.

thanks @elek for the retrigger。

@JacksonYao287
Copy link
Contributor Author

JacksonYao287 commented May 14, 2021

Now with the new version there is a 15-sec delay. @avijayanhwx Would it make sense to check for rocksdb.RocksDB instead? I think it would appear in all lines coming from RocksDB:

@adoroszlai , in this test case , it seems that several rocksdb benchmarks are carried out and then the corresponding benchmark stats are dumped. so I think maybe in this version of rocksdb , more benchmarks are involved , and thus it takes longer time to complete

@adoroszlai adoroszlai merged commit eff4914 into apache:master May 14, 2021
@adoroszlai
Copy link
Contributor

Thanks @JacksonYao287 for the patch, @elek for the review. We can address the delay separately.

@JacksonYao287 JacksonYao287 deleted the HDDS-5208 branch July 20, 2021 11:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants