db: remove code related to atomic units

[RaduBerinde]

Note: the first commit is #3152.

Fixes #1913.

db: don't allow table overlaps in compaction tests

With all formats that tolerate split keys deprecated, we no longer
have to deal with tables overlapping in terms of user keys.

This commit adds a check to disallow any key overlap between tables in
compaction tests and removes offending test cases.

db: remove code related to atomic units

The concept of "atomic units" is related to expanding the boundary of
a compaction so that it does not cross a split user key; this can now
be removed.

[cockroach-teamcity]

This change is 

[sumeerbhola]

Thanks for doing this!

Reviewed 4 of 4 files at r5, 6 of 6 files at r6, all commit messages.
Reviewable status: 9 of 88 files reviewed, 5 unresolved discussions (waiting on @RaduBerinde)

---

level_checker.go line 280 at r6 (raw file):

//
// We do this check as follows:
// - For each level that can have untruncated tombstones, truncate the tombstones.

I think the tombstones should be already truncated when written by compactions and flushes, so we should not try to truncate before checking.

---

level_checker.go line 392 at r6 (raw file):

			}
			truncate := func(t keyspan.Span) keyspan.Span {
				//// Same checks as in keyspan.Truncate.

why this commented out code?

---

testdata/compaction_expand_inputs line 66 at r4 (raw file):

define
a.SET.1-b.RANGEDEL.72057594037927935

this is the only RANGEDEL case in this file and this is not considered overlapping wrt user key so we should retain this part of it.

---

testdata/compaction_setup_inputs line 125 at r4 (raw file):

L2
  a.SET.3-c.SET.4
  c.SET.3-d.SET.2 compacting

can you alter this test to not have overlapping user keys but continue to have the already compacting file sandwiched between two files that are going to be picked by the setup-inputs interval.

---

testdata/compaction_setup_inputs_multilevel_write_amp line 284 at r4 (raw file):

setup-inputs avail-bytes=20 a a
L1
  a.SET.1-b.SET.2 size=4

If we have
L1
a-c
L2
a-d
d-e
L3
a-b
c-g

I think the d-e file in L2 will not get picked if the maxSize limit is exceeded.

[RaduBerinde]

testdata/compaction_setup_inputs line 125 at r4 (raw file):

Previously, sumeerbhola wrote…

can you alter this test to not have overlapping user keys but continue to have the already compacting file sandwiched between two files that are going to be picked by the setup-inputs interval.

Changing to

setup-inputs a d
L1
  a.SET.5-f.SET.6
L2
  a.SET.3-c.SET.4
  d.SET.3-e.SET.2 compacting
  f.SET.3-g.SET.6
----

Interestingly, the existing atomic units code did not deem it as is-compacting. It looks like it doesn't look at the files already in the range, only at the "expanding" edges.. Do you think this is a bug worth fixing in older versions?

[RaduBerinde]

TFTR!

Reviewable status: 0 of 88 files reviewed, 5 unresolved discussions (waiting on @sumeerbhola)

---

level_checker.go line 280 at r6 (raw file):

Previously, sumeerbhola wrote…

I think the tombstones should be already truncated when written by compactions and flushes, so we should not try to truncate before checking.

Done.

---

level_checker.go line 392 at r6 (raw file):

Previously, sumeerbhola wrote…

why this commented out code?

Ah, this was my draft way of disabling the tombstone truncation logic. Cleaned up.

---

testdata/compaction_expand_inputs line 66 at r4 (raw file):

Previously, sumeerbhola wrote…

this is the only RANGEDEL case in this file and this is not considered overlapping wrt user key so we should retain this part of it.

Done, I adjusted the last table since it was overlapping in d

---

testdata/compaction_setup_inputs_multilevel_write_amp line 284 at r4 (raw file):

Previously, sumeerbhola wrote…

If we have
L1
a-c
L2
a-d
d-e
L3
a-b
c-g

I think the d-e file in L2 will not get picked if the maxSize limit is exceeded.

Done.

[RaduBerinde]

This is RFAL. Thanks!

[sumeerbhola]

thanks for doing this!

Reviewed 2 of 88 files at r7, 8 of 86 files at r8, all commit messages.
Reviewable status: 10 of 88 files reviewed, 6 unresolved discussions (waiting on @RaduBerinde)

---

compaction_picker.go line 420 at r8 (raw file):

	// Determine the sstables in the output level which overlap with the input
	// sstables, and then expand those tables to a clean cut. No need to do

nit: "and then expand those tables to a clean cut" is now stale

[RaduBerinde]

TFTR!

Reviewable status: 10 of 88 files reviewed, 5 unresolved discussions (waiting on @sumeerbhola)