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

libroach: disable rocksdb subcompactions #35210

Merged
merged 1 commit into from
Feb 26, 2019

Conversation

ajkr
Copy link
Contributor

@ajkr ajkr commented Feb 26, 2019

The feature is not widely used so is not the most stable, especially
when combined with other uncommonly used features like range tombstones.
In my bin/workload kv --read-percent 0 experiments it is not necessary
since the frequent foreground fdatasyncs prevent compaction from
falling behind, with or without subcompactions enabled.

Release note: None

The feature is not widely used so is not the most stable, especially
when combined with other uncommonly used features like range tombstones.
In my `bin/workload kv --read-percent 0` experiments it is not necessary
since the frequent foreground `fdatasync`s prevent compaction from
falling behind, with or without subcompactions enabled.

Release note: None
@ajkr ajkr requested a review from a team February 26, 2019 17:24
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Collaborator

@petermattis petermattis left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained

@ajkr
Copy link
Contributor Author

ajkr commented Feb 26, 2019

bors r+

craig bot pushed a commit that referenced this pull request Feb 26, 2019
35157: sql: Add "or-if-not-exists" semantics to `CPut` and use it in index entry writes r=bdarnell a=dt

This switches index updates to use a new `CPut` behavior, to fix the
handling of valid update operations that are processed during the
backfill for indexes that have values -- such as UNIQUE indexes as well
as non-unique indexes that include STORING columns.

We need to use `CPut` for maintaining index entries (at least for UNIQUE
indexes) to avoid clobbering entries for other rows. During the backfill
we cannot assume an entry with the old value actually exists however.
Thus a normal `CPut` on them from the prior value to the new value also
asserts they exist, which isn't always true.

This issue is a little hard to hit: we only emit a `CPut` from the old
value to the new value when a) the key is the same and b) the values are
different. If any indexed values changed, we'd be deleting the old entry
and `CPut`ting a new one which we (correctly) expect not to exist at
all. Only when updating the same key to a new value -- i.e. when handing
an update to only a STORING column or some UNIQUE cases -- do we do this
in-place CPut from old to new and then only when we happen to be
operating on keys not yet reached by the backfiller do we incorrectly
assume it exists with value old.

Since we put many CPuts in the batch at once, we can't easily retry in
SQL with nil vs old values so we need new CPut semantics to fix this.

This adds a flag to `CPut` to optionally allow a put which expects a given existing 
value to succeed if instead that key does not exist.

This is useful in sitations where the client is trying to avoid overwriting an existing 
entry other than the one it thinks should exist, but also wants to allow for it not to exist.
Specifically, when writing secondary index entries, particularly for unique indexes, 
we use `CPut` to ensure that we're not overwriting the index entry for a different 
row -- if we are, we expect the `CPut` to fail.

Fixes #33260.

Release note (bug fix): Fix bug when that would return errors when
handling valid UPDATEs during periods of an index creation schema
change.


35210: libroach: disable rocksdb subcompactions r=ajkr a=ajkr

The feature is not widely used so is not the most stable, especially
when combined with other uncommonly used features like range tombstones.
In my `bin/workload kv --read-percent 0` experiments it is not necessary
since the frequent foreground `fdatasync`s prevent compaction from
falling behind, with or without subcompactions enabled.

Release note: None

35213: Makefile: don't build krb5 on musl r=mjibson a=mjibson

It produces a linking error. It may be fixable, but for now it's not
important, so disable it. In the future if requested by users we can
figure out if/how to fix the musl build.

Release note: None

Co-authored-by: David Taylor <tinystatemachine@gmail.com>
Co-authored-by: Andrew Kryczka <andrew.kryczka2@gmail.com>
Co-authored-by: Matt Jibson <matt.jibson@gmail.com>
@craig
Copy link
Contributor

craig bot commented Feb 26, 2019

Build succeeded

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.

None yet

3 participants