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

Provide our own C interface to rocksdb. #128

Merged
merged 3 commits into from
Oct 17, 2014
Merged

Conversation

petermattis
Copy link
Collaborator

Stop using the rocksdb C interface and instead provide our own C
interface to rocksdb. This makes it cleaner to implement various bits of
cockroach-specific functionality in C++ such as the compaction filter
logic and merge operator.

@bdarnell
Copy link
Contributor

LGTM

Stop using the rocksdb C interface and instead provide our own C
interface to rocksdb. This makes it cleaner to implement various bits of
cockroach-specific functionality in C++ such as the compaction filter
logic and merge operator.

// Applies a batch of operations (puts, merges and deletes) to the
// database atomically.
DBStatus DBWrite(DBEngine* db, DBBatch *batch);
Copy link
Member

Choose a reason for hiding this comment

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

I've started to regret making the DBBatch a different sort of object. I think we should revert to the Viewfinder model, where a batch is just another type of engine (as is snapshot) and give DBEngine a Flush() method which is normally a noop but executes a write batch in the event the engine is really a DBBatch.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Heh, guess I didn't beat that horse long enough. Yeah, happy to switch over to the viewfinder model in a subsequent change.

Do you think DBSnapshot should be a type of DBEngine as well?

Copy link
Member

Choose a reason for hiding this comment

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

Yes. Same as with Viewfinder. It'll simplify the go code as well--so annoying to have to call SnapshotGet(), etc. And having to create a batch object and then commit it when you don't really need the batch functionality is a pain.

@spencerkimball
Copy link
Member

LGTM

petermattis added a commit that referenced this pull request Oct 17, 2014
Provide our own C interface to rocksdb.
@petermattis petermattis merged commit c829bc5 into master Oct 17, 2014
@petermattis petermattis deleted the pmattis/roachlib branch October 17, 2014 18:38
soniabhishek added a commit to soniabhishek/cockroach that referenced this pull request Feb 15, 2017
pav-kv pushed a commit to pav-kv/cockroach that referenced this pull request Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants