colblk: separate prefixes and suffixes in index blocks

[jbowens]

No description provided.

[cockroach-teamcity]

This change is 

[RaduBerinde]

Very cool! I know it's still a draft but posted some early comments.

Reviewable status: 0 of 22 files reviewed, 6 unresolved discussions (waiting on @jbowens)

---

sstable/colblk/index_block.go line 104 at r2 (raw file):

	// Decompose the separator into prefix and suffix.
	s := w.split(separator)
	commonPrefixLen := s

[nit] feels like a strange value to pass to the first one, shouldn't 0 also work? It doesn't look like we care about its value for the first key.

---

sstable/colblk/index_block.go line 119 at r2 (raw file):

// UnsafeSeparator returns the separator of the i'th entry. If r is the number
// of rows, it is required that r-2 <= i < r.

[nit] i=r-2 or i=r-1 is more clea

---

sstable/colblk/index_block.go line 121 at r2 (raw file):

// of rows, it is required that r-2 <= i < r.
func (w *IndexBlockWriter) UnsafeSeparator(i int) []byte {
	if i < w.rows-2 || i >= w.rows {

[nit] we already have this check in UnsafeGet

---

sstable/colblk/index_block.go line 382 at r2 (raw file):

				return i.row < i.n
			}
			i.row = i.d.bd.Rows()

i.row = n; return false

---

sstable/colblk/index_block.go line 411 at r2 (raw file):

	// Fall back to a slow scan forward.
	//
	// TODO(jackson): This can be improved by adding a PrefixBytes.NextUniqueKey

Maybe we should always include a prefixChanged bitmap inside the prefix bytes?

---

sstable/colblk/index_block.go line 416 at r2 (raw file):

	// with offset indexes, so it's deferred for now.
	for i.row++; i.row < i.d.bd.Rows(); i.row++ {
		if i.SeparatorGT(key, true /* orEqual */) {

We want to compare just the suffix, no? SeparatorGT checks the prefix again

[jbowens]

Reviewable status: 0 of 54 files reviewed, 6 unresolved discussions (waiting on @RaduBerinde)

---

sstable/colblk/index_block.go line 416 at r2 (raw file):

Previously, RaduBerinde wrote…

We want to compare just the suffix, no? SeparatorGT checks the prefix again

Unfortunately as long as we don't know when the prefix changes, we have to do a prefix comparison too

[RaduBerinde]

Reviewable status: 0 of 54 files reviewed, 6 unresolved discussions (waiting on @jbowens)

---

sstable/colblk/index_block.go line 416 at r2 (raw file):

Previously, jbowens (Jackson Owens) wrote…

Unfortunately as long as we don't know when the prefix changes, we have to do a prefix comparison too

Yeah, sorry, I meant to delete that comment