Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
storage: err on snapshot of large ranges
Browse files Browse the repository at this point in the history
If a range needs to be split, return an err rather than attempting to generate a snapshot.
This avoids generating excessively large snapshots.

Suggested in cockroachdb#7581.
dt committed Jul 12, 2016
1 parent 2f82193 commit 5c30cf0
Showing 3 changed files with 59 additions and 16 deletions.
7 changes: 6 additions & 1 deletion storage/replica.go
Original file line number Diff line number Diff line change
@@ -2568,9 +2568,14 @@ func (r *Replica) loadSystemConfigSpan() ([]roachpb.KeyValue, []byte, error) {
// to be split.
func (r *Replica) needsSplitBySize() bool {
r.mu.Lock()
res := r.needsSplitBySizeLocked()
r.mu.Unlock()
return res
}

func (r *Replica) needsSplitBySizeLocked() bool {
maxBytes := r.mu.maxBytes
size := r.mu.state.Stats.Total()
r.mu.Unlock()
return maxBytes > 0 && size > maxBytes
}

4 changes: 4 additions & 0 deletions storage/replica_raftstorage.go
Original file line number Diff line number Diff line change
@@ -257,6 +257,10 @@ func (r *Replica) GetFirstIndex() (uint64, error) {
func (r *Replica) Snapshot() (raftpb.Snapshot, error) {
rangeID := r.RangeID

if r.needsSplitBySizeLocked() {
return raftpb.Snapshot{}, raft.ErrSnapshotTemporarilyUnavailable
}

// If a snapshot is in progress, see if it's ready.
if r.mu.snapshotChan != nil {
select {
64 changes: 49 additions & 15 deletions storage/replica_raftstorage_test.go
Original file line number Diff line number Diff line change
@@ -14,7 +14,7 @@
//
// Author: Tamir Duberstein (tamird@gmail.com)

package storage_test
package storage

import (
"math/rand"
@@ -24,39 +24,73 @@ import (
"github.com/cockroachdb/cockroach/internal/client"
"github.com/cockroachdb/cockroach/keys"
"github.com/cockroachdb/cockroach/roachpb"
"github.com/cockroachdb/cockroach/util/leaktest"
"github.com/cockroachdb/cockroach/util/randutil"
"github.com/cockroachdb/cockroach/util/tracing"
)

const keySize = 1 << 7 // 128 B
const valSize = 1 << 10 // 1 KiB

func fillTestRange(t testing.TB, rep *Replica, rangeID roachpb.RangeID, snapSize int) {
src := rand.New(rand.NewSource(0))
for i := 0; i < snapSize/(keySize+valSize); i++ {
key := keys.MakeRowSentinelKey(randutil.RandBytes(src, keySize))
val := randutil.RandBytes(src, valSize)
pArgs := putArgs(key, val)
if _, pErr := client.SendWrappedWith(rep, nil, roachpb.Header{
RangeID: rangeID,
}, &pArgs); pErr != nil {
t.Fatal(pErr)
}
}
}

func TestSkipLargeReplicaSnapshot(t *testing.T) {
defer leaktest.AfterTest(t)()
store, _, stopper := createTestStore(t)
store.DisableSplitQueue(true)
// We want to manually control the size of the raft log.
defer stopper.Stop()

const rangeID = 1
const snapSize = 1 << 25 // 32 MiB

rep, err := store.GetReplica(rangeID)
if err != nil {
t.Fatal(err)
}

fillTestRange(t, rep, rangeID, snapSize)

if _, err := rep.Snapshot(); err != nil {
t.Fatal(err)
}

fillTestRange(t, rep, rangeID, snapSize*2)

if _, err := rep.Snapshot(); err == nil {
t.Fatal("snapshot of a very large range should fail")
}
}

func BenchmarkReplicaSnapshot(b *testing.B) {
defer tracing.Disable()()
defer config.TestingDisableTableSplits()()
store, stopper, _ := createTestStore(b)
store, _, stopper := createTestStore(b)
// We want to manually control the size of the raft log.
store.DisableRaftLogQueue(true)
defer stopper.Stop()

const rangeID = 1
const keySize = 1 << 7 // 128 B
const valSize = 1 << 10 // 1 KiB
const snapSize = 1 << 25 // 32 MiB

rep, err := store.GetReplica(rangeID)
if err != nil {
b.Fatal(err)
}

src := rand.New(rand.NewSource(0))
for i := 0; i < snapSize/(keySize+valSize); i++ {
key := keys.MakeRowSentinelKey(randutil.RandBytes(src, keySize))
val := randutil.RandBytes(src, valSize)
pArgs := putArgs(key, val)
if _, pErr := client.SendWrappedWith(rep, nil, roachpb.Header{
RangeID: rangeID,
}, &pArgs); pErr != nil {
b.Fatal(pErr)
}
}
fillTestRange(b, rep, rangeID, snapSize)

b.ResetTimer()
for i := 0; i < b.N; i++ {

0 comments on commit 5c30cf0

Please sign in to comment.