sql: remove manually split ranges during TRUNCATE and DROP #38221
Conversation
|
This change is |
jeffrey-xiao
force-pushed
the
truncate-remove-splits
branch
2 times, most recently
from
a3b1614
to
4615820
Compare
There was a problem hiding this comment.
Reviewed 4 of 4 files at r1.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @nvanbenschoten)
There was a problem hiding this comment.
Reviewed 2 of 4 files at r1.
Reviewable status:
pkg/cmd/roachtest/clearrange.go, line 55 at r1 (raw file):
// 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
You tested that this roachtest passes with this change, right?
pkg/sql/drop_index.go, line 245 at r1 (raw file):
if (desc.StickyBit != hlc.Timestamp{}) && span.Key.Compare(desc.StartKey.AsRawKey()) <= 0 { if err := p.ExecCfg().DB.AdminUnsplit(ctx, desc.StartKey); err != nil { return err
Do we want to swallow "key is not the start of a range" errors? That would mean that the sticky bit was removed and the range was merged concurrently with all of this. It's not likely, but also not something we should fail the DROP INDEX statement for.
Same question below.
pkg/sql/logictest/testdata/logic_test/crdb_internal, line 490 at r1 (raw file):
# Testing split_enforced_until when truncating and dropping.
Nice tests.
pkg/sql/logictest/testdata/logic_test/crdb_internal, line 495 at r1 (raw file):
statement ok ALTER INDEX foo@idx SPLIT AT VALUES (1), (2), (3)
Let's query crdb_internal.ranges WHERE split_enforced_until IS NOT NULL before each of these drops to show that something is happening.
jeffrey-xiao
force-pushed
the
truncate-remove-splits
branch
3 times, most recently
from
4d37bf8
to
f30c754
Compare
There was a problem hiding this comment.
Reviewable status:
pkg/cmd/roachtest/clearrange.go, line 55 at r1 (raw file):
Previously, nvanbenschoten (Nathan VanBenschoten) wrote…
You tested that this roachtest passes with this change, right?
I've tested that clearrange/checks=false passes with this change.
pkg/sql/drop_index.go, line 245 at r1 (raw file):
Previously, nvanbenschoten (Nathan VanBenschoten) wrote…
Do we want to swallow "key is not the start of a range" errors? That would mean that the sticky bit was removed and the range was merged concurrently with all of this. It's not likely, but also not something we should fail the
DROP INDEXstatement for.Same question below.
Makes sense. Done.
There was a problem hiding this comment.
Reviewable status:
|
TFTRs! bors r+ |
Build failed |
|
bors r+ |
Build failed |
|
bors r+ |
Build failed |
jeffrey-xiao
force-pushed
the
truncate-remove-splits
branch
from
f30c754
to
ddf688b
Compare
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 it was waiting for manual splits to be automatically merged, which is not possible. Release note: None
jeffrey-xiao
force-pushed
the
truncate-remove-splits
branch
from
ddf688b
to
7375d59
Compare
|
bors r+ |
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>
Build succeeded |
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