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

storage/engine: provide Pebble implementation of Engine interface #39674

Closed
petermattis opened this issue Aug 14, 2019 · 1 comment
Closed

storage/engine: provide Pebble implementation of Engine interface #39674

petermattis opened this issue Aug 14, 2019 · 1 comment
Assignees
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)

Comments

@petermattis
Copy link
Collaborator

"Using Pebble" means providing a storage/engine.Engine implementation which uses Pebble. The intention is for this implementation to sit side by side with the RocksDB implementation and allow a flag to control which is used. Implementing the Engine interface also implies implementing the Batch and Iterator interfaces. It is possible RocksDB-isms have crept into these interfaces and they will need to be generalized. There are also a handful of direct uses of the engine.RocksDB implementation outside of the storage/engine package which will need to be cleaned up (e.g. cli/debug). git grep -w 'engine.RocksDB' for all of the uses.

Note that some bits of the MVCC layer will need to be ported from C++ back to Go. Specifically, Iterator.MVCC{Scan,Get} and the timeseries merge operator.

@petermattis petermattis added the C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) label Aug 14, 2019
itsbilal added a commit to itsbilal/cockroach that referenced this issue Sep 26, 2019
This change implements the interfaces defined in
`pkg/storage/engine/engine.go` for Pebble, in particular `Engine`,
`Iterator`, and related ones. Also ports the MVCC Scanner to Go
for use in `iter.MVCC{Get,Scan}`.

Other related changes elsewhere in the codebase to let an incomplete
Engine implementer (i.e. without WithSSTables, and without a compactor)
run otherwise.

Also adds a parameter in the `--store` command line flag to specify
what engine to use. `--store=engine=pebble,path=node1` starts a Pebble
based store.

Fixes cockroachdb#39674.

TODO:
- [ ] Refactor tests to run for both RocksDB and Pebble Engines, instead
of one-or-the-other
- [ ] Add more test coverage, especially around batches
- [ ] Function signature comments
- [ ] Comments comments comments

Release note: None
itsbilal added a commit to itsbilal/cockroach that referenced this issue Sep 30, 2019
This change implements the interfaces defined in
`pkg/storage/engine/engine.go` for Pebble, in particular `Engine`,
`Iterator`, and related ones. Also ports the MVCC Scanner to Go
for use in `iter.MVCC{Get,Scan}`.

Other related changes elsewhere in the codebase to let an incomplete
Engine implementer (i.e. without WithSSTables, and without a compactor)
run otherwise.

Also adds a parameter in the `--store` command line flag to specify
what engine to use. `--store=engine=pebble,path=node1` starts a Pebble
based store.

NOTE: High risk change, do not merge this for 19.2.

Fixes cockroachdb#39674.

TODO:
- [ ] Add more test coverage, especially around batches

Release note: None
itsbilal added a commit to itsbilal/cockroach that referenced this issue Oct 1, 2019
This change implements the interfaces defined in
`pkg/storage/engine/engine.go` for Pebble, in particular `Engine`,
`Iterator`, and related ones. Also ports the MVCC Scanner to Go
for use in `iter.MVCC{Get,Scan}`.

Other related changes elsewhere in the codebase to let an incomplete
Engine implementer (i.e. without WithSSTables, and without a compactor)
run otherwise.

Also adds a parameter in the `--store` command line flag to specify
what engine to use. `--store=engine=pebble,path=node1` starts a Pebble
based store.

NOTE: High risk change, do not merge this for 19.2.

Fixes cockroachdb#39674.

TODO:
- [ ] Add more test coverage, especially around batches

Release note: None
itsbilal added a commit to itsbilal/cockroach that referenced this issue Oct 2, 2019
This change implements the interfaces defined in
`pkg/storage/engine/engine.go` for Pebble, in particular `Engine`,
`Iterator`, and related ones. Also ports the MVCC Scanner to Go
for use in `iter.MVCC{Get,Scan}`.

Other related changes elsewhere in the codebase to let an incomplete
Engine implementer (i.e. without WithSSTables, and without a compactor)
run otherwise.

Also adds a parameter in the `--store` command line flag to specify
what engine to use. `--store=engine=pebble,path=node1` starts a Pebble
based store.

NOTE: High risk change, do not merge this for 19.2.

Fixes cockroachdb#39674.

TODO:
- [ ] Add more test coverage, especially around batches

Release note: None
itsbilal added a commit to itsbilal/cockroach that referenced this issue Oct 3, 2019
This change implements the interfaces defined in
`pkg/storage/engine/engine.go` for Pebble, in particular `Engine`,
`Iterator`, and related ones. Also ports the MVCC Scanner to Go
for use in `iter.MVCC{Get,Scan}`.

Other related changes elsewhere in the codebase to let an incomplete
Engine implementer (i.e. without WithSSTables, and without a compactor)
run otherwise.

Also adds a parameter in the `--store` command line flag to specify
what engine to use. `--store=engine=pebble,path=node1` starts a Pebble
based store.

NOTE: High risk change, do not merge this for 19.2.

Fixes cockroachdb#39674.

TODO:
- [ ] Add more test coverage, especially around batches

Release note: None
itsbilal added a commit to itsbilal/cockroach that referenced this issue Oct 8, 2019
This change implements the interfaces defined in
`pkg/storage/engine/engine.go` for Pebble, in particular `Engine`,
`Iterator`, and related ones.

The MVCC scanner is implemented in a separate commit, so `MVCCGet`
and `MVCCScan` are currently stubs that do nothing.

Other related changes elsewhere in the codebase to let an incomplete
Engine implementer (i.e. without WithSSTables, and without a compactor)
run otherwise.

One part of the work for cockroachdb#39674.

TODO:
- [ ] Add more test coverage, especially around batches

Release note: None
itsbilal added a commit to itsbilal/cockroach that referenced this issue Oct 8, 2019
This change ports the MVCC Scanner from libroach to go, for use
with Pebble's iterators. One half of the work involved in cockroachdb#39674.
Also updates pebbleIterator to call it in pebbleIterator.MVCC*.

Release note: None
itsbilal added a commit to itsbilal/cockroach that referenced this issue Oct 10, 2019
This change implements the interfaces defined in
`pkg/storage/engine/engine.go` for Pebble, in particular `Engine`,
`Iterator`, and related ones.

The MVCC scanner is implemented in a separate commit, so `MVCCGet`
and `MVCCScan` are currently stubs that do nothing.

Other related changes elsewhere in the codebase to let an incomplete
Engine implementer (i.e. without WithSSTables, and without a compactor)
run otherwise.

One part of the work for cockroachdb#39674.

TODO:
- [ ] Add more test coverage, especially around batches

Release note: None
itsbilal added a commit to itsbilal/cockroach that referenced this issue Oct 11, 2019
This change implements the interfaces defined in
`pkg/storage/engine/engine.go` for Pebble, in particular `Engine`,
`Iterator`, and related ones.

The MVCC scanner is implemented in a separate commit, so `MVCCGet`
and `MVCCScan` are currently stubs that do nothing.

Other related changes elsewhere in the codebase to let an incomplete
Engine implementer (i.e. without WithSSTables, and without a compactor)
run otherwise.

One part of the work for cockroachdb#39674.

TODO:
- [ ] Add more test coverage, especially around batches

Release note: None
itsbilal added a commit to itsbilal/cockroach that referenced this issue Oct 11, 2019
This change implements the interfaces defined in
`pkg/storage/engine/engine.go` for Pebble, in particular `Engine`,
`Iterator`, and related ones.

The MVCC scanner is implemented in a separate commit, so `MVCCGet`
and `MVCCScan` are currently stubs that do nothing.

Other related changes elsewhere in the codebase to let an incomplete
Engine implementer (i.e. without WithSSTables, and without a compactor)
run otherwise.

One part of the work for cockroachdb#39674.

TODO:
- [ ] Add more test coverage, especially around batches

Release note: None
craig bot pushed a commit that referenced this issue Oct 11, 2019
41199: engine: Implement Engine, Iterator interfaces for Pebble r=itsbilal a=itsbilal

This change implements the interfaces defined in
`pkg/storage/engine/engine.go` for Pebble, in particular `Engine`,
`Iterator`, and related ones.

The MVCC scanner is implemented in a separate commit, so `MVCCGet`
and `MVCCScan` are currently stubs that do nothing.

Other related changes elsewhere in the codebase to let an incomplete
Engine implementer (i.e. without WithSSTables, and without a compactor)
run otherwise.

One part of the work for #39674.

TODO:
- [ ] Add more test coverage, especially around batches

Release note: None

Co-authored-by: Bilal Akhtar <[email protected]>
itsbilal added a commit to itsbilal/cockroach that referenced this issue Oct 14, 2019
This change ports the MVCC Scanner from libroach to go, for use
with Pebble's iterators. One half of the work involved in cockroachdb#39674.
Also updates pebbleIterator to call it in pebbleIterator.MVCC*.

Release note: None
itsbilal added a commit to itsbilal/cockroach that referenced this issue Oct 14, 2019
This change ports the MVCC Scanner from libroach to go, for use
with Pebble's iterators. One half of the work involved in cockroachdb#39674.
Also updates pebbleIterator to call it in pebbleIterator.MVCC*.

Release note: None
itsbilal added a commit to itsbilal/cockroach that referenced this issue Oct 15, 2019
This change ports the MVCC Scanner from libroach to go, for use
with Pebble's iterators. One half of the work involved in cockroachdb#39674.
Also updates pebbleIterator to call it in pebbleIterator.MVCC*.

Release note: None
craig bot pushed a commit that referenced this issue Oct 16, 2019
41452: engine: Port MVCCScanner to Go, implement MVCCScan for pebble r=itsbilal a=itsbilal

This change ports the MVCC Scanner from libroach to go, for use
with Pebble's iterators. One half of the work involved in #39674.
Also updates pebbleIterator to call it in pebbleIterator.MVCC*.

First commit is #41199 , which should merge soon.

Release note: None

41453: cli: Add flag to choose pebble as storage engine  r=itsbilal a=itsbilal

This change adds a --storage-engine CLI flag, which has options
`rocksdb` and `pebble`. Specifying `pebble` ensures all stores created
on this node, including the temp engine, use pebble instead of rocksdb.

This is a per-node flag instead of a per-store flag to eliminate cases
where we have to maintain two caches, one for rocksdb instances and
one for pebble instances.

Also remove --temp-engine flag, which is now unnecessary.

Release note (cli change): Add command-line argument --storage-engine
to specify type of storage engine to use. Options are rocksdb (default)
and pebble.

Co-authored-by: Bilal Akhtar <[email protected]>
@petermattis
Copy link
Collaborator Author

The primary work here is done, and what remains has been split out into separate issues such as #41739, #41744, and #41598.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants