Skip to content

Commit

Permalink
Merge cockroachdb#31828
Browse files Browse the repository at this point in the history 
31828: storage: remove older migrations r=spencerkimball a=spencerkimball

Both of these migrations are safe to remove for v2.2.

Release note: None

Co-authored-by: Spencer Kimball <spencer.kimball@gmail.com>
  • Loading branch information
@spencerkimball
craig[bot] and spencerkimball committed Oct 25, 2018
2 parents ae756ca + d23a6d1 commit fa7fb35
Show file tree
Hide file tree
Showing 6 changed files with 16 additions and 312 deletions.
6 changes: 3 additions & 3 deletions pkg/keys/constants.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,10 @@ var (
// localStoreSuggestedCompactionSuffix stores suggested compactions to
// be aggregated and processed on the store.
localStoreSuggestedCompactionSuffix = []byte("comp")
// localRemovedLeakedRaftEntriesSuffix marks that a store has completed
// its migration to remove all possibly-leaked Raft entries.
// TODO(nvanbenschoten): This can be removed in 2.2.

// localRemovedLeakedRaftEntriesSuffix is DEPRECATED and remains to prevent reuse.
localRemovedLeakedRaftEntriesSuffix = []byte("dlre")
_ = localRemovedLeakedRaftEntriesSuffix

// LocalStoreSuggestedCompactionsMin is the start of the span of
// possible suggested compaction keys for a store.
Expand Down
7 changes: 0 additions & 7 deletions pkg/keys/keys.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,13 +115,6 @@ func DecodeStoreSuggestedCompactionKey(key roachpb.Key) (start, end roachpb.Key,
return start, end, nil
}

// StoreRemovedLeakedRaftEntriesKey returns a store-local key that marks
// when a store has completed its migration to remove all possibly-leaked
// Raft entries on all replicas.
func StoreRemovedLeakedRaftEntriesKey() roachpb.Key {
return MakeStoreKey(localRemovedLeakedRaftEntriesSuffix, nil)
}

// NodeLivenessKey returns the key for the node liveness record.
func NodeLivenessKey(nodeID roachpb.NodeID) roachpb.Key {
key := make(roachpb.Key, 0, len(NodeLivenessPrefix)+9)
Expand Down
137 changes: 0 additions & 137 deletions pkg/storage/migrations.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,140 +13,3 @@
// permissions and limitations under the License.

package storage

import (
"context"
"time"

"github.com/cockroachdb/cockroach/pkg/keys"
"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/storage/engine"
"github.com/cockroachdb/cockroach/pkg/util/hlc"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
)

// clearLegacyTombstone removes the legacy tombstone for the given rangeID.
func clearLegacyTombstone(eng engine.Writer, rangeID roachpb.RangeID) error {
return eng.Clear(engine.MakeMVCCMetadataKey(keys.RaftTombstoneIncorrectLegacyKey(rangeID)))
}

// migrateLegacyTombstones rewrites all legacy tombstones into the correct key.
// It can be removed in binaries post v2.1.
func migrateLegacyTombstones(ctx context.Context, eng engine.Engine) error {
var tombstone roachpb.RaftTombstone
handleTombstone := func(rangeID roachpb.RangeID) (more bool, _ error) {
batch := eng.NewBatch()
defer batch.Close()

tombstoneKey := keys.RaftTombstoneKey(rangeID)

{
// If there's already a new-style tombstone, pick the larger NextReplicaID
// (this will be the new-style tombstone's).
var exTombstone roachpb.RaftTombstone
ok, err := engine.MVCCGetProto(ctx, batch, tombstoneKey,
hlc.Timestamp{}, true, nil, &exTombstone)
if err != nil {
return false, err
}
if ok && exTombstone.NextReplicaID > tombstone.NextReplicaID {
tombstone.NextReplicaID = exTombstone.NextReplicaID
}
}

if err := engine.MVCCPutProto(ctx, batch, nil, tombstoneKey,
hlc.Timestamp{}, nil, &tombstone); err != nil {
return false, err
}
if err := clearLegacyTombstone(batch, rangeID); err != nil {
return false, err
}
// Specify sync==false because we don't want to sync individually,
// but see the end of the surrounding method where we sync explicitly.
err := batch.Commit(false /* sync */)
return err == nil, err
}
err := IterateIDPrefixKeys(ctx, eng, keys.RaftTombstoneIncorrectLegacyKey, &tombstone,
handleTombstone)
if err != nil {
return err
}

// Write a final bogus batch so that we get to do a sync commit, which
// implicitly also syncs everything written before.
batch := eng.NewBatch()
defer batch.Close()

if err := clearLegacyTombstone(batch, 1 /* rangeID */); err != nil {
return err
}
return batch.Commit(true /* sync */)
}

// removeLeakedRaftEntries iterates over all replicas and ensures that all
// Raft entries that are beneath a replica's truncated index are removed.
// Earlier versions of Cockroach permitted a race where a replica's truncated
// index could be moved forward without the corresponding Raft entries being
// deleted atomically. This introduced a window in which an untimely crash
// could abandon Raft entries until the next log truncation.
// TODO(nvanbenschoten): It can be removed in binaries post v2.1.
func removeLeakedRaftEntries(
ctx context.Context, clock *hlc.Clock, eng engine.Engine, v *storeReplicaVisitor,
) error {
// Check if migration has already been performed.
marker := keys.StoreRemovedLeakedRaftEntriesKey()
found, err := engine.MVCCGetProto(ctx, eng, marker, hlc.Timestamp{}, false, nil, nil)
if found || err != nil {
return err
}

// Iterate over replicas and clear out any leaked raft entries. Visit
// them in increasing rangeID order so all accesses to the engine are in
// increasing order, which experimentally speeds this up by about 35%.
tBegin := timeutil.Now()
leaked := 0
v.InOrder().Visit(func(r *Replica) bool {
var ts roachpb.RaftTruncatedState
ts, err = r.raftTruncatedState(ctx)
if err != nil {
return false
}

// If any Raft entries were leaked then it must be true that the last
// entry that was truncated was also leaked. We use this to create a
// fast-path to rule out replicas that have not leaked any entries.
last := keys.RaftLogKey(r.RangeID, ts.Index)
found, err = engine.MVCCGetProto(ctx, eng, last, hlc.Timestamp{}, false, nil, nil)
if !found || err != nil {
return err == nil
}
leaked++

// Start at index zero and clear entries up through the truncated index.
start := engine.MakeMVCCMetadataKey(keys.RaftLogKey(r.RangeID, 0))
end := engine.MakeMVCCMetadataKey(last.PrefixEnd())

iter := eng.NewIterator(engine.IterOptions{UpperBound: end.Key})
defer iter.Close()

err = eng.ClearIterRange(iter, start, end)
return err == nil
})
if err != nil {
return err
}

f := log.Eventf
dur := timeutil.Since(tBegin)
if leaked > 0 || dur > 2*time.Second {
f = log.Infof
}
f(ctx, "found %d replicas with abandoned raft entries in %s", leaked, dur)

// Set the migration marker so that we can avoid checking for leaked entries
// again in the future. It doesn't matter what we actually use as the value,
// so just use the current time.
now := clock.Now()
return engine.MVCCPutProto(ctx, eng, nil, marker, hlc.Timestamp{}, nil, &now)
}
22 changes: 0 additions & 22 deletions pkg/storage/replica_raftstorage.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -332,15 +332,6 @@ func (r *Replica) raftTruncatedStateLocked(
return ts, nil
}

// raftTruncatedState returns metadata about the log that preceded the first
// current entry. This includes both entries that have been compacted away and
// the dummy entries that make up the starting point of an empty log.
func (r *Replica) raftTruncatedState(ctx context.Context) (roachpb.RaftTruncatedState, error) {
r.mu.Lock()
defer r.mu.Unlock()
return r.raftTruncatedStateLocked(ctx)
}

// FirstIndex implements the raft.Storage interface.
func (r *replicaRaftStorage) FirstIndex() (uint64, error) {
ctx := r.AnnotateCtx(context.TODO())
Expand Down Expand Up @@ -858,19 +849,6 @@ func (r *Replica) applySnapshot(
}
}

// Nodes running v2.0 and earlier may send an incorrect Raft tombstone (see
// #12154) that was supposed to be unreplicated. Simply remove it.
//
// NB: this can be removed post v2.1. This is because when we are running a
// binary at v2.2, we know that peers are at least running v2.1, which will
// never send out snapshots with incorrect tombstones. v2.0 nodes can send out
// these incorrect snapshots if they were upgraded from a v1.1 store with
// incorrect tombstones and never rebooted while
// VersionUnreplicatedTombstoneKey was active.
if err := clearLegacyTombstone(batch, r.RangeID); err != nil {
return errors.Wrap(err, "while clearing legacy tombstone key")
}

// The log entries are all written to distinct keys so we can use a
// distinct batch.
distinctBatch := batch.Distinct()
Expand Down
33 changes: 0 additions & 33 deletions pkg/storage/store.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1247,30 +1247,6 @@ func (s *Store) Start(ctx context.Context, stopper *stop.Stopper) error {
now := s.cfg.Clock.Now()
s.startedAt = now.WallTime

// Migrate legacy tombstones away. This is safe to do unconditionally: this is
// a post-v2.0 binary, so we're guaranteed that every node in the cluster is
// running a version that understands the non-legacy tombstones (v2.0 or
// later).
//
// We want to run this migration the first time the node boots with this
// binary version so that we can assume local data never contains legacy range
// tombstones. For simplicity, we do it on *every* boot. Should this be found
// to impact startup times too much, we can make it only run the first time
// this binary version is booted.
{
tBegin := timeutil.Now()
if err := migrateLegacyTombstones(ctx, s.engine); err != nil {
return errors.Wrapf(err, "migrating legacy tombstones for %v", s.engine)
}
f := log.Eventf

dur := timeutil.Since(tBegin)
if dur > 10*time.Second {
f = log.Infof
}
f(ctx, "ran legacy tombstone migration in %s", dur)
}

// Iterate over all range descriptors, ignoring uncommitted versions
// (consistent=false). Uncommitted intents which have been abandoned
// due to a split crashing halfway will simply be resolved on the
Expand Down Expand Up @@ -1330,15 +1306,6 @@ func (s *Store) Start(ctx context.Context, stopper *stop.Stopper) error {
return err
}

// Ensure that no Raft entries were abandoned by previous versions of Cockroach
// that did not delete Raft entries atomically with applying log truncation
// Raft commands. This will only be performed once, after which this call will
// see a migration marker and quickly no-op.
err = removeLeakedRaftEntries(ctx, s.Clock(), s.engine, newStoreReplicaVisitor(s))
if err != nil {
return errors.Wrapf(err, "checking for leaked raft entries for %v", s.engine)
}

// Start Raft processing goroutines.
s.cfg.Transport.Listen(s.StoreID(), s)
s.processRaft(ctx)
Expand Down
Loading

0 comments on commit fa7fb35

Please sign in to comment.