Skip to content

Revert "Upgrade Rocksdb to 6.29.4.1 (#14886)"#14962

Merged
lhotari merged 1 commit into
apache:masterfrom
lhotari:lh-revert-rocksdb-upgrade
Mar 31, 2022
Merged

Revert "Upgrade Rocksdb to 6.29.4.1 (#14886)"#14962
lhotari merged 1 commit into
apache:masterfrom
lhotari:lh-revert-rocksdb-upgrade

Conversation

@lhotari

@lhotari lhotari commented Mar 31, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

Fixes #14961
Fixes #14936

Motivation

BookKeeper 4.14.x isn't compatible with Rocksdb 6.29.4.1

2022-03-31T02:28:35,315+0000 [BookieHighPriorityThread-3181-OrderedExecutor-4-0] ERROR org.apache.bookkeeper.proto.ReadEntryProcessor - Unexpected exception reading at 20:-1 : 'org.rocksdb.ReadOptions org.rocksdb.ReadOptions.setIterateUpperBound(org.rocksdb.Slice)'
java.lang.NoSuchMethodError: 'org.rocksdb.ReadOptions org.rocksdb.ReadOptions.setIterateUpperBound(org.rocksdb.Slice)'
	at org.apache.bookkeeper.bookie.storage.ldb.KeyValueStorageRocksDB.getFloor(KeyValueStorageRocksDB.java:257) ~[org.apache.bookkeeper-bookkeeper-server-4.14.4.jar:4.14.4]
	at org.apache.bookkeeper.bookie.storage.ldb.EntryLocationIndex.getLastEntryInLedgerInternal(EntryLocationIndex.java:106) ~[org.apache.bookkeeper-bookkeeper-server-4.14.4.jar:4.14.4]
	at org.apache.bookkeeper.bookie.storage.ldb.EntryLocationIndex.getLastEntryInLedger(EntryLocationIndex.java:99) ~[org.apache.bookkeeper-bookkeeper-server-4.14.4.jar:4.14.4]
	at org.apache.bookkeeper.bookie.storage.ldb.SingleDirectoryDbLedgerStorage.getLastEntry(SingleDirectoryDbLedgerStorage.java:559) ~[org.apache.bookkeeper-bookkeeper-server-4.14.4.jar:4.14.4]
	at org.apache.bookkeeper.bookie.storage.ldb.SingleDirectoryDbLedgerStorage.getEntry(SingleDirectoryDbLedgerStorage.java:401) ~[org.apache.bookkeeper-bookkeeper-server-4.14.4.jar:4.14.4]
	at org.apache.bookkeeper.bookie.storage.ldb.DbLedgerStorage.getEntry(DbLedgerStorage.java:200) ~[org.apache.bookkeeper-bookkeeper-server-4.14.4.jar:4.14.4]
	at org.apache.bookkeeper.bookie.LedgerDescriptorImpl.readEntry(LedgerDescriptorImpl.java:160) ~[org.apache.bookkeeper-bookkeeper-server-4.14.4.jar:4.14.4]
	at org.apache.bookkeeper.bookie.Bookie.readEntry(Bookie.java:1500) ~[org.apache.bookkeeper-bookkeeper-server-4.14.4.jar:4.14.4]
	at org.apache.bookkeeper.proto.ReadEntryProcessor.processPacket(ReadEntryProcessor.java:83) [org.apache.bookkeeper-bookkeeper-server-4.14.4.jar:4.14.4]
	at org.apache.bookkeeper.proto.PacketProcessorBase.safeRun(PacketProcessorBase.java:106) [org.apache.bookkeeper-bookkeeper-server-4.14.4.jar:4.14.4]
	at org.apache.bookkeeper.common.util.SafeRunnable.run(SafeRunnable.java:36) [org.apache.bookkeeper-bookkeeper-common-4.14.4.jar:4.14.4]
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128) [?:?]
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628) [?:?]
	at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30) [io.netty-netty-common-4.1.74.Final.jar:4.1.74.Final]
	at java.lang.Thread.run(Thread.java:829) [?:?]

Modifications

This reverts commit 12faf2d.

@lhotari lhotari added type/bug The PR fixed a bug or issue reported a bug component/bookkeeper doc-not-needed Your PR changes do not impact docs labels Mar 31, 2022
@lhotari lhotari mentioned this pull request Mar 31, 2022
Closed
@lhotari

lhotari commented Mar 31, 2022

Copy link
Copy Markdown
Member Author

the problem was also referenced in #12166 (comment)

eolivelli
eolivelli approved these changes Mar 31, 2022
View reviewed changes

@eolivelli eolivelli left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

we should do the change on BK and wait for a release

@lhotari lhotari mentioned this pull request Mar 31, 2022
Closed
@lhotari

lhotari commented Mar 31, 2022 •
edited
Loading

Copy link
Copy Markdown
Member Author

In Java, it's possible to break binary compabitility ("ABI") without breaking API source compatibility.

here's the location where RocksDB ABI broke: facebook/rocksdb@1001bc0#r69967694
This commit was included in RocksDB 6.17.3. Code compiled with RocksDB <6.17.3 isn't compatible with a newer RocksDB version at runtime.

We need a Bookkeeper release that has been compiled with the new version before we can upgrade RocksDB.

@lhotari lhotari mentioned this pull request Mar 31, 2022
Closed
@lhotari
lhotari merged commit 02eb31b into apache:master Mar 31, 2022
@lhotari

lhotari commented Apr 1, 2022

Copy link
Copy Markdown
Member Author

"There is no guarantee of Binary Compatibility between RocksDB versions, I am afraid you will have to recompile."
facebook/rocksdb@1001bc0#r70067997

Nicklee007 pushed a commit to Nicklee007/pulsar that referenced this pull request Apr 20, 2022
@lhotari nicklixinyang
Revert "Upgrade Rocksdb to 6.29.4.1 (apache#14886)" (apache#14962)
27b1152 
Fixes apache#14961

This reverts commit 12faf2d.

Code compiled with RocksDB <6.17.3 isn't compatible with a newer RocksDB version at runtime.
This was referenced May 10, 2023
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@eolivelli eolivelli eolivelli approved these changes
@codelipenghui codelipenghui codelipenghui approved these changes
@nicoloboschi nicoloboschi nicoloboschi approved these changes
@mattisonchao mattisonchao mattisonchao approved these changes
@merlimat merlimat Awaiting requested review from merlimat
@dlg99 dlg99 Awaiting requested review from dlg99
@BewareMyPower BewareMyPower Awaiting requested review from BewareMyPower
@congbobo184 congbobo184 Awaiting requested review from congbobo184
@michaeljmarshall michaeljmarshall Awaiting requested review from michaeljmarshall

Assignees

@lhotari lhotari

Labels

doc-not-needed Your PR changes do not impact docs type/bug The PR fixed a bug or issue reported a bug

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

RocksDB upgrade to 6.29.4.1 breaks Bookies pulsar-io Sinks & Sources tests fail (flake?) with "Failed to open ledger"

5 participants

@lhotari @eolivelli @codelipenghui @nicoloboschi @mattisonchao