Skip to content

cli/cliflags: add COCKROACH_STORAGE_ENGINE env var#42185

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
petermattis:pmattis/storage-engine-envvar
Nov 5, 2019
Merged

cli/cliflags: add COCKROACH_STORAGE_ENGINE env var#42185
craig[bot] merged 1 commit intocockroachdb:masterfrom
petermattis:pmattis/storage-engine-envvar

Conversation

@petermattis
Copy link
Copy Markdown
Collaborator

roachprod passes through COCKROACH_* env vars allowing
COCKROACH_STORAGE_ENGINE=pebble roachprod start to work.

Fixes #41620

Release note (cli change): Add COCKROACH_STORAGE_ENGINE env var which
is tied to the --storage-engine flag. Allows selection of "pebble" as
an alternative to the default of "rocksdb".

`roachprod` passes through COCKROACH_* env vars allowing
`COCKROACH_STORAGE_ENGINE=pebble roachprod start` to work.

Fixes cockroachdb#41620

Release note (cli change): Add COCKROACH_STORAGE_ENGINE env var which
is tied to the `--storage-engine` flag. Allows selection of "pebble" as
an alternative to the default of "rocksdb".
@petermattis petermattis requested a review from a team as a code owner November 5, 2019 15:04
@petermattis
Copy link
Copy Markdown
Collaborator Author

I still need to check that COCKROACH_STORAGE_ENGINE=pebble roachtest run works. I think it should, just haven't verified that it does.

@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

@petermattis
Copy link
Copy Markdown
Collaborator Author

Yep, verified COCKROACH_STORAGE_ENGINE=pebble roachtest run -l 'kv95/enc=false/nodes=1$' uses Pebble instead of RocksDB.

Copy link
Copy Markdown
Contributor

@itsbilal itsbilal left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 1 of 1 files at r1.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @ajkr and @sumeerbhola)

@petermattis
Copy link
Copy Markdown
Collaborator Author

TFTR!

bors r+

craig bot pushed a commit that referenced this pull request Nov 5, 2019
42086: sql: remove hard limit handling from heuristic planner r=savoie a=savoie

Previously, the heuristic planner function `applyLimit` propagated and set both
hard and soft limits. Hard limit propagation is now handled in the optimizer by
using rules to push Limit operators down the tree as far as is possible, and to
remove Limits by adding hard limits to child Scan operators. As a result, a
majority of the logic within `applyLimit` that supported propagating hard
limits was never exercised, since optimization rules would have already
eliminated situations in which Limit operators high up in the tree could be the
cause of hard limits further down the tree. The one remaining case for which
`applyLimit` was still responsible for propagating hard limits was eliminated
by the normalization rule introduced in #41908.

The entirety of `applyLimit` will eventually be removed and replaced by the
optimizer. Removing hard limit propagation from it entirely is a step in this
direction, and will make it easier to refactor soft limit propagation later.

This patch changes `applyLimit` into `applySoftLimit`, which no longer performs
any hard limit propagation; the responsibility for setting any hard limits on
scanNodes and spoolNodes is now left to the optimizer.

It also renames `setUnlimited` to `propagateSoftLimits` in order to indicate
more clearly that this function is not used to remove any limits that might
have been set on a node, but instead to trigger soft limit propagation in case
a limit hint can be introduced further down the tree.

In the case where the optimizer does not generate a limited Scan in place of a
Limit around a Scan (because the Scan cannot be constrained), previously
`applyLimit` would still set the `hardLimit` field of the scan.  This change
will result in the `softLimit` being set instead.  The end state of the
planNode tree is otherwise the same as before this refactor.

Release note: None

42181: storage/engine: port the final few MVCC benchmarks to Pebble r=petermattis a=petermattis

The `MVCCDeleteRange`, `ClearRange`, and `ClearIterRange` benchmarks
appear to give nonsensical results. I suspect the benchmarks themselves
are at fault and need to be rewritten.

```
name                          old time/op    new time/op    delta
MVCCScanTransactionalData-16    4.20ms ± 1%    3.39ms ± 0%  -19.15%  (p=0.000 n=9+9)
```

Release note: None

42185: cli/cliflags: add COCKROACH_STORAGE_ENGINE env var r=petermattis a=petermattis

`roachprod` passes through COCKROACH_* env vars allowing
`COCKROACH_STORAGE_ENGINE=pebble roachprod start` to work.

Fixes #41620

Release note (cli change): Add COCKROACH_STORAGE_ENGINE env var which
is tied to the `--storage-engine` flag. Allows selection of "pebble" as
an alternative to the default of "rocksdb".

Co-authored-by: Céline O'Neil <celineloneil@gmail.com>
Co-authored-by: Peter Mattis <petermattis@gmail.com>
@craig
Copy link
Copy Markdown
Contributor

craig bot commented Nov 5, 2019

Build succeeded

@craig craig bot merged commit a6f7101 into cockroachdb:master Nov 5, 2019
@petermattis petermattis deleted the pmattis/storage-engine-envvar branch November 5, 2019 17:35
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.

roachtest: add flag (or env var) to request pebble

3 participants