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

Read only support for rocksdb #98

Merged
merged 5 commits into from
Apr 7, 2019
Merged

Conversation

eugeneware
Copy link
Contributor

This PR adds read only rocksdb support.

This PR closes #72 and quite a few comments on #13

There's a new readOnly boolean option to the database options object that can be used to open a rocksdb database in readonly mode.

I've also included some tests which use chmod to set the database to read only to set the various edge cases.

I've also published a version to @eugeneware/rocksdb in case this doesn't get merged for a while for people to use.

This was referenced Apr 5, 2019
@eugeneware
Copy link
Contributor Author

eugeneware commented Apr 5, 2019

OK. So looks like I'll need a windows machine to try to debug the appveyer messages.

@vweevers
Copy link
Member

vweevers commented Apr 5, 2019

It's probably due to:

Caveats: on Windows only the write permission can be changed, and the distinction among the permissions of group, owner or others is not implemented.
https://nodejs.org/api/fs.html#fs_fs_chmod_path_mode_callback

But I don't think we have to use chmod for the tests at all. Just test a db.get() and db.put() on a db with readOnly: true. No need to test other operations like del or batch, or other cases that are handled by RocksDB (i.e. not our code).

@eugeneware
Copy link
Contributor Author

@vweevers So, just test that it can still work on regular databases, don't worry about testing whether it can work with read-only databases? I can do that :-)

My own use case was using the database from a read-only file system, but you're right, the other major use case is just loading a database without mutating any state, which is probably what most people are trying to do.

@vweevers
Copy link
Member

vweevers commented Apr 5, 2019

Side note: because readOnly: true implies no lockfile can be written.. does that mean you can open a db from multiple processes? Cool! 😃

@eugeneware
Copy link
Contributor Author

@vweevers I believe so! I haven't tested this my self with a live writing process. But would definitely work for 2 read only processes.

@vweevers vweevers added this to In progress in Level (old board) via automation Apr 5, 2019
@vweevers vweevers added the semver-minor New features that are backward compatible label Apr 5, 2019
Copy link
Member

@vweevers vweevers left a comment

Choose a reason for hiding this comment

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

I still prefer not using chmod, feels out of scope, but we can postpone that discussion.

@vweevers vweevers requested a review from peakji April 5, 2019 16:36
@vweevers
Copy link
Member

vweevers commented Apr 7, 2019

We can simplify some of the tests when upgrading abstract-leveldown.

@vweevers vweevers merged commit eb8e90b into Level:master Apr 7, 2019
Level (old board) automation moved this from In progress to Done Apr 7, 2019
@vweevers vweevers mentioned this pull request Apr 7, 2019
@vweevers
Copy link
Member

3.1.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver-minor New features that are backward compatible
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Open rocksdb in ReadOnly Mode
2 participants