From 01b0567632ae593d1314730cfd20a18538cba11d Mon Sep 17 00:00:00 2001 From: George Kudrayvtsev Date: Tue, 20 Sep 2022 12:41:02 -0700 Subject: [PATCH 1/5] Rename explorer -> index for accuracy --- exp/lighthorizon/tools/{explorer.go => index.go} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename exp/lighthorizon/tools/{explorer.go => index.go} (100%) diff --git a/exp/lighthorizon/tools/explorer.go b/exp/lighthorizon/tools/index.go similarity index 100% rename from exp/lighthorizon/tools/explorer.go rename to exp/lighthorizon/tools/index.go From c7a8fc12294f27614b2df972e9aabe855fe76115 Mon Sep 17 00:00:00 2001 From: George Kudrayvtsev Date: Tue, 20 Sep 2022 15:22:28 -0700 Subject: [PATCH 2/5] Add ability to 'unset' bits and purge from index store --- exp/lighthorizon/index/types/bitmap.go | 93 +++++++++++++++++ exp/lighthorizon/index/types/bitmap_test.go | 64 ++++++++++++ exp/lighthorizon/tools/index.go | 105 +++++++++++++++++++- exp/lighthorizon/tools/index_test.go | 58 +++++++++++ 4 files changed, 316 insertions(+), 4 deletions(-) create mode 100644 exp/lighthorizon/tools/index_test.go diff --git a/exp/lighthorizon/index/types/bitmap.go b/exp/lighthorizon/index/types/bitmap.go index 256f8c8d35..baa107f3f0 100644 --- a/exp/lighthorizon/index/types/bitmap.go +++ b/exp/lighthorizon/index/types/bitmap.go @@ -47,6 +47,14 @@ func (i *BitmapIndex) SetActive(index uint32) error { return i.setActive(index) } +func (i *BitmapIndex) SetInactive(index uint32) error { + i.mutex.Lock() + defer i.mutex.Unlock() + return i.setInactive(index) +} + +// bitShiftLeft returns a byte with the bit set corresponding to the index. In +// other words, it flips the bit corresponding to the index's "position" mod-8. func bitShiftLeft(index uint32) byte { if index%8 == 0 { return 1 @@ -55,10 +63,18 @@ func bitShiftLeft(index uint32) byte { } } +// rangeFirstBit returns the index of the first *possible* active bit in the +// bitmap. In other words, if you just have SetActive(12), this will return 9, +// because you have one byte (0b0001_0000) and the *first* value the bitmap can +// represent is 9. func (i *BitmapIndex) rangeFirstBit() uint32 { return (i.firstBit-1)/8*8 + 1 } +// rangeLastBit returns the index of the last *possible* active bit in the +// bitmap. In other words, if you just have SetActive(12), this will return 16, +// because you have one byte (0b0001_0000) and the *last* value the bitmap can +// represent is 16. func (i *BitmapIndex) rangeLastBit() uint32 { return i.rangeFirstBit() + uint32(len(i.bitmap))*8 - 1 } @@ -113,6 +129,83 @@ func (i *BitmapIndex) setActive(index uint32) error { return nil } +func (i *BitmapIndex) setInactive(index uint32) error { + // Is this index even active in the first place? + if i.firstBit == 0 || index < i.rangeFirstBit() || index > i.rangeLastBit() { + return nil // not really an error + } + + loc := (index - i.rangeFirstBit()) / 8 // which byte? + b := bitShiftLeft(index) // which bit w/in the byte? + i.bitmap[loc] &= ^b // unset only that bit + + // If unsetting this bit made the first byte empty OR we unset the earliest + // set bit, we need to find the next "first" active bit. + if loc == 0 && i.firstBit == index { + // find the next active bit to set as the start + nextBit, err := i.nextActiveBit(index) + if err == io.EOF { + i.firstBit = 0 + i.lastBit = 0 + i.bitmap = []byte{} + } else if err != nil { + return err + } else { + i.firstBit = nextBit + } + + for i.bitmap[0] == 0 { + i.bitmap = i.bitmap[1:] // trim first (now-empty) byte off + } + } else if int(loc) == len(i.bitmap)-1 { + if i.bitmap[loc] == 0 { + i.bitmap = i.bitmap[:loc-1] // trim last (now-empty) byte + + // find the latest non-empty byte, to set as the new "end" + for j := len(i.bitmap) - 1; j >= 0; j-- { + if i.bitmap[j] == 0 { + continue + } + + offset, _ := maxBitAfter(i.bitmap[j], 0) + byteOffset := uint32(len(i.bitmap)-1) * 8 + i.lastBit = i.rangeFirstBit() + byteOffset + offset + i.bitmap = i.bitmap[:j+1] + break + } + } else { + // Otherwise, do we need to adjust the range? + var idx uint32 = 1 + whichBit := index % 8 + if whichBit != 0 { + idx = 8 - whichBit + } + + _, ok := maxBitAfter(i.bitmap[loc], idx+1) + + // Imagine we had 0b0011_0100 and we unset the last active bit. + // ^ + // Then, we need to adjust our internal lastBit tracker to represent + // the ^ bit above. This means finding the first previous set bit. + if !ok { // no further bits are set + j := int(idx) + for ; j >= 0 && !ok; j-- { + _, ok = maxBitAfter(i.bitmap[loc], uint32(j)) + } + + // We know from the earlier conditional that *some* bit is set, + // so we know that j represents the index of the bit that's the + // new "last active" bit. + firstByte := i.rangeFirstBit() + byteOffset := uint32(len(i.bitmap)-1) * 8 + i.lastBit = firstByte + byteOffset + uint32(j+1) + } + } + } + + return nil +} + //lint:ignore U1000 Ignore unused function temporarily func (i *BitmapIndex) isActive(index uint32) bool { if index >= i.firstBit && index <= i.lastBit { diff --git a/exp/lighthorizon/index/types/bitmap_test.go b/exp/lighthorizon/index/types/bitmap_test.go index 7d5848f67a..634bda71d3 100644 --- a/exp/lighthorizon/index/types/bitmap_test.go +++ b/exp/lighthorizon/index/types/bitmap_test.go @@ -127,6 +127,70 @@ func TestSetActive(t *testing.T) { assert.Equal(t, uint32(26), index.lastBit) } +// TestSetInactive ensures that you can flip active bits off and the bitmap +// compresses in size accordingly. +func TestSetInactive(t *testing.T) { + index := &BitmapIndex{} + index.SetActive(17) + index.SetActive(17 + 9) + index.SetActive(17 + 9 + 10) + assert.Equal(t, []byte{0b1000_0000, 0b0100_0000, 0b0001_0000}, index.bitmap) + + // disabling bits should work + index.SetInactive(17) + assert.False(t, index.isActive(17)) + + // it should trim off the first byte now + assert.Equal(t, []byte{0b0100_0000, 0b0001_0000}, index.bitmap) + assert.EqualValues(t, 17+9, index.firstBit) + assert.EqualValues(t, 17+9+10, index.lastBit) + + // it should compress empty bytes on shrink + index = &BitmapIndex{} + index.SetActive(1) + index.SetActive(1 + 2) + index.SetActive(1 + 9) + index.SetActive(1 + 9 + 8 + 9) + assert.Equal(t, []byte{0b1010_0000, 0b0100_0000, 0b0000_0000, 0b0010_0000}, index.bitmap) + + // ...from the left + index.SetInactive(1) + assert.Equal(t, []byte{0b0010_0000, 0b0100_0000, 0b0000_0000, 0b0010_0000}, index.bitmap) + index.SetInactive(3) + assert.Equal(t, []byte{0b0100_0000, 0b0000_0000, 0b0010_0000}, index.bitmap) + assert.EqualValues(t, 1+9, index.firstBit) + assert.EqualValues(t, 1+9+8+9, index.lastBit) + + // ...and the right + index.SetInactive(1 + 9 + 8 + 9) + assert.Equal(t, []byte{0b0100_0000}, index.bitmap) + assert.EqualValues(t, 1+9, index.firstBit) + assert.EqualValues(t, 1+9, index.lastBit) + + // ensure right-hand compression it works for multiple bytes, too + index = &BitmapIndex{} + index.SetActive(2) + index.SetActive(2 + 2) + index.SetActive(2 + 9) + index.SetActive(2 + 9 + 8 + 9) + index.SetActive(2 + 9 + 8 + 10) + assert.Equal(t, []byte{0b0101_0000, 0b0010_0000, 0b0000_0000, 0b0001_1000}, index.bitmap) + + index.setInactive(2 + 9 + 8 + 10) + assert.Equal(t, []byte{0b0101_0000, 0b0010_0000, 0b0000_0000, 0b0001_0000}, index.bitmap) + assert.EqualValues(t, 2+9+8+9, index.lastBit) + + index.setInactive(2 + 9 + 8 + 9) + assert.Equal(t, []byte{0b0101_0000, 0b0010_0000}, index.bitmap) + assert.EqualValues(t, 2, index.firstBit) + assert.EqualValues(t, 2+9, index.lastBit) + + index.setInactive(2 + 2) + assert.Equal(t, []byte{0b0100_0000, 0b0010_0000}, index.bitmap) + assert.EqualValues(t, 2, index.firstBit) + assert.EqualValues(t, 2+9, index.lastBit) +} + func TestNextActive(t *testing.T) { t.Run("empty", func(t *testing.T) { index := &BitmapIndex{} diff --git a/exp/lighthorizon/tools/index.go b/exp/lighthorizon/tools/index.go index d05175cb1b..0f2305035a 100644 --- a/exp/lighthorizon/tools/index.go +++ b/exp/lighthorizon/tools/index.go @@ -5,6 +5,7 @@ import ( "io" "os" "os/signal" + "strconv" "strings" "syscall" "time" @@ -27,7 +28,7 @@ var ( func AddIndexCommands(parent *cobra.Command) *cobra.Command { cmd := &cobra.Command{ Use: "index", - Long: "Lets you view details about an index source.", + Long: "Lets you view details about an index source and modify it.", Example: ` index view file:///tmp/indices index view file:///tmp/indices GAGJZWQ5QT34VK3U6W6YKRYFIK6YSAXQC6BHIIYLG6X3CE5QW2KAYNJR @@ -163,9 +164,36 @@ view gcs://indices --limit=10 GAXLQGKIUAIIUHAX4GJO3J7HFGLBCNF6ZCZSTLJE7EKO5IUHGL }, } + purge := &cobra.Command{ + Use: "purge ", + Long: "Purges all indices for the given ledger range.", + Example: `purge s3://indices 10000 10005`, + RunE: func(cmd *cobra.Command, args []string) error { + if len(args) != 3 { + return cmd.Usage() + } + + path := args[0] + start, err := strconv.ParseUint(args[1], 10, 32) + if err != nil { + return cmd.Usage() + } + end, err := strconv.ParseUint(args[2], 10, 32) + if err != nil { + return cmd.Usage() + } + + r := historyarchive.Range{Low: uint32(start), High: uint32(end)} + log.Infof("Purging all indices from %s for ledger range: [%d, %d].", + path, r.Low, r.High) + + return purgeIndex(path, r) + }, + } + view.Flags().Uint("limit", 10, "a maximum number of accounts or checkpoints to show") view.Flags().String("index-name", "", "filter for a particular index") - cmd.AddCommand(stats, view) + cmd.AddCommand(stats, view, purge) if parent == nil { return cmd @@ -179,12 +207,15 @@ func getIndex(path, account, indexName string, limit uint) map[uint32][]string { store, err := index.Connect(path) if err != nil { - log.Fatal(err) + log.Fatalf("Failed to connect to index store at %s: %v", path, err) + return nil } indices, err := store.Read(account) if err != nil { - log.Fatal(err) + log.Fatalf("Failed to read indices for %s from index store at %s: %v", + account, path, err) + return nil } // It's better to summarize activity and then group it by index rather than @@ -257,3 +288,69 @@ func showAccounts(path string, limit uint) []string { return accounts } + +func purgeIndex(path string, r historyarchive.Range) error { + freq := historyarchive.DefaultCheckpointFrequency + store, err := index.Connect(path) + if err != nil { + log.Fatalf("Failed to connect to index store at %s: %v", path, err) + return err + } + + accounts, err := store.ReadAccounts() + if err != nil { + log.Fatalf("Failed read accounts: %v", err) + return err + } + + purged := 0 + for _, account := range accounts { + L := log.WithField("account", account) + + indices, err := store.Read(account) + if err != nil { + L.Errorf("Failed to read indices: %v", err) + continue + } + + for name, index := range indices { + var err error + active := uint32(0) + for err == nil { + if active*freq < r.Low { // too low, skip ahead + active, err = index.NextActiveBit(active + 1) + continue + } else if active*freq > r.High { // too high, we're done + break + } + + L.WithField("index", name). + Debugf("Purged checkpoint %d (ledgers %d through %d).", + active, active*freq, (active+1)*freq-1) + + purged++ + + index.SetInactive(active) + active, err = index.NextActiveBit(active) + } + + if err != nil && err != io.EOF { + L.WithField("index", name). + Errorf("Iterating over index failed: %v", err) + continue + } + + } + + store.AddParticipantToIndexesNoBackend(account, indices) + if err := store.Flush(); err != nil { + log.WithField("account", account). + Errorf("Flushing index failed: %v", err) + continue + } + } + + log.Infof("Purged %d values across %d accounts from all indices at %s.", + purged, len(accounts), path) + return nil +} diff --git a/exp/lighthorizon/tools/index_test.go b/exp/lighthorizon/tools/index_test.go new file mode 100644 index 0000000000..6d42f88f30 --- /dev/null +++ b/exp/lighthorizon/tools/index_test.go @@ -0,0 +1,58 @@ +package tools + +import ( + "path/filepath" + "testing" + + "github.com/stellar/go/exp/lighthorizon/index" + "github.com/stellar/go/historyarchive" + "github.com/stellar/go/keypair" + "github.com/stellar/go/support/log" + "github.com/stretchr/testify/require" +) + +const ( + freq = historyarchive.DefaultCheckpointFrequency +) + +func TestIndexPurge(t *testing.T) { + log.SetLevel(log.DebugLevel) + + tempFile := "file://" + filepath.Join(t.TempDir(), "index-store") + accounts := []string{keypair.MustRandom().Address()} + + idx, err := index.Connect(tempFile) + require.NoError(t, err) + + for _, chk := range []uint32{14, 15, 16, 17, 20, 25, 123} { + require.NoError(t, idx.AddParticipantsToIndexes(chk, "test", accounts)) + } + + idx.Flush() // saves to disk + + // Try purging the index + err = purgeIndex(tempFile, historyarchive.Range{Low: 15 * freq, High: 22 * freq}) + require.NoError(t, err) + + // Check to make sure it worked. + idx, err = index.Connect(tempFile) + require.NoError(t, err) + + // Ensure that the index is in the expected state. + indices, err := idx.Read(accounts[0]) + require.NoError(t, err) + require.Contains(t, indices, "test") + + index := indices["test"] + i, err := index.NextActiveBit(0) + require.NoError(t, err) + require.EqualValues(t, 14, i) + + i, err = index.NextActiveBit(15) + require.NoError(t, err) + require.EqualValues(t, 25, i) + + i, err = index.NextActiveBit(i + 1) + require.NoError(t, err) + require.EqualValues(t, 123, i) +} From a986ef8b79f8ecf601e76239bf7367da1dedd4e1 Mon Sep 17 00:00:00 2001 From: George Kudrayvtsev Date: Thu, 22 Sep 2022 13:07:32 -0700 Subject: [PATCH 3/5] Simplify empty compression checks --- exp/lighthorizon/index/types/bitmap.go | 48 ++++++++++----------- exp/lighthorizon/index/types/bitmap_test.go | 9 +++- 2 files changed, 31 insertions(+), 26 deletions(-) diff --git a/exp/lighthorizon/index/types/bitmap.go b/exp/lighthorizon/index/types/bitmap.go index baa107f3f0..c48e58b7ea 100644 --- a/exp/lighthorizon/index/types/bitmap.go +++ b/exp/lighthorizon/index/types/bitmap.go @@ -151,12 +151,10 @@ func (i *BitmapIndex) setInactive(index uint32) error { } else if err != nil { return err } else { + // Trim all (now-)empty bytes off the front. + i.bitmap = i.bitmap[int(nextBit/8)-int(i.firstBit/8):] i.firstBit = nextBit } - - for i.bitmap[0] == 0 { - i.bitmap = i.bitmap[1:] // trim first (now-empty) byte off - } } else if int(loc) == len(i.bitmap)-1 { if i.bitmap[loc] == 0 { i.bitmap = i.bitmap[:loc-1] // trim last (now-empty) byte @@ -173,33 +171,35 @@ func (i *BitmapIndex) setInactive(index uint32) error { i.bitmap = i.bitmap[:j+1] break } - } else { - // Otherwise, do we need to adjust the range? - var idx uint32 = 1 - whichBit := index % 8 - if whichBit != 0 { - idx = 8 - whichBit - } + } else if i.lastBit == index { + // Otherwise, do we need to adjust the range? Imagine we had + // 0b0011_0100 and we unset the last active bit. + // ^ + // Then, we need to adjust our internal lastBit tracker to represent + // the ^ bit above. This means finding the first previous set bit. - _, ok := maxBitAfter(i.bitmap[loc], idx+1) + // Get the "bit number" of the last active bit (i.e. the one we just + // turned off) to mark the starting point for the search. + idx := uint32(1) + if index%8 != 0 { + idx = 8 - (index % 8) + } // Imagine we had 0b0011_0100 and we unset the last active bit. // ^ // Then, we need to adjust our internal lastBit tracker to represent // the ^ bit above. This means finding the first previous set bit. - if !ok { // no further bits are set - j := int(idx) - for ; j >= 0 && !ok; j-- { - _, ok = maxBitAfter(i.bitmap[loc], uint32(j)) - } - - // We know from the earlier conditional that *some* bit is set, - // so we know that j represents the index of the bit that's the - // new "last active" bit. - firstByte := i.rangeFirstBit() - byteOffset := uint32(len(i.bitmap)-1) * 8 - i.lastBit = firstByte + byteOffset + uint32(j+1) + j, ok := int(idx), false + for ; j >= 0 && !ok; j-- { + _, ok = maxBitAfter(i.bitmap[loc], uint32(j)) } + + // We know from the earlier conditional that *some* bit is set, so + // we know that j represents the index of the bit that's the new + // "last active" bit. + firstByte := i.rangeFirstBit() + byteOffset := 8 * uint32(len(i.bitmap)-1) + i.lastBit = firstByte + byteOffset + uint32(j) + 1 } } diff --git a/exp/lighthorizon/index/types/bitmap_test.go b/exp/lighthorizon/index/types/bitmap_test.go index 634bda71d3..c5cfc3a04d 100644 --- a/exp/lighthorizon/index/types/bitmap_test.go +++ b/exp/lighthorizon/index/types/bitmap_test.go @@ -172,15 +172,20 @@ func TestSetInactive(t *testing.T) { index.SetActive(2) index.SetActive(2 + 2) index.SetActive(2 + 9) + index.SetActive(2 + 9 + 8 + 6) index.SetActive(2 + 9 + 8 + 9) index.SetActive(2 + 9 + 8 + 10) - assert.Equal(t, []byte{0b0101_0000, 0b0010_0000, 0b0000_0000, 0b0001_1000}, index.bitmap) + assert.Equal(t, []byte{0b0101_0000, 0b0010_0000, 0b0000_0000, 0b1001_1000}, index.bitmap) index.setInactive(2 + 9 + 8 + 10) - assert.Equal(t, []byte{0b0101_0000, 0b0010_0000, 0b0000_0000, 0b0001_0000}, index.bitmap) + assert.Equal(t, []byte{0b0101_0000, 0b0010_0000, 0b0000_0000, 0b1001_0000}, index.bitmap) assert.EqualValues(t, 2+9+8+9, index.lastBit) index.setInactive(2 + 9 + 8 + 9) + assert.Equal(t, []byte{0b0101_0000, 0b0010_0000, 0b0000_0000, 0b1000_0000}, index.bitmap) + assert.EqualValues(t, 2+9+8+6, index.lastBit) + + index.setInactive(2 + 9 + 8 + 6) assert.Equal(t, []byte{0b0101_0000, 0b0010_0000}, index.bitmap) assert.EqualValues(t, 2, index.firstBit) assert.EqualValues(t, 2+9, index.lastBit) From 3381caf27981882eeae39934a3ec51ccad7c3720 Mon Sep 17 00:00:00 2001 From: George Kudrayvtsev Date: Mon, 26 Sep 2022 14:03:38 -0700 Subject: [PATCH 4/5] Fix set/unset bugs with fuzzer test --- exp/lighthorizon/index/types/bitmap.go | 85 +++++++++++++-------- exp/lighthorizon/index/types/bitmap_test.go | 34 +++++++++ 2 files changed, 88 insertions(+), 31 deletions(-) diff --git a/exp/lighthorizon/index/types/bitmap.go b/exp/lighthorizon/index/types/bitmap.go index c48e58b7ea..8586f0e335 100644 --- a/exp/lighthorizon/index/types/bitmap.go +++ b/exp/lighthorizon/index/types/bitmap.go @@ -2,9 +2,12 @@ package index import ( "bytes" + "fmt" "io" + "strings" "sync" + "github.com/stellar/go/support/ordered" "github.com/stellar/go/xdr" ) @@ -102,20 +105,15 @@ func (i *BitmapIndex) setActive(index uint32) error { // Expand the bitmap if index < i.rangeFirstBit() { // ...to the left - c := (i.rangeFirstBit() - index) / 8 - if (i.rangeFirstBit()-index)%8 != 0 { - c++ - } - newBytes := make([]byte, c) + newBytes := make([]byte, distance(index, i.rangeFirstBit())) i.bitmap = append(newBytes, i.bitmap...) - b := bitShiftLeft(index) i.bitmap[0] = i.bitmap[0] | b i.firstBit = index } else if index > i.rangeLastBit() { // ... to the right - newBytes := make([]byte, (index-i.rangeLastBit())/8+1) + newBytes := make([]byte, distance(i.rangeLastBit(), index)) i.bitmap = append(i.bitmap, newBytes...) b := bitShiftLeft(index) loc := (index - i.rangeFirstBit()) / 8 @@ -129,6 +127,12 @@ func (i *BitmapIndex) setActive(index uint32) error { return nil } +// distance returns how many bytes occur between the two given indices. Note +// that j >= i, otherwise the result will be negative. +func distance(i, j uint32) int { + return (int(j)-1)/8 - (int(i)-1)/8 +} + func (i *BitmapIndex) setInactive(index uint32) error { // Is this index even active in the first place? if i.firstBit == 0 || index < i.rangeFirstBit() || index > i.rangeLastBit() { @@ -152,54 +156,52 @@ func (i *BitmapIndex) setInactive(index uint32) error { return err } else { // Trim all (now-)empty bytes off the front. - i.bitmap = i.bitmap[int(nextBit/8)-int(i.firstBit/8):] + diff := distance(i.firstBit, nextBit) + i.bitmap = i.bitmap[diff:] i.firstBit = nextBit } } else if int(loc) == len(i.bitmap)-1 { - if i.bitmap[loc] == 0 { - i.bitmap = i.bitmap[:loc-1] // trim last (now-empty) byte + idx := -1 + if i.bitmap[loc] == 0 { // find the latest non-empty byte, to set as the new "end" - for j := len(i.bitmap) - 1; j >= 0; j-- { - if i.bitmap[j] == 0 { - continue - } - - offset, _ := maxBitAfter(i.bitmap[j], 0) - byteOffset := uint32(len(i.bitmap)-1) * 8 - i.lastBit = i.rangeFirstBit() + byteOffset + offset - i.bitmap = i.bitmap[:j+1] - break + j := len(i.bitmap) - 1 + for i.bitmap[j] == 0 { + j-- } - } else if i.lastBit == index { - // Otherwise, do we need to adjust the range? Imagine we had - // 0b0011_0100 and we unset the last active bit. - // ^ - // Then, we need to adjust our internal lastBit tracker to represent - // the ^ bit above. This means finding the first previous set bit. + i.bitmap = i.bitmap[:j+1] + idx = 8 + } else if i.lastBit == index { // Get the "bit number" of the last active bit (i.e. the one we just // turned off) to mark the starting point for the search. - idx := uint32(1) + idx = 8 if index%8 != 0 { - idx = 8 - (index % 8) + idx = int(index % 8) } + } + // Do we need to adjust the range? Imagine we had 0b0011_0100 and we + // unset the last active bit. + // ^ + // Then, we need to adjust our internal lastBit tracker to represent the + // ^ bit above. This means finding the first previous set bit. + if idx > -1 { + l := uint32(len(i.bitmap) - 1) // Imagine we had 0b0011_0100 and we unset the last active bit. // ^ // Then, we need to adjust our internal lastBit tracker to represent // the ^ bit above. This means finding the first previous set bit. j, ok := int(idx), false for ; j >= 0 && !ok; j-- { - _, ok = maxBitAfter(i.bitmap[loc], uint32(j)) + _, ok = maxBitAfter(i.bitmap[l], uint32(j)) } // We know from the earlier conditional that *some* bit is set, so // we know that j represents the index of the bit that's the new // "last active" bit. firstByte := i.rangeFirstBit() - byteOffset := 8 * uint32(len(i.bitmap)-1) - i.lastBit = firstByte + byteOffset + uint32(j) + 1 + i.lastBit = firstByte + (l * 8) + uint32(j) + 1 } } @@ -343,3 +345,24 @@ func (i *BitmapIndex) Buffer() *bytes.Buffer { func (i *BitmapIndex) Flush() []byte { return i.Buffer().Bytes() } + +// DebugCompare returns a string that compares this bitmap to another bitmap +// byte-by-byte in binary form as two columns. +func (i *BitmapIndex) DebugCompare(j *BitmapIndex) string { + output := make([]string, ordered.Max(len(i.bitmap), len(j.bitmap))) + for n := 0; n < len(output); n++ { + if n < len(i.bitmap) { + output[n] += fmt.Sprintf("%08b", i.bitmap[n]) + } else { + output[n] += " " + } + + output[n] += " | " + + if n < len(j.bitmap) { + output[n] += fmt.Sprintf("%08b", j.bitmap[n]) + } + } + + return strings.Join(output, "\n") +} diff --git a/exp/lighthorizon/index/types/bitmap_test.go b/exp/lighthorizon/index/types/bitmap_test.go index c5cfc3a04d..c5e7864872 100644 --- a/exp/lighthorizon/index/types/bitmap_test.go +++ b/exp/lighthorizon/index/types/bitmap_test.go @@ -3,6 +3,7 @@ package index import ( "fmt" "io" + "math/rand" "testing" "github.com/stretchr/testify/assert" @@ -194,6 +195,39 @@ func TestSetInactive(t *testing.T) { assert.Equal(t, []byte{0b0100_0000, 0b0010_0000}, index.bitmap) assert.EqualValues(t, 2, index.firstBit) assert.EqualValues(t, 2+9, index.lastBit) + + index.setInactive(1) // should be a no-op + assert.Equal(t, []byte{0b0100_0000, 0b0010_0000}, index.bitmap) + assert.EqualValues(t, 2, index.firstBit) + assert.EqualValues(t, 2+9, index.lastBit) +} + +// TestFuzzerSetInactive attempt to fuzz random bits into two bitmap sets, one +// by addition, and one by subtraction - then, it compares the outcome. +func TestFuzzySetUnset(t *testing.T) { + permLen := uint32(128) // should be a multiple of 8 + setBitsCount := permLen / 2 + + for n := 0; n < 10_000; n++ { + randBits := rand.Perm(int(permLen)) + setBits := randBits[:setBitsCount] + clearBits := randBits[setBitsCount:] + + // set all first, then clear the others + clearBitmap := &BitmapIndex{} + for i := uint32(1); i <= permLen; i++ { + clearBitmap.setActive(i) + } + + setBitmap := &BitmapIndex{} + for i := range setBits { + setBitmap.setActive(uint32(setBits[i]) + 1) + clearBitmap.setInactive(uint32(clearBits[i]) + 1) + } + + require.Equalf(t, setBitmap, clearBitmap, + "bitmaps aren't equal:\n%s", setBitmap.DebugCompare(clearBitmap)) + } } func TestNextActive(t *testing.T) { From 8595b1a8a71c1d639bc377ccafa0b6a6cf3a24b4 Mon Sep 17 00:00:00 2001 From: George Kudrayvtsev Date: Mon, 26 Sep 2022 14:06:37 -0700 Subject: [PATCH 5/5] Move helpers below all BitmapIndex methods --- exp/lighthorizon/index/types/bitmap.go | 45 +++++++++++++------------- 1 file changed, 22 insertions(+), 23 deletions(-) diff --git a/exp/lighthorizon/index/types/bitmap.go b/exp/lighthorizon/index/types/bitmap.go index 8586f0e335..171115938b 100644 --- a/exp/lighthorizon/index/types/bitmap.go +++ b/exp/lighthorizon/index/types/bitmap.go @@ -127,12 +127,6 @@ func (i *BitmapIndex) setActive(index uint32) error { return nil } -// distance returns how many bytes occur between the two given indices. Note -// that j >= i, otherwise the result will be negative. -func distance(i, j uint32) int { - return (int(j)-1)/8 - (int(i)-1)/8 -} - func (i *BitmapIndex) setInactive(index uint32) error { // Is this index even active in the first place? if i.firstBit == 0 || index < i.rangeFirstBit() || index > i.rangeLastBit() { @@ -156,8 +150,7 @@ func (i *BitmapIndex) setInactive(index uint32) error { return err } else { // Trim all (now-)empty bytes off the front. - diff := distance(i.firstBit, nextBit) - i.bitmap = i.bitmap[diff:] + i.bitmap = i.bitmap[distance(i.firstBit, nextBit):] i.firstBit = nextBit } } else if int(loc) == len(i.bitmap)-1 { @@ -303,21 +296,6 @@ func (i *BitmapIndex) nextActiveBit(position uint32) (uint32, error) { return 0, io.EOF } -func maxBitAfter(b byte, after uint32) (uint32, bool) { - if b == 0 { - // empty byte - return 0, false - } - - for shift := uint32(after); shift < 8; shift++ { - mask := byte(0b1000_0000) >> shift - if mask&b != 0 { - return shift, true - } - } - return 0, false -} - func (i *BitmapIndex) ToXDR() xdr.BitmapIndex { i.mutex.RLock() defer i.mutex.RUnlock() @@ -366,3 +344,24 @@ func (i *BitmapIndex) DebugCompare(j *BitmapIndex) string { return strings.Join(output, "\n") } + +func maxBitAfter(b byte, after uint32) (uint32, bool) { + if b == 0 { + // empty byte + return 0, false + } + + for shift := uint32(after); shift < 8; shift++ { + mask := byte(0b1000_0000) >> shift + if mask&b != 0 { + return shift, true + } + } + return 0, false +} + +// distance returns how many bytes occur between the two given indices. Note +// that j >= i, otherwise the result will be negative. +func distance(i, j uint32) int { + return (int(j)-1)/8 - (int(i)-1)/8 +}