Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Kaizen] Upgrade rocksdbjni version #928

Merged
merged 1 commit into from
Feb 19, 2021
Merged

Conversation

dzajkowski
Copy link
Contributor

Description

Bump rocksdb version

Copy link

@pullrequest pullrequest bot left a comment

Choose a reason for hiding this comment

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

✅ This pull request was sent to the PullRequest network.


@dzajkowski you can click here to see the review status or cancel the code review job - or - cancel by adding [!pr] to the title of the pull request.

Copy link

@pullrequest pullrequest bot left a comment

Choose a reason for hiding this comment

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

This looks like a reasonable upgrade. I just had the one comment pointed out that it can be bumped up a bit further.

Image of Eric E Eric E


Reviewed with ❤️ by PullRequest

@@ -48,7 +48,7 @@ object Dependencies {

val rocksDb = Seq(
// use "5.18.3" for older macOS
"org.rocksdb" % "rocksdbjni" % "6.11.4"
"org.rocksdb" % "rocksdbjni" % "6.15.2"
Copy link

Choose a reason for hiding this comment

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

Note that the version looks to be up to 6.15.5 without breaking changes: https://github.com/facebook/rocksdb/releases/tag/v6.15.5.

  • For Java builds, fix errors due to missing compression library includes.
  • Fix a race condition between DB startups and shutdowns in managing the periodic background worker threads. One effect of this race condition could be the process being terminated.
  • Since 6.15.0, TransactionDB returns error Statuses from calls to DeleteRange() and calls to Write() where the WriteBatch contains a range deletion. Previously such operations may have succeeded while not providing the expected transactional guarantees. There are certain cases where range deletion can still be used on such DBs; see the API doc on TransactionDB::DeleteRange() for details.
    OptimisticTransactionDB now returns error Statuses from calls to DeleteRange() and calls to Write() where the WriteBatch contains a range deletion. Previously such operations may have succeeded while not providing the expected transactional guarantees.

Image of Eric E Eric E

Copy link

@pullrequest pullrequest bot left a comment

Choose a reason for hiding this comment

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

This version bump looks good to me 👍 Looks like Eric also provided a more recent version that might be beneficial for bugs and performance.

Image of Brian Brian


Reviewed with ❤️ by PullRequest

@dzajkowski dzajkowski force-pushed the feature/upgrade-rocksdb branch from 08db3fe to 08043d8 Compare February 17, 2021 07:30
Copy link

@pullrequest pullrequest bot left a comment

Choose a reason for hiding this comment

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

⚠️ Warning

PullRequest detected a force-push on this branch. This may have caused some information to be lost, and additional time may be required to complete review of the code. Read More

@dzajkowski dzajkowski force-pushed the feature/upgrade-rocksdb branch 2 times, most recently from 5fd0106 to fa3c65b Compare February 17, 2021 09:56
Copy link
Contributor

@1015bit 1015bit left a comment

Choose a reason for hiding this comment

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

👍

@dzajkowski dzajkowski force-pushed the feature/upgrade-rocksdb branch from fa3c65b to ffc8cf4 Compare February 19, 2021 11:37
@dzajkowski dzajkowski merged commit 382d616 into develop Feb 19, 2021
@dzajkowski dzajkowski deleted the feature/upgrade-rocksdb branch April 9, 2021 12:01
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.

3 participants