libroach: try to ingest SSTs without write-stalls #34800
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This change is |
|
I haven't tested this yet on a loaded roachprod cluster but I wanted to see what #34212 thinks of it. |
|
saw this |
|
whoops, yep, accidentally got the bool check backwards -- fixed. |
same issue. Looks like you cannot set the boolean to true |
|
ah ha I see you define another status var here. Modified locally and running |
|
okay this works . I do see another error: I'm pretty sure that's not related. Is there a concern with a race in which data enters the memtable between the flush and the second ingest? |
|
I ran it for another 10 minutes without hitting any problems. |
|
whoops, yeah, fixed the shadowed |
|
re:race. If a write makes it in that would require another flush, on the retry we do allow a write_stall so it should just go ahead and get it done that time. Hopefully a) that is unlikely to happen at all and b) if it does happen, since we just did a flush already, this one should be a very small, very quick, blip at worst. |
petermattis
reviewed
Feb 12, 2019
c-deps/libroach/db.cc
Outdated
|
|
||
| // It is possible we failed because the memtable wanted to flush but we did | ||
| // not allow a blocking flush on the first try. Do a manual, non-blocking | ||
| // flush and wait for it, then try again. |
There was a problem hiding this comment.
It is confusing to say that the flush is non-blocking and that we'll wait for it. Perhaps something like: Perform a manual flush which will not stall other writes.
This attempts to ingest without allowing a memtable flush that could cause write-stalls. If that fails, it then does its own, no-stall flush and waits for it before retrying the ingest. On the re-attempt, the ingest is allowed to do a blocking flush if it needs to, but the hope is that the explicit flush means it will not have to. Release note (perforrmance improvement): reduce impact of bulk data ingestion on foreground traffic with by controlling RocksDB flushes. Release note: None
|
bors r+ |
|
👎 Rejected by PR status |
|
manually setting cla status bors r+ |
Build failed (retrying...) |
Build failed (retrying...) |
craig bot
pushed a commit
that referenced
this pull request
Feb 25, 2019
34800: libroach: try to ingest SSTs without write-stalls r=dt a=dt This attempts to ingest without allowing a memtable flush that could cause write-stalls. If that fails, it then does its own, no-stall flush and waits for it before retrying the ingest. On the re-attempt, the ingest is allowed to do a blocking flush if it needs to, but the hope is that the explicit flush means it will not have to. Release note (perforrmance improvement): reduce impact of bulk data ingestion on foreground traffic with by controlling RocksDB flushes. Release note: None Co-authored-by: David Taylor <tinystatemachine@gmail.com>
Build succeeded |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This attempts to ingest without allowing a memtable flush that could
cause write-stalls. If that fails, it then does its own, no-stall flush
and waits for it before retrying the ingest. On the re-attempt, the
ingest is allowed to do a blocking flush if it needs to, but the hope is
that the explicit flush means it will not have to.
Release note (perforrmance improvement): reduce impact of bulk data ingestion on foreground traffic with by controlling RocksDB flushes.
Release note: None