Skip to content
This repository has been archived by the owner on Dec 1, 2024. It is now read-only.

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 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
@vweevers vweevers mentioned this pull request Apr 7, 2019
@vweevers
Copy link
Member

3.1.0

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
semver-minor New features that are backward compatible
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Open rocksdb in ReadOnly Mode
2 participants