change(state): Allow opening the database in a read-only mode

[upbqdn]

Motivation

Close #8078.

Depends-On: #8080

PR Author Checklist

Check before marking the PR as ready for review:

• Will the PR name make sense to users?
• Does the PR have a priority label?
• Have you added or updated tests?
• Is the documentation up to date?

Specifications

RocksDB's "read-only" and "secondary" modes: https://github.com/facebook/rocksdb/wiki/Read-only-and-Secondary-instances

Rust bindings:

• open_cf_descriptors_read_only

Solution

• Add a read-only option when opening the database.

Notes

• Writing functionality is still exposed even though the database can be opened in read-only mode, so I opened Make the writing API of read-only databases inaccessible #8098.
• The "secondary" mode seems like a better alternative because the "read-only" mode seems only to open a snapshot of the database, but I didn't manage to figure out what the secondary_path in open_cf_descriptors_as_secondary should be.

Testing

• I manually accessed the database using the zebra-scan crate in a separate binary while Zebra was running.

Review

Reviewer Checklist

Check before approving the PR:

• Does the PR scope match the ticket?
• Are there enough tests to make sure it works? Do the tests cover the PR motivation?
• Are all the PR blockers dealt with?
  PR blockers can be dealt with in new tickets or PRs.

And check the PR Author checklist is complete.

[teor2345]

• Writing functionality is still exposed even though the database can be opened in read-only mode.

We can open another ticket to clean that up. It's not really a problem because trying to write will fail. (And either panic or return an error, depending on how we handle RocksDB write errors.)

One possible solution is having a type that implements all the read methods, but only implements the two write methods: write(batch) and spawn_format_change() when it is a read-write database. This can be implemented using a generic parameter that allows writing.

For example, the second generic parameter here is required to be DBWithThreadModeInner, we could do something similar with a ReadWriteDatabase unit struct:
https://docs.rs/rocksdb/latest/rocksdb/struct.DBCommon.html#impl-DBCommon%3CT,+DBWithThreadModeInner%3E

And here are the methods that work regardless of the generic type (for us that would be read methods):
https://docs.rs/rocksdb/latest/rocksdb/struct.DBCommon.html#impl-DBCommon%3CT,+D%3E

• The "secondary" mode seems like a better alternative because the "read-only" mode seems only to open a snapshot of the database, but I didn't manage to figure out what the secondary_path in open_cf_descriptors_as_secondary should be.

I don't think the difference is that important, but a random temporary directory would avoid locking conflicts with multiple secondaries:
https://github.com/facebook/rocksdb/wiki/Read-only-and-Secondary-instances

When using ldb I've just been giving an empty string, which I assume uses the current directory, or a temporary directory, or no directory. It hasn't caused any errors or created any files yet.

[teor2345]

Thanks! This is some great work!

I'm not sure if it's out of scope for the MVP, happy to let the team make a call on that.

[upbqdn]

I'm putting this PR out of the draft since it looks like we can merge it.

[teor2345]

Thanks! I've scheduled this after the scanner tests merge, because they are blocking other work.

[teor2345]

I did a bit of a merge and cleanup, let's see how it goes

[upbqdn]

There was still an issue with path resolution that should be fixed now.

I also updated the description and labels. All should be good to go.

[teor2345]

Looks great! I've made this merge wait until after #8080, because that's blocking some final tests.

[upbqdn]

How did you do that?

[teor2345]

Using Depends-On: #8080 in the PR description:
https://docs.mergify.com/workflow/schedule-merge/#using-depends-on-to-manage-merge-dependencies

They also have a Merge-After: time field now.

[teor2345]

@Mergifyio refresh

[mergify]

refresh

✅ Pull request refreshed

[teor2345]

This only failed the merge queue due to #7898