From 7ae8accfa8cedb6d7454da25abc0231f01fd43c2 Mon Sep 17 00:00:00 2001 From: Jason Aten Date: Mon, 21 Sep 2020 17:09:19 -0500 Subject: [PATCH] Bitmap.DirectAdd avoids returning overfull containers - log Debugf when we repair a fragment block - better run-run roaring testing for over-sized containers - add which fragment path to panic on container too big - include container contents in roaring hash for pilosa-chk/pilosa-check-backup --- fragment.go | 3 ++ roaring/roaring.go | 16 ++++++ roaring/roaring_internal_test.go | 92 +++++++++++++++++++++++++++++++- txfactory.go | 11 +++- 4 files changed, 119 insertions(+), 3 deletions(-) diff --git a/fragment.go b/fragment.go index d248f4665..122688ec5 100644 --- a/fragment.go +++ b/fragment.go @@ -3526,6 +3526,9 @@ func (s *fragmentSyncer) syncFragment() error { // to determine which bits to update, we consider the primary // replica to be correct, and overwrite the non-primary replicas // with the primary's data. + + s.Fragment.holder.Logger.Debugf("sync block from primary: index='%v' field='%v' view='%v' shard='%v' id=%d", s.Fragment.index, s.Fragment.field, s.Fragment.view, s.Fragment.shard, blockID) + switch s.FieldType { case FieldTypeInt, FieldTypeDecimal: // Synchronize block from the primary replica. diff --git a/roaring/roaring.go b/roaring/roaring.go index 384149ea6..89c9618bf 100644 --- a/roaring/roaring.go +++ b/roaring/roaring.go @@ -362,6 +362,17 @@ func (b *Bitmap) DirectAdd(v uint64) bool { cont := b.Containers.GetOrCreate(highbits(v)) newC, changed := cont.add(lowbits(v)) if newC != cont { + // avoid returning invalid container, and avoid a slow optimize call. + switch newC.typeID { + case ContainerArray: + if len(newC.array()) > ArrayMaxSize { + newC = newC.arrayToBitmap() + } + case ContainerRun: + if len(newC.runs()) > runMaxSize { + newC = newC.runToBitmap() + } + } b.Containers.Put(highbits(v), newC) } return changed @@ -4830,7 +4841,12 @@ func unionRunRunInPlace(a, b *Container) *Container { a.setRuns(runs) a.setN(n) + + if len(runs) > runMaxSize { + a = a.runToBitmap() + } return a + } // unionInterval16InPlace merges two slice of intervals in place (in a). diff --git a/roaring/roaring_internal_test.go b/roaring/roaring_internal_test.go index c75df2140..1a459fd8a 100644 --- a/roaring/roaring_internal_test.go +++ b/roaring/roaring_internal_test.go @@ -4398,7 +4398,8 @@ func TestUnionRunRunInPlaceBitwiseCompare(t *testing.T) { out2 := unionRunRunInPlace(arun, brun) out1.Repair() - err := out1.BitwiseCompare(out2.runToBitmap()) + // out2 may no longer be a run container, so don't assume that. + err := out1.BitwiseCompare(out2) if err != nil { t.Fatal(err) } @@ -4443,3 +4444,92 @@ func TestCloneRoaringIterator(t *testing.T) { t.Fatalf("keys != keys2. keys='%#v'; keys2='%#v'", keys, keys2) } } + +// we were seeing unionInterval16InPlace() returning too +// large an run container, which was causing problems when +// we write to the transactional backends. Verify that +// unionRunRunInPlace() converts to bitmap if its too large. +// +func TestContainer_unionRunRunInPlace_TwoBigRunArrays(t *testing.T) { + + a := NewContainerRun(nil) + b := NewContainerRun(nil) + + for i := uint16(0); i < 8192; i++ { + if i%3 == 0 { + a, _ = a.runAdd(i) + } + } + for i := uint16(0); i < 8192; i++ { + if i%3 == 1 { + b, _ = b.runAdd(i) + } + } + + c := unionRunRunInPlace(a, b) + + typ := ContainerType(c) + if typ == ContainerRun { + nr := len(c.runs()) + if nr > runMaxSize { + panic(fmt.Sprintf("runs is over runMaxSize: %v", nr)) + } + } +} + +// we were seeing unionInterval16InPlace() returning too +// large an array container, which was causing problems when +// we write to the transactional backends. Verify that +// unionArrayArrayInPlace() converts to bitmap if its too large. +// Confirms that optimize() is done at the end of unionArrayArrayInPlace(). +func TestContainer_unionArrayArrayInPlace_TwoBigArrayArrays(t *testing.T) { + + a := NewContainerArray(nil) + b := NewContainerArray(nil) + + for i := uint16(0); i < 4096; i++ { + a, _ = a.arrayAdd(i) + } + for i := uint16(4096); i < 8192; i++ { + b, _ = b.arrayAdd(i) + } + + c := unionArrayArrayInPlace(a, b) + + typ := ContainerType(c) + if typ == ContainerArray { + nr := len(c.array()) + if nr > ArrayMaxSize { + panic(fmt.Sprintf("arrays is over arrayMaxSize: %v", nr)) + } + } +} + +// the love child of the above two tests. +func TestContainer_unionInPlace_ArrayUnionRun(t *testing.T) { + + for k := 0; k < 2; k++ { + a := NewContainerArray(nil) + for i := uint16(0); i < 8192; i += 2 { + a, _ = a.arrayAdd(i) + } + + b := NewContainerRun(nil) + for i := uint16(8192); i < 8192*2; i++ { + if i%3 == 1 { + b, _ = b.runAdd(i) + } + } + + var c *Container + if k == 0 { + c = a.unionInPlace(b) + } else { + c = b.unionInPlace(a) + } + typ := ContainerType(c) + if typ == ContainerArray { + panic("should be impossible to have an array") + } + } +} diff --git a/txfactory.go b/txfactory.go index 10e89395b..a72f451c3 100644 --- a/txfactory.go +++ b/txfactory.go @@ -857,12 +857,19 @@ func (idx *Index) StringifiedRoaringKeys(hashOnly, showOps bool, o Txo) (r strin } func RoaringFragmentChecksum(path string, index, field, view string, shard uint64) (r string, hotbits int) { + defer func() { + r := recover() + if r != nil { + panic(fmt.Sprintf("caught panic on path='%v', index='%v', field='%v', view='%v', shard='%v': %v", + path, index, field, view, shard, r)) + } + }() hasher := blake3.New() showOps := false hashOnly := true - _, hotbits, err := stringifiedRawRoaringFragment(path, index, field, view, shard, showOps, hashOnly, hasher) + hash, hotbits, err := stringifiedRawRoaringFragment(path, index, field, view, shard, showOps, hashOnly, hasher) panicOn(err) - fmt.Fprintf(hasher, "%v/%v/%v/%v", index, field, view, shard) + fmt.Fprintf(hasher, "%v/%v/%v/%v/%v", index, field, view, shard, hash) var buf [16]byte _, _ = hasher.Digest().Read(buf[0:]) return fmt.Sprintf("%x", buf), hotbits