-
Notifications
You must be signed in to change notification settings - Fork 557
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
Allow opening an existing DB in snapshot-only mode #13937
Conversation
Adds a new API call to `ZeebeDbFactory` which allows opening existing databases in a snapshot-only mode. This returns a `ZeebeDb` which can ONLY take snapshots, and will throw a `new UnsupportedOperationException` for any other operations. While it would be nice to have a proper read-only DB implementation, there are no transactions in a such a DB, and our whole access to the DB is coupled with the transaction API. As we were thinking of switching away from transactions, it will likely be easier after that to implement a read-only DB.
5affe90
to
7071cdb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @npepinpe 🚋 🚀
@@ -56,31 +57,50 @@ public ZeebeRocksDbFactory( | |||
|
|||
@Override | |||
public ZeebeTransactionDb<ColumnFamilyType> createDb(final File pathName) { | |||
final ZeebeTransactionDb<ColumnFamilyType> db; | |||
final List<AutoCloseable> closeables = Collections.synchronizedList(new ArrayList<>()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❓ Do you remember why this is actually a synchronized list 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No idea, I just copied it and assumed we have a good reason for it 😄 Possibly it's related to creating transaction/transaction contexts from possibly many threads? 🤷
zb-db/src/test/java/io/camunda/zeebe/db/impl/rocksdb/ZeebeRocksDbFactoryTest.java
Show resolved
Hide resolved
bors merge |
Build succeeded: |
This pull request requires a change in zeebe-process-test: @npepinpe Please have a look |
Description
This PR adds a new API call to
ZeebeDbFactory
which allows opening existing databases in a snapshot-only mode. This returns aZeebeDb
which can ONLY take snapshots, and will throw anew UnsupportedOperationException
for any other operations.While it would be nice to have a proper read-only DB implementation, there are no transactions in a such a DB, and our whole access to the DB is coupled with the transaction API. As we were thinking of switching away from transactions, it will likely be easier after that to implement a read-only DB.
This will be used as part of the fix for #13775 to create the runtime directory faster via hard-links instead of a full copy.
Related issues
related to #13775
Definition of Done
Not all items need to be done depending on the issue and the pull request.
Code changes:
backport stable/1.3
) to the PR, in case that fails you need to create backports manually.Testing:
Documentation:
Other teams:
If the change impacts another team an issue has been created for this team, explaining what they need to do to support this change.
Please refer to our review guidelines.