Skip to content
This repository has been archived by the owner on Aug 3, 2022. It is now read-only.

Port database backend to rocksdb to support warm-starting the server #24

Merged
merged 4 commits into from
Nov 18, 2016
Merged

Port database backend to rocksdb to support warm-starting the server #24

merged 4 commits into from
Nov 18, 2016

Conversation

kriswehner
Copy link

This is preparation for #6 which would let us get a consistent copy of the database using the rocksdb checkpoint mechanism.

This also brings in the dependency on gorocksdb.

Kris Wehner added 4 commits November 13, 2016 13:29
- Drop the printlns I was using during debugging
- Don't destroy the options structure for the DB with it open. This
is a sure route to seg faults.
- Fix the length check on the readOneRange() call so it works correctly
@gbrail
Copy link
Contributor

gbrail commented Nov 15, 2016

I like the direction of this and it makes sense to me. But I'll need some time while I travel this week to look at it carefully and adapt things like the Docker build to it.

@gbrail
Copy link
Contributor

gbrail commented Nov 15, 2016

While I love the idea of an "embedded build" for rocksdb support, in practice it seems to be a pain in the rear. But I want to make this a repeatable build, including Docker, before actually merging this.

We have two choices:

  1. We can rely on the embedded rocksdb packages from CockroachDB, and require that people pass the "embed" tag on every build.

  2. We can rely on having rocksdb on the local box, which is easier for us all once it's install, but it means that getting it in production is harder because RocksDB has to be there first. This isn't a huge issue for us because we typically run in Docker but it could complicate production deployments for others, since RocksDB isn't there in every package system and typically has to be rebuilt from source. Plus, it doesn't run on Alpine Linux, at least not a few months ago.

I'm leaning toward the second, with something in the build process to warn people when they have the wrong version.

@kriswehner
Copy link
Author

We'd favor the second. We've already got a simple docker build going for it, as we always run in docker as well.

@gbrail gbrail merged commit cbb2936 into apigee-labs:master Nov 18, 2016
@gbrail
Copy link
Contributor

gbrail commented Nov 18, 2016

Dealing with all the Docker stuff took a while and it was a busy week. But it looks good now and despite the more complex build it cleaned up a lot of code. Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants