storage: remove spurious call to maybeInlineSideloadedRaftCommand #31627
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 |
benesch
approved these changes
Oct 19, 2018
|
This would be really awkward to get wrong, so @bdarnell and @nvanbenschoten to double check please. |
nvanbenschoten
approved these changes
Oct 19, 2018
There was a problem hiding this comment.
This brings up an interesting quetion - what happens to the raftEntryCache when we have sideloaded entries? Looks like we keep the entries inlined in the cache. That seems like a quick way to completely flush the rest of the cache, which is size-bound. I can't imagine that's good for any other Ranges that happen to live on the same Store as a Range in the middle of an AddSstRequest proposal.
Reviewed 1 of 1 files at r1, 1 of 1 files at r2.
Reviewable status:complete! 0 of 0 LGTMs obtained (and 1 stale)
|
Filed an issue for the entry cache: #31688 (hope GitHub didn't lose it) bors r=nvanbenschoten,benesch |
|
bors r=nvanbenschoten,benesch |
Entries are "thinned" only when passed to `r.append()` (i.e. written to disk) and they are always returned "fat" from `Entries()` (i.e. Raft's way to get entries from disk). Consequently Raft never sees thin entries and won't ask us to commit them. Touches cockroachdb#31618. Release note: None
If this were passed as nil, we'd be returning "thin" (i.e. with sideloading payloads not inlined) Entries. This isn't supposed to happen, but check it. See: cockroachdb#31618 (comment). Release note: None
tbg
force-pushed
the
fix/useless-inline
branch
from
e5df97a to
5500cb5
Compare
Canceled |
|
bors r=nvanbenschoten,benesch |
craig bot
pushed a commit
that referenced
this pull request
Oct 22, 2018
31556: importccl: re-enable job control tests r=mjibson a=mjibson I tracked down the problem. It was that after the CANCEL JOB was issued, sometimes the 2nd stage of the IMPORT (the shuffle) would have started, and sometimes it wouldn't have. If it did not start then RunJob would block forever trying to send on the allowResponse chan. Fix this by making a draining go routine after the first block. Closes #24623 Closes #24658 Release note: None 31627: storage: remove spurious call to maybeInlineSideloadedRaftCommand r=nvanbenschoten,benesch a=tschottdorf Entries are "thinned" only when passed to `r.append()` (i.e. written to disk) and they are always returned "fat" from `Entries()` (i.e. Raft's way to get entries from disk). Consequently Raft never sees thin entries and won't ask us to commit them. Touches #31618. Release note: None 31695: bitarray: don't allow FromEncodingParts to return invalid bit array r=knz a=benesch It is invalid for a bit array's lastBitsUsed field to be greater than 64. The FromEncodingParts function, however, would happily construct an invalid bitarray if given a too-large lastBitsUsed value. Teach the FromEncodingParts to return an error instead. This presented as a panic when attempting to pretty-print a key with a bitarray whose lastBitsUsed encoded value was 65. Such a key can be created when calling PrefixEnd on a key with a bitarray whose lastBitsUsed value is 64. By returning an error instead, the pretty-printing code will try again after calling UndoPrefixEnd and be able to print the key. Fix #31115. Release note: None 31697: partitionccl: deflake TestDropIndexWithZoneConfigCCL r=danhhz,eriktrinh a=benesch A particularly adversarial goroutine schedule can cause this test to observe the moment in time where the data is dropped but the zone config is not. Deflake by retrying the check for the dropped zone config until it succeeds (or times out). Fix #31678. Release note: None Co-authored-by: Matt Jibson <[email protected]> Co-authored-by: Tobias Schottdorf <[email protected]> Co-authored-by: Nikhil Benesch <[email protected]>
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.
Entries are "thinned" only when passed to
r.append()(i.e. written todisk) and they are always returned "fat" from
Entries()(i.e. Raft's wayto get entries from disk). Consequently Raft never sees thin entries and
won't ask us to commit them.
Touches #31618.
Release note: None