archivist: range and scan fixes (#1735)

Fix a couple bugs with ranges in stellar-archivist. This just makes it behave more like it's documented to / how users would expect:

 - Treat the range values as inclusive rather than excluding the high endpoint.
 - Don't clobber the destination pointer when a mirror range doesn't end on that value.
 - Fix off-by-one in Range.Size() method
 - Tighten listing-driven scan precision, skip out-of-range checkpoints
 - Avoid listing all buckets when scanning a subrange.
 - Scan all buckets and emit error if missing root HAS

Author: graydon

support/historyarchive/archive_test.go

@@ -155,6 +155,46 @@ func TestScan(t *testing.T) {
 	GetRandomPopulatedArchive().Scan(opts)
 }
 
+func TestScanSize(t *testing.T) {
+	defer cleanup()
+	opts := testOptions()
+	arch := GetRandomPopulatedArchive()
+	arch.Scan(opts)
+	assert.Equal(t, opts.Range.Size(),
+		len(arch.checkpointFiles["history"]))
+}
+
+func TestScanSizeSubrange(t *testing.T) {
+	defer cleanup()
+	opts := testOptions()
+	arch := GetRandomPopulatedArchive()
+	opts.Range.Low = NextCheckpoint(opts.Range.Low)
+	opts.Range.High = PrevCheckpoint(opts.Range.High)
+	arch.Scan(opts)
+	assert.Equal(t, opts.Range.Size(),
+		len(arch.checkpointFiles["history"]))
+}
+
+func TestScanSizeSubrangeFewBuckets(t *testing.T) {
+	defer cleanup()
+	opts := testOptions()
+	arch := GetRandomPopulatedArchive()
+	opts.Range.Low = 0x1ff
+	opts.Range.High = 0x1ff
+	arch.Scan(opts)
+	// We should only scan one checkpoint worth of buckets.
+	assert.Less(t, len(arch.allBuckets), 40)
+}
+
+func TestScanSizeSubrangeAllBuckets(t *testing.T) {
+	defer cleanup()
+	opts := testOptions()
+	arch := GetRandomPopulatedArchive()
+	arch.Scan(opts)
+	// We should scan all checkpoints worth of buckets.
+	assert.Less(t, 300, len(arch.allBuckets))
+}
+
 func countMissing(arch *Archive, opts *CommandOptions) int {
 	n := 0
 	arch.Scan(opts)
@@ -200,6 +240,43 @@ func TestMirrorThenRepair(t *testing.T) {
 	assert.Equal(t, 0, countMissing(dst, opts))
 }
 
+func (a *Archive) MustGetRootHAS() HistoryArchiveState {
+	has, e := a.GetRootHAS()
+	if e != nil {
+		panic("failed to get root HAS")
+	}
+	return has
+}
+
+func TestMirrorSubsetDoPointerUpdate(t *testing.T) {
+	defer cleanup()
+	opts := testOptions()
+	src := GetRandomPopulatedArchive()
+	dst := GetTestArchive()
+	Mirror(src, dst, opts)
+	oldHigh := opts.Range.High
+	assert.Equal(t, oldHigh, dst.MustGetRootHAS().CurrentLedger)
+	opts.Range.High = NextCheckpoint(oldHigh)
+	src.AddRandomCheckpoint(opts.Range.High)
+	Mirror(src, dst, opts)
+	assert.Equal(t, opts.Range.High, dst.MustGetRootHAS().CurrentLedger)
+}
+
+func TestMirrorSubsetNoPointerUpdate(t *testing.T) {
+	defer cleanup()
+	opts := testOptions()
+	src := GetRandomPopulatedArchive()
+	dst := GetTestArchive()
+	Mirror(src, dst, opts)
+	oldHigh := opts.Range.High
+	assert.Equal(t, oldHigh, dst.MustGetRootHAS().CurrentLedger)
+	src.AddRandomCheckpoint(NextCheckpoint(oldHigh))
+	opts.Range.Low = 0x7f
+	opts.Range.High = 0xff
+	Mirror(src, dst, opts)
+	assert.Equal(t, oldHigh, dst.MustGetRootHAS().CurrentLedger)
+}
+
 func TestDryRunNoRepair(t *testing.T) {
 	defer cleanup()
 	opts := testOptions()

support/historyarchive/mirror.go

@@ -93,8 +93,20 @@ func Mirror(src *Archive, dst *Archive, opts *CommandOptions) error {
 	log.Printf("copied %d checkpoints, %d buckets, range %s",
 		opts.Range.Size(), len(bucketFetch), opts.Range)
 	close(tick)
-	e = dst.PutRootHAS(rootHAS, opts)
-	errs += noteError(e)
+	if rootHAS.CurrentLedger == opts.Range.High {
+		log.Printf("updating destination archive current-ledger pointer to 0x%8.8x",
+			rootHAS.CurrentLedger)
+		e = dst.PutRootHAS(rootHAS, opts)
+		errs += noteError(e)
+	} else {
+		dstHAS, e := dst.GetRootHAS()
+		if e != nil {
+			errs += noteError(e)
+		} else {
+			log.Printf("leaving destination archive current-ledger pointer at 0x%8.8x",
+				dstHAS.CurrentLedger)
+		}
+	}
 	if errs != 0 {
 		return fmt.Errorf("%d errors while mirroring", errs)
 	}

support/historyarchive/range.go

@@ -67,7 +67,7 @@ func (r Range) String() string {
 func (r Range) Checkpoints() chan uint32 {
 	ch := make(chan uint32)
 	go func() {
-		for i := uint64(r.Low); i < uint64(r.High); i += uint64(CheckpointFreq) {
+		for i := uint64(r.Low); i <= uint64(r.High); i += uint64(CheckpointFreq) {
 			ch <- uint32(i)
 		}
 		close(ch)
@@ -76,7 +76,7 @@ func (r Range) Checkpoints() chan uint32 {
 }
 
 func (r Range) Size() int {
-	return int(r.High-r.Low) / int(CheckpointFreq)
+	return 1 + (int(r.High-r.Low) / int(CheckpointFreq))
 }
 
 func (r Range) collapsedString() string {

support/historyarchive/range_test.go

@@ -10,6 +10,50 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
+func (r Range) allCheckpoints() []uint32 {
+	var s []uint32
+	for chk := range r.Checkpoints() {
+		s = append(s, chk)
+	}
+	return s
+}
+
+func TestRangeSize(t *testing.T) {
+	assert.Equal(t, 1,
+		MakeRange(0x3f, 0x3f).Size())
+
+	assert.Equal(t, 2,
+		MakeRange(0x3f, 0x7f).Size())
+
+	assert.Equal(t, 2,
+		MakeRange(0, 100).Size())
+
+	assert.Equal(t, 4,
+		MakeRange(0xff3f, 0xffff).Size())
+}
+
+func TestRangeEnumeration(t *testing.T) {
+	assert.Equal(t,
+		[]uint32{0x3f, 0x7f},
+		MakeRange(0x3f, 0x7f).allCheckpoints())
+
+	assert.Equal(t,
+		[]uint32{0x3f},
+		MakeRange(0x3f, 0x3f).allCheckpoints())
+
+	assert.Equal(t,
+		[]uint32{0x3f},
+		MakeRange(0, 0).allCheckpoints())
+
+	assert.Equal(t,
+		[]uint32{0x3f, 0x7f},
+		MakeRange(0, 0x40).allCheckpoints())
+
+	assert.Equal(t,
+		[]uint32{0xff},
+		MakeRange(0xff, 0x40).allCheckpoints())
+}
+
 func TestFmtRangeList(t *testing.T) {
 
 	assert.Equal(t,

support/historyarchive/scan.go

@@ -133,6 +133,9 @@ func (arch *Archive) ScanCheckpointsFast(opts *CommandOptions) error {
 				}
 				ch, es := arch.ListCategoryCheckpoints(r.category, r.pathprefix)
 				for n := range ch {
+					if n < opts.Range.Low || n > opts.Range.High {
+						continue
+					}
 					tick <- true
 					arch.NoteCheckpointFile(r.category, n, true)
 					if opts.Verify {
@@ -197,11 +200,22 @@ func (arch *Archive) ScanBuckets(opts *CommandOptions) error {
 
 	var errs uint32
 
-	// First scan _all_ buckets if we can; if not, we'll do an exists-check
-	// on each bucket as we go. But this is faster when we can do it.
+	// First scan _all_ buckets if we can (and should -- if asked to look at the
+	// entire range); if not, we'll do an exists-check on each bucket as we
+	// go. But this is faster when we can do it.
 	doList := arch.backend.CanListFiles()
+	has, err := arch.GetRootHAS()
+	if err == nil {
+		fullRange := MakeRange(0, has.CurrentLedger)
+		doList = doList && opts.Range.Size() == fullRange.Size()
+	} else {
+		log.Print("Error retrieving root archive state, possibly corrupt archive:", err)
+		log.Print("Continuing and will do an exists-check on each bucket as we go, this will be slower")
+	}
 	if doList {
 		errs += noteError(arch.ScanAllBuckets())
+	} else {
+		log.Printf("Scanning buckets for %d checkpoints", opts.Range.Size())
 	}
 
 	// Grab the set of checkpoints we have HASs for, to read references.