support/datastore: Make resumability robust to unexpected overlaps in adjacent ranges

support/datastore/resumablemanager_test.go

@@ -5,8 +5,9 @@
 	"testing"
 
 	"github.com/pkg/errors"
-	"github.com/stellar/go/historyarchive"
 	"github.com/stretchr/testify/require"
+
+	"github.com/stellar/go/historyarchive"
 )
 
 func TestResumability(t *testing.T) {
@@ -61,6 +62,42 @@
 			},
 			networkName: "test",
 		},
+		{
+			name:         "start and end ledger are in same file, data store does not have it",
+			startLedger:  64,
+			endLedger:    68,
+			absentLedger: 64,
+			findStartOk:  true,
+			ledgerBatchConfig: LedgerBatchConfig{
+				FilesPerPartition: uint32(100),
+				LedgersPerFile:    uint32(64),
+			},
+			networkName: "test",
+		},
+		{
+			name:         "start and end ledger are in same file, data store has it",
+			startLedger:  128,
+			endLedger:    130,
+			absentLedger: 0,
+			findStartOk:  false,
+			ledgerBatchConfig: LedgerBatchConfig{
+				FilesPerPartition: uint32(100),
+				LedgersPerFile:    uint32(64),
+			},
+			networkName: "test",
+		},
+		{
+			name:         "ledger range overlaps with an range which is already exported",
+			startLedger:  2,
+			endLedger:    127,
+			absentLedger: 2,
+			findStartOk:  true,
+			ledgerBatchConfig: LedgerBatchConfig{
+				FilesPerPartition: uint32(1000),
+				LedgersPerFile:    uint32(64),
+			},
+			networkName: "test",
+		},
 		{
 			name:         "binary search encounters an error during datastore retrieval",
 			startLedger:  24,
@@ -198,24 +235,36 @@
 	mockDataStore.On("Exists", ctx, "FFFFFFFF--0-9.xdr.zstd").Return(true, nil).Once()
 
 	//"End ledger same as start, data store does not have it"
-	mockDataStore.On("Exists", ctx, "FFFFFFF5--10-19.xdr.zstd").Return(false, nil).Once()
+	mockDataStore.On("Exists", ctx, "FFFFFFF5--10-19.xdr.zstd").Return(false, nil).Twice()
+
+	// start and end ledger are in same file, data store does not have it
+	mockDataStore.On("Exists", ctx, "FFFFFFFF--0-6399/FFFFFFBF--64-127.xdr.zstd").Return(false, nil).Twice()
+
+	// start and end ledger are in same file, data store has it
+	mockDataStore.On("Exists", ctx, "FFFFFFFF--0-6399/FFFFFF7F--128-191.xdr.zstd").Return(true, nil).Once()
+
+	// ledger range overlaps with an range which is already exported
+	mockDataStore.On("Exists", ctx, "FFFFFFFF--0-63999/FFFFFFBF--64-127.xdr.zstd").Return(true, nil).Once()
+	mockDataStore.On("Exists", ctx, "FFFFFFFF--0-63999/FFFFFFFF--0-63.xdr.zstd").Return(false, nil).Once()
 
 	//"binary search encounters an error during datastore retrieval",
 	mockDataStore.On("Exists", ctx, "FFFFFFEB--20-29.xdr.zstd").Return(false, errors.New("datastore error happened")).Once()
 
 	//"Data store is beyond boundary aligned start ledger"
+	mockDataStore.On("Exists", ctx, "FFFFFFCD--50-59.xdr.zstd").Return(false, nil).Once()
 	mockDataStore.On("Exists", ctx, "FFFFFFE1--30-39.xdr.zstd").Return(true, nil).Once()
 	mockDataStore.On("Exists", ctx, "FFFFFFD7--40-49.xdr.zstd").Return(false, nil).Once()
 
 	//"Data store is beyond non boundary aligned start ledger"
 	mockDataStore.On("Exists", ctx, "FFFFFFB9--70-79.xdr.zstd").Return(true, nil).Once()
-	mockDataStore.On("Exists", ctx, "FFFFFFAF--80-89.xdr.zstd").Return(false, nil).Once()
+	mockDataStore.On("Exists", ctx, "FFFFFFAF--80-89.xdr.zstd").Return(false, nil).Twice()
 
 	//"Data store is beyond start and end ledger"
 	mockDataStore.On("Exists", ctx, "FFFFFEFB--260-269.xdr.zstd").Return(true, nil).Once()
 	mockDataStore.On("Exists", ctx, "FFFFFEF1--270-279.xdr.zstd").Return(true, nil).Once()
 
 	//"Data store is not beyond start ledger"
+	mockDataStore.On("Exists", ctx, "FFFFFF87--120-129.xdr.zstd").Return(false, nil).Once()
 	mockDataStore.On("Exists", ctx, "FFFFFF91--110-119.xdr.zstd").Return(false, nil).Once()
 	mockDataStore.On("Exists", ctx, "FFFFFF9B--100-109.xdr.zstd").Return(false, nil).Once()
 	mockDataStore.On("Exists", ctx, "FFFFFFA5--90-99.xdr.zstd").Return(false, nil).Once()

support/datastore/resumeablemanager.go

@@ -5,6 +5,7 @@ import (
 	"sort"
 
 	"github.com/pkg/errors"
+
 	"github.com/stellar/go/historyarchive"
 	"github.com/stellar/go/support/log"
 )
@@ -77,6 +78,29 @@ func (rm resumableManagerService) FindStart(ctx context.Context, start, end uint
 			return 0, false, errors.Errorf("Invalid start value of %v, it is greater than network's latest ledger of %v", start, networkLatest)
 		}
 		end = networkLatest
+	} else if end >= rm.ledgerBatchConfig.LedgersPerFile {
+		// Adjacent ranges may end up overlapping due to the clamping behavior in adjustLedgerRange()
+		// https://github.com/stellar/go/blob/fff01229a5af77dee170a37bf0c71b2ce8bb8474/exp/services/ledgerexporter/internal/config.go#L173-L192
+		// For example, assuming 64 ledgers per file, [2, 100] and [101, 150] get adjusted to [2, 127] and [64, 191]
+		// If we export [64, 191] and then try to resume on [2, 127], the binary search logic will determine that
+		// [2, 127] is fully exported because the midpoint of [2, 127] is present.
+		// To fix this issue we query the end ledger and if it is present, we only do the binary search on the
+		// preceding sub range. This will allow resumability to work on adjacent ranges that end up overlapping
+		// due to adjustLedgerRange().
+		// Note that if there is an overlap the size of the overlap will never be larger than the number of files
+		// per partition and that is why it is sufficient to only check if the end ledger is present.
+		exists, err := rm.dataStore.Exists(ctx, rm.ledgerBatchConfig.GetObjectKeyFromSequenceNumber(end))
+		if err != nil {
+			return 0, false, err
+		}
+		if exists {
+			end -= rm.ledgerBatchConfig.LedgersPerFile
+			if start > end {
+				// data store had all ledgers for requested range, no resumability needed.
+				log.Infof("Resumability found no absent object keys in requested ledger range")
+				return 0, false, nil
+			}
+		}
 	}
 
 	rangeSize := max(int(end-start), 1)