Skip to content

Commit

Permalink
Merge #38221
Browse files Browse the repository at this point in the history 
38221: sql: remove manually split ranges during TRUNCATE and DROP r=jeffrey-xiao a=jeffrey-xiao

If a table/index is dropped/truncated, any manually split ranges in the table/index should be unsplit so the automatic merge queue can clean up after it.

Additionally, re-enable splits in clearrange which was previously timing out because manual splits could not be automatically merged.

Release note: None

Co-authored-by: Jeffrey Xiao <jeffrey.xiao1998@gmail.com>
  • Loading branch information
@jeffrey-xiao
craig[bot] and jeffrey-xiao committed Jun 25, 2019
2 parents c9ad321 + 7375d59 commit 7d3fdbf
Show file tree
Hide file tree
Showing 4 changed files with 139 additions and 6 deletions.
6 changes: 1 addition & 5 deletions pkg/cmd/roachtest/clearrange.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,8 @@ func runClearRange(ctx context.Context, t *test, c *cluster, aggressiveChecks bo

// NB: on a 10 node cluster, this should take well below 3h.
tBegin := timeutil.Now()
// Currently we must set `--ranges=0` since automatic merges cannot merge ranges created by
// manual splits, and dropping the table does not remove manual splits. The latter may change
// in the future in which case it'll be fine to set `--ranges` to something else. But for now,
// setting it to nonzero causes the test to hang forever waiting on ranges to be merged.
c.Run(ctx, c.Node(1), "./cockroach", "workload", "fixtures", "import", "bank",
"--payload-bytes=10240", "--ranges=0", "--rows=65104166", "--seed=4", "--db=bigbank")
"--payload-bytes=10240", "--ranges=10", "--rows=65104166", "--seed=4", "--db=bigbank")
c.l.Printf("import took %.2fs", timeutil.Since(tBegin).Seconds())
c.Stop(ctx)
t.Status()
Expand Down
28 changes: 28 additions & 0 deletions pkg/sql/drop_index.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,15 @@ package sql
import (
"context"
"fmt"
"strings"

"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
"github.com/cockroachdb/cockroach/pkg/sql/privilege"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/sqlbase"
"github.com/cockroachdb/cockroach/pkg/util/hlc"
"github.com/cockroachdb/errors"
)

Expand Down Expand Up @@ -222,6 +225,31 @@ func (p *planner) dropIndexByName(
found := false
for i, idxEntry := range tableDesc.Indexes {
if idxEntry.ID == idx.ID {
// Unsplit all manually split ranges in the index so they can be
// automatically merged by the merge queue.
span := tableDesc.IndexSpan(idxEntry.ID)
ranges, err := ScanMetaKVs(ctx, p.txn, span)
if err != nil {
return err
}
for _, r := range ranges {
var desc roachpb.RangeDescriptor
if err := r.ValueProto(&desc); err != nil {
return err
}
// We have to explicitly check that the range descriptor's start key
// lies within the span of the index since ScanMetaKVs returns all
// intersecting spans.
if (desc.StickyBit != hlc.Timestamp{}) && span.Key.Compare(desc.StartKey.AsRawKey()) <= 0 {
// Swallow "key is not the start of a range" errors because it would
// mean that the sticky bit was removed and merged concurrently. DROP
// INDEX should not fail because of this.
if err := p.ExecCfg().DB.AdminUnsplit(ctx, desc.StartKey); err != nil && strings.Contains(err.Error(), "is not the start of a range") {
return err
}
}
}

// the idx we picked up with FindIndexByID at the top may not
// contain the same field any more due to other schema changes
// intervening since the initial lookup. So we send the recent
Expand Down
24 changes: 24 additions & 0 deletions pkg/sql/drop_table.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,17 @@ package sql
import (
"context"
"fmt"
"strings"

"github.com/cockroachdb/cockroach/pkg/jobs"
"github.com/cockroachdb/cockroach/pkg/jobs/jobspb"
"github.com/cockroachdb/cockroach/pkg/keys"
"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/sql/privilege"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/sqlbase"
"github.com/cockroachdb/cockroach/pkg/util/errorutil/unimplemented"
"github.com/cockroachdb/cockroach/pkg/util/hlc"
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
"github.com/cockroachdb/errors"
)
Expand Down Expand Up @@ -350,6 +353,27 @@ func (p *planner) initiateDropTable(
tableDesc.DropTime = timeutil.Now().UnixNano()
}

// Unsplit all manually split ranges in the table so they can be
// automatically merged by the merge queue.
ranges, err := ScanMetaKVs(ctx, p.txn, tableDesc.TableSpan())
if err != nil {
return err
}
for _, r := range ranges {
var desc roachpb.RangeDescriptor
if err := r.ValueProto(&desc); err != nil {
return err
}
if (desc.StickyBit != hlc.Timestamp{}) {
// Swallow "key is not the start of a range" errors because it would mean
// that the sticky bit was removed and merged concurrently. DROP TABLE
// should not fail because of this.
if err := p.ExecCfg().DB.AdminUnsplit(ctx, desc.StartKey); err != nil && strings.Contains(err.Error(), "is not the start of a range") {
return err
}
}
}

tableDesc.State = sqlbase.TableDescriptor_DROP
if drainName {
// Queue up name for draining.
Expand Down
87 changes: 86 additions & 1 deletion pkg/sql/logictest/testdata/logic_test/crdb_internal
View file
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ node_id store_id attrs used
1 1 [] 0

statement ok
CREATE TABLE foo (a INT PRIMARY KEY); INSERT INTO foo VALUES(1)
CREATE TABLE foo (a INT PRIMARY KEY, INDEX idx(a)); INSERT INTO foo VALUES(1)

statement ok
ALTER TABLE foo SPLIT AT VALUES(2)
Expand Down Expand Up @@ -485,3 +485,88 @@ ORDER BY key, f
SELECT crdb_internal.force_retry(_) true false
SELECT crdb_internal.force_retry(_) true true
SET application_name = DEFAULT false false


# Testing split_enforced_until when truncating and dropping.
statement ok
ALTER TABLE foo SPLIT AT VALUES (1), (2), (3)

statement ok
ALTER INDEX foo@idx SPLIT AT VALUES (1), (2), (3)

query TT colnames
SELECT start_pretty, end_pretty FROM crdb_internal.ranges WHERE split_enforced_until IS NOT NULL
----
start_pretty end_pretty
/Table/56/1/1 /Table/56/1/2
/Table/56/1/2 /Table/56/1/3
/Table/56/1/3 /Table/56/2/1
/Table/56/2/1 /Table/56/2/2
/Table/56/2/2 /Table/56/2/3
/Table/56/2/3 /Max

statement ok
TRUNCATE TABLE foo

query TT colnames
SELECT start_pretty, end_pretty FROM crdb_internal.ranges WHERE split_enforced_until IS NOT NULL
----
start_pretty end_pretty

statement ok
ALTER TABLE foo SPLIT AT VALUES (1), (2), (3)

statement ok
ALTER INDEX foo@idx SPLIT AT VALUES (1), (2), (3)

query TT colnames
SELECT start_pretty, end_pretty FROM crdb_internal.ranges WHERE split_enforced_until IS NOT NULL
----
start_pretty end_pretty
/Table/57/1/1 /Table/57/1/2
/Table/57/1/2 /Table/57/1/3
/Table/57/1/3 /Table/57/2/1
/Table/57/2/1 /Table/57/2/2
/Table/57/2/2 /Table/57/2/3
/Table/57/2/3 /Max

statement ok
DROP TABLE foo

query TT colnames
SELECT start_pretty, end_pretty FROM crdb_internal.ranges WHERE split_enforced_until IS NOT NULL
----
start_pretty end_pretty

statement ok
CREATE TABLE foo (a INT PRIMARY KEY, INDEX idx(a)); INSERT INTO foo VALUES(1)

statement ok
ALTER TABLE foo SPLIT AT VALUES (1), (2), (3)

statement ok
ALTER INDEX foo@idx SPLIT AT VALUES (1), (2), (3)

query TT colnames
SELECT start_pretty, end_pretty FROM crdb_internal.ranges WHERE split_enforced_until IS NOT NULL
----
start_pretty end_pretty
/Table/58/1/1 /Table/58/1/2
/Table/58/1/2 /Table/58/1/3
/Table/58/1/3 /Table/58/2/1
/Table/58/2/1 /Table/58/2/2
/Table/58/2/2 /Table/58/2/3
/Table/58/2/3 /Max

statement ok
DROP INDEX foo@idx

# Verify only the start keys of the manually split ranges because the merge queue could merge the
# ranges [/Table/58/1/3, /Table/58/2/1) with its right neighbors.
query T colnames
SELECT start_pretty FROM crdb_internal.ranges WHERE split_enforced_until IS NOT NULL
----
start_pretty
/Table/58/1/1
/Table/58/1/2
/Table/58/1/3

0 comments on commit 7d3fdbf

Please sign in to comment.