From 0bf41dd8cf6563510f4b90148f04e8aa47939a04 Mon Sep 17 00:00:00 2001 From: Tobias Schottdorf Date: Fri, 19 Oct 2018 13:21:32 +0200 Subject: [PATCH 1/2] storage: remove spurious call to maybeInlineSideloadedRaftCommand 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 --- pkg/storage/replica.go | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/pkg/storage/replica.go b/pkg/storage/replica.go index 26dbf493fc92..5d2e6150fbc7 100644 --- a/pkg/storage/replica.go +++ b/pkg/storage/replica.go @@ -4323,16 +4323,9 @@ func (r *Replica) handleRaftReadyRaftMuLocked( for _, e := range rd.CommittedEntries { switch e.Type { case raftpb.EntryNormal: - // Committed entries come straight from the Raft log. Consequently, - // sideloaded SSTables are not usually inlined. - if newEnt, err := maybeInlineSideloadedRaftCommand( - ctx, r.RangeID, e, r.raftMu.sideloaded, r.store.raftEntryCache, - ); err != nil { - const expl = "maybeInlineSideloadedRaftCommand" - return stats, expl, errors.Wrap(err, expl) - } else if newEnt != nil { - e = *newEnt - } + // NB: Committed entries are handed to us by Raft. Raft does not + // know about sideloading. Consequently the entries here are all + // already inlined. var commandID storagebase.CmdIDKey var command storagepb.RaftCommand From 5500cb54ae73ce1973258a3b154554f4e454f061 Mon Sep 17 00:00:00 2001 From: Tobias Schottdorf Date: Fri, 19 Oct 2018 13:24:15 +0200 Subject: [PATCH 2/2] storage: check that sideloaded storage is present 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: https://github.com/cockroachdb/cockroach/issues/31618#issuecomment-431305161. Release note: None --- pkg/storage/replica_raftstorage.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/storage/replica_raftstorage.go b/pkg/storage/replica_raftstorage.go index d667cf74fe39..2ee0e3e55dba 100644 --- a/pkg/storage/replica_raftstorage.go +++ b/pkg/storage/replica_raftstorage.go @@ -84,6 +84,9 @@ func (r *replicaRaftStorage) Entries(lo, hi, maxBytes uint64) ([]raftpb.Entry, e readonly := r.store.Engine().NewReadOnly() defer readonly.Close() ctx := r.AnnotateCtx(context.TODO()) + if r.raftMu.sideloaded == nil { + return nil, errors.New("sideloaded storage is uninitialized") + } return entries(ctx, r.mu.stateLoader, readonly, r.RangeID, r.store.raftEntryCache, r.raftMu.sideloaded, lo, hi, maxBytes) }