Apply changes from code review

Signed-off-by: Christian Haudum <[email protected]>
grafana · Sep 5, 2024 · 1eb3b56 · 1eb3b56
1 parent a7c0a5e
commit 1eb3b56
Show file tree

Hide file tree

Showing 6 changed files with 9 additions and 22 deletions.
diff --git a/pkg/bloomgateway/processor.go b/pkg/bloomgateway/processor.go
@@ -145,8 +145,8 @@ func (p *processor) processBlock(_ context.Context, bq *bloomshipper.CloseableBl
 		return err
 	}
 
-	// We require V3 schema
-	if !schema.IsCurrentSchema() {
+	// We require V3+ schema
+	if schema.Version() < v1.V3 {
 		return v1.ErrUnsupportedSchemaVersion
 	}
 

diff --git a/pkg/storage/bloom/v1/bloom_builder.go b/pkg/storage/bloom/v1/bloom_builder.go
@@ -46,8 +46,6 @@ func (b *BloomBlockBuilder) Append(bloom *Bloom) (BloomOffset, error) {
 		}
 	}
 
-	// version := b.opts.Schema.version
-
 	b.scratch.Reset()
 	if err := bloom.Encode(b.scratch); err != nil {
 		return BloomOffset{}, errors.Wrap(err, "encoding bloom")

diff --git a/pkg/storage/bloom/v1/builder.go b/pkg/storage/bloom/v1/builder.go
@@ -66,18 +66,6 @@ func (b BlockOptions) Encode(enc *encoding.Encbuf) {
 	enc.PutBE64(b.BlockSize)
 }
 
-// func NewDefaultBlockOptions(maxBlockSizeBytes, maxBloomSizeBytes uint64) BlockOptions {
-// 	opts := NewBlockOptionsFromSchema(Schema{
-// 		version:     DefaultSchemaVersion,
-// 		encoding:    chunkenc.EncNone,
-// 		nGramLength: 0,
-// 		nGramSkip:   0,
-// 	})
-// 	opts.BlockSize = maxBlockSizeBytes
-// 	opts.UnencodedBlockOptions.MaxBloomSizeBytes = maxBloomSizeBytes
-// 	return opts
-// }
-
 func NewBlockOptions(enc chunkenc.Encoding, nGramLength, nGramSkip, maxBlockSizeBytes, maxBloomSizeBytes uint64) BlockOptions {
 	opts := NewBlockOptionsFromSchema(Schema{
 		version:     CurrentSchemaVersion,

diff --git a/pkg/storage/bloom/v1/builder_test.go b/pkg/storage/bloom/v1/builder_test.go
@@ -550,6 +550,9 @@ func TestMergeBuilder_Roundtrip(t *testing.T) {
 
 	_, _, err = mb.Build(builder)
 	require.Nil(t, err)
+	// checksum changes as soon as the contents of the block or the encoding change
+	// once the block format is stable, calculate the checksum and assert its correctness
+	// require.Equal(t, uint32(0x2a6cdba6), checksum)
 
 	// ensure the new block contains one copy of all the data
 	// by comparing it against an iterator over the source data

diff --git a/pkg/storage/bloom/v1/schema.go b/pkg/storage/bloom/v1/schema.go
@@ -17,6 +17,8 @@ func (v Version) String() string {
 }
 
 const (
+	magicNumber = uint32(0xCA7CAFE5)
+
 	// Add new versions below
 	V1 Version = iota
 	// V2 supports single series blooms encoded over multiple pages
@@ -58,8 +60,8 @@ func (s Schema) Compatible(other Schema) bool {
 	return s == other
 }
 
-func (s Schema) IsCurrentSchema() bool {
-	return s.version == CurrentSchemaVersion
+func (s Schema) Version() Version {
+	return s.version
 }
 
 func (s Schema) NGramLen() int {

diff --git a/pkg/storage/bloom/v1/util.go b/pkg/storage/bloom/v1/util.go
@@ -9,10 +9,6 @@ import (
 	"github.com/grafana/loki/v3/pkg/util/mempool"
 )
 
-const (
-	magicNumber = uint32(0xCA7CAFE5)
-)
-
 var (
 	castagnoliTable = crc32.MakeTable(crc32.Castagnoli)