sstable: clean up and document maximum handle lengths

This commit removes the +1 from valblk.HandleMaxLen. The code treats the handle
as the data after the valuePrefix byte, noninclusive of the byte. The assertion
that blockHandleLikelyMaxLen is large enough now incorporates the +1 of the
valuePrefix byte explicitly. This was the only usage the depended on
HandleMaxLen being inclusive of the valuePrefix byte.

Additionally, this commit documents blockHandleMaxLenWithoutProperties and adds
an assertion that blockHandleLikelyMaxLen is larger than the length of an
inline blob value handle (blob.MaxInlineHandleLength) plus the valuePrefix
byte.

Author: jbowens

sstable/internal.go

@@ -7,6 +7,7 @@ package sstable
 import (
 	"github.com/cockroachdb/pebble/internal/base"
 	"github.com/cockroachdb/pebble/internal/keyspan"
+	"github.com/cockroachdb/pebble/sstable/blob"
 	"github.com/cockroachdb/pebble/sstable/valblk"
 )
 
@@ -35,5 +36,14 @@ const valueBlocksIndexHandleMaxLen = blockHandleMaxLenWithoutProperties + 3
 // Assert blockHandleLikelyMaxLen >= valueBlocksIndexHandleMaxLen.
 const _ = uint(blockHandleLikelyMaxLen - valueBlocksIndexHandleMaxLen)
 
-// Assert blockHandleLikelyMaxLen >= valblk.HandleMaxLen.
-const _ = uint(blockHandleLikelyMaxLen - valblk.HandleMaxLen)
+// Assert blockHandleLikelyMaxLen >= (valblk.HandleMaxLen+1).
+//
+// The additional 1 is for the 'valuePrefix' byte which prefaces values in
+// recent SSTable versions.
+const _ = uint(blockHandleLikelyMaxLen - valblk.HandleMaxLen - 1)
+
+// Assert blockHandleLikelyMaxLen >= (blob.MaxInlineHandleLength+1).
+//
+// The additional 1 is for the 'valuePrefix' byte which prefaces values in recent
+// SSTable versions.
+const _ = uint(blockHandleLikelyMaxLen - blob.MaxInlineHandleLength - 1)

sstable/table.go

@@ -186,7 +186,10 @@ Data blocks have some additional features:
 */
 
 const (
-	blockHandleMaxLenWithoutProperties = 10 + 10
+	// blockHandleMaxLenWithoutProperties is the maximum length of a block
+	// handle that does not encode any block properties. It consists of just
+	// the offset and length, each of which is a varint-encoded uint64.
+	blockHandleMaxLenWithoutProperties = 2 * binary.MaxVarintLen64
 	// blockHandleLikelyMaxLen can be used for pre-allocating buffers to
 	// reduce memory copies. It is not guaranteed that a block handle will not
 	// exceed this length.

sstable/valblk/valblk.go

@@ -186,11 +186,10 @@ type Handle struct {
 
 // HandleMaxLen is the maximum length of a variable-width encoded Handle.
 //
-// Handle fields are varint encoded, so maximum 5 bytes each, plus 1 byte for
-// the valuePrefix. This could alternatively be group varint encoded, but
-// experiments were inconclusive
+// Handle fields are varint encoded, so maximum 5 bytes each. This could
+// alternatively be group varint encoded, but experiments were inconclusive
 // (https://github.com/cockroachdb/pebble/pull/1443#issuecomment-1270298802).
-const HandleMaxLen = 5*3 + 1
+const HandleMaxLen = 3 * binary.MaxVarintLen32
 
 // EncodeHandle encodes the Handle into dst in a variable-width encoding and
 // returns the number of bytes encoded.