colblk: implement synthetic prefix and suffix

[RaduBerinde]

First commit is #3996

colblk: implement synthetic prefix and suffix

DataBlockIter now takes into account the synthetic prefix and suffix
transforms.

[cockroach-teamcity]

This change is 

[RaduBerinde]

Forgot to add some benchmarks and compare the no-transform case before/after, will do that soon.

[jbowens]

Reviewed 2 of 9 files at r2, all commit messages.
Reviewable status: 2 of 9 files reviewed, 2 unresolved discussions (waiting on @RaduBerinde)

---

sstable/colblk/data_block.go line 947 at r2 (raw file):

		!i.transforms.HideObsoletePoints &&
		!i.transforms.SyntheticPrefix.IsSet() &&
		!i.transforms.SyntheticSuffix.IsSet()

should we put this on a AnySet() Transforms method or the like?

---

sstable/colblk/data_block.go line 977 at r2 (raw file):

}

// seekGEInternal has is a wrapper around keySeeker.SeekGE which takes into

nit: s/has is/is/

[RaduBerinde]

TFTR!

Reviewable status: 2 of 9 files reviewed, 1 unresolved discussion (waiting on @jbowens)

---

sstable/colblk/data_block.go line 947 at r2 (raw file):

Previously, jbowens (Jackson Owens) wrote…

should we put this on a AnySet() Transforms method or the like?

Good point, done.

[RaduBerinde]

Updated benchmark as well.
Short benchmark before/after:

name                                                                                old time/op    new time/op    delta
CockroachDataBlockIterShort/AlphaLen=8,Prefix=8,Shared=4,ValueLen=8/Next-10           12.2ns ± 0%    12.6ns ± 0%  +3.05%  (p=0.001 n=7+7)
CockroachDataBlockIterShort/AlphaLen=8,Prefix=8,Shared=4,ValueLen=8/SeekGE-10          114ns ± 0%     114ns ± 1%    ~     (p=0.454 n=8+8)
CockroachDataBlockIterShort/AlphaLen=8,Prefix=128,Shared=64,ValueLen=128/Next-10      10.5ns ± 0%    10.5ns ± 0%  -0.28%  (p=0.024 n=8+8)
CockroachDataBlockIterShort/AlphaLen=8,Prefix=128,Shared=64,ValueLen=128/SeekGE-10    91.0ns ± 0%    91.1ns ± 1%    ~     (p=0.555 n=8+8)

Transform benchmark shows very little difference, which is pretty cool!

name                                                                                                     time/op
CockroachDataBlockIterTransforms/AlphaLen=8,Prefix=8,Shared=4,ValueLen=8/Next-10                         12.6ns ± 1%
CockroachDataBlockIterTransforms/AlphaLen=8,Prefix=8,Shared=4,ValueLen=8/SeekGE-10                        114ns ± 1%
CockroachDataBlockIterTransforms/AlphaLen=8,Prefix=8,Shared=4,ValueLen=8,SynthSeqNum/Next-10             12.8ns ± 0%
CockroachDataBlockIterTransforms/AlphaLen=8,Prefix=8,Shared=4,ValueLen=8,SynthSeqNum/SeekGE-10            114ns ± 0%
CockroachDataBlockIterTransforms/AlphaLen=8,Prefix=8,Shared=4,ValueLen=8,HideObsolete/Next-10            13.6ns ± 0%
CockroachDataBlockIterTransforms/AlphaLen=8,Prefix=8,Shared=4,ValueLen=8,HideObsolete/SeekGE-10           116ns ± 0%
CockroachDataBlockIterTransforms/AlphaLen=8,Prefix=8,Shared=4,ValueLen=8,SyntheticPrefix/Next-10         12.6ns ± 0%
CockroachDataBlockIterTransforms/AlphaLen=8,Prefix=8,Shared=4,ValueLen=8,SyntheticPrefix/SeekGE-10        116ns ± 0%
CockroachDataBlockIterTransforms/AlphaLen=8,Prefix=8,Shared=4,ValueLen=8,SyntheticSuffix/Next-10         13.0ns ± 4%
CockroachDataBlockIterTransforms/AlphaLen=8,Prefix=8,Shared=4,ValueLen=8,SyntheticSuffix/SeekGE-10        117ns ± 0%
CockroachDataBlockIterTransforms/AlphaLen=8,Prefix=128,Shared=64,ValueLen=128/Next-10                    10.8ns ± 0%
CockroachDataBlockIterTransforms/AlphaLen=8,Prefix=128,Shared=64,ValueLen=128/SeekGE-10                  91.6ns ± 0%
CockroachDataBlockIterTransforms/AlphaLen=8,Prefix=128,Shared=64,ValueLen=128,SynthSeqNum/Next-10        10.8ns ± 0%
CockroachDataBlockIterTransforms/AlphaLen=8,Prefix=128,Shared=64,ValueLen=128,SynthSeqNum/SeekGE-10      91.7ns ± 0%
CockroachDataBlockIterTransforms/AlphaLen=8,Prefix=128,Shared=64,ValueLen=128,HideObsolete/Next-10       11.3ns ± 1%
CockroachDataBlockIterTransforms/AlphaLen=8,Prefix=128,Shared=64,ValueLen=128,HideObsolete/SeekGE-10     94.2ns ± 1%
CockroachDataBlockIterTransforms/AlphaLen=8,Prefix=128,Shared=64,ValueLen=128,SyntheticPrefix/Next-10    10.7ns ± 0%
CockroachDataBlockIterTransforms/AlphaLen=8,Prefix=128,Shared=64,ValueLen=128,SyntheticPrefix/SeekGE-10  94.1ns ± 0%
CockroachDataBlockIterTransforms/AlphaLen=8,Prefix=128,Shared=64,ValueLen=128,SyntheticSuffix/Next-10    10.9ns ± 0%
CockroachDataBlockIterTransforms/AlphaLen=8,Prefix=128,Shared=64,ValueLen=128,SyntheticSuffix/SeekGE-10  95.6ns ± 0%

[RaduBerinde]

Merged with the recent DataBlockIter changes, PTAL.

[jbowens]

Reviewed 3 of 9 files at r2, 2 of 3 files at r3, 6 of 6 files at r5, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @RaduBerinde)

---

sstable/colblk/data_block.go line 868 at r5 (raw file):

	i.split = split
	i.getLazyValuer = getLazyValuer
	i.keySeeker = i.keySchema.NewKeySeeker()

Hrm, I didn't think we could do this in InitOnce because we're making the assumption that we won't Init/InitHandle after a call Close (which is then we release the seeker back to the pool).

---

sstable/colblk/data_block.go line 1026 at r5 (raw file):

	if i.noTransforms {
		// Fast path.
		i.row, _ = i.keySeeker.SeekGE(key, i.row, searchDir)

code coverage is saying the fast path isn't covered by our tests; is that right? I thought our datadriven unit tests covered the no transform SeekGE case?

---

sstable/colblk/data_block.go line 1123 at r5 (raw file):

	// Inline decodeKey(), adding obsolete points logic.
	if i.noTransforms {
		// Fast path.

ditto re: no-transform code coverage

[RaduBerinde]

sstable/colblk/data_block.go line 1026 at r5 (raw file):

Previously, jbowens (Jackson Owens) wrote…

code coverage is saying the fast path isn't covered by our tests; is that right? I thought our datadriven unit tests covered the no transform SeekGE case?

Wow, great catch!! I missed a few ! in NoTransforms()

[RaduBerinde]

Reviewable status: 9 of 12 files reviewed, 3 unresolved discussions (waiting on @jbowens)

---

sstable/colblk/data_block.go line 868 at r5 (raw file):

Previously, jbowens (Jackson Owens) wrote…

Hrm, I didn't think we could do this in InitOnce because we're making the assumption that we won't Init/InitHandle after a call Close (which is then we release the seeker back to the pool).

Reverted this part.

---

sstable/colblk/data_block.go line 1123 at r5 (raw file):

Previously, jbowens (Jackson Owens) wrote…

ditto re: no-transform code coverage

Fixed.