Repository: stellar/go
Pull request #5325 (2024-05-29 23:42:16)

exp/services/ledgerexporter/internal: Fix adjustLedgerRange()
<!-- If you're making a doc PR or something tiny where the below is irrelevant, delete this
template and use a short description, but in your description aim to include both what the
change is, and why it is being made, with enough context for anyone to understand. -->

<details>
  <summary>PR Checklist</summary>
  
### PR Structure

* [ ] This PR has reasonably narrow scope (if not, break it down into smaller PRs).
* [ ] This PR avoids mixing refactoring changes with feature changes (split into two PRs
  otherwise).
* [ ] This PR's title starts with name of package that is most changed in the PR, ex.
  `services/friendbot`, or `all` or `doc` if the changes are broad or impact many
  packages.

### Thoroughness

* [ ] This PR adds tests for the most critical parts of the new functionality or fixes.
* [ ] I've updated any docs ([developer docs](https://developers.stellar.org/api/), `.md`
  files, etc... affected by this change). Take a look in the `docs` folder for a given service,
  like [this one](https://github.com/stellar/go/tree/master/services/horizon/internal/docs).

### Release planning

* [ ] I've updated the relevant CHANGELOG ([here](services/horizon/CHANGELOG.md) for Horizon) if
  needed with deprecations, added features, breaking changes, and DB schema changes.
* [ ] I've decided if this PR requires a new major/minor version according to
  [semver](https://semver.org/), or if it's mainly a patch change. The PR is targeted at the next
  release branch if it's not a patch change.
</details>

### What

`adjustLedgerRange()` attempts to clamp the start and end ledger for a given ledger range so that they align on the ledgers per file boundary. However, there is a bug in the implementation because the end ledger should be aligned to the last ledger in the file.

For example, if we attempt to export ledgers in the range of [1, 50] assuming 64 ledgers per file, the adjusted range should be [2, 63] based on this logic:

https://github.com/stellar/go/blob/master/exp/services/ledgerexporter/internal/exportmanager.go#L54-L64

The current code instead adjusts the range to [2, 64] which is incorrect because 64 is the start ledger of the next ledger file

### Known limitations

[N/A]



Patch:
diff --git a/exp/services/ledgerexporter/internal/config.go b/exp/services/ledgerexporter/internal/config.go
index 2f6589a7..bab184bd 100644
--- a/exp/services/ledgerexporter/internal/config.go
+++ b/exp/services/ledgerexporter/internal/config.go
@@ -182,10 +182,7 @@ func (config *Config) adjustLedgerRange() {
 
 	// Align the end ledger (for bounded cases) to the nearest "LedgersPerFile" boundary.
 	if config.EndLedger != 0 {
-		// Add an extra batch only if "LedgersPerFile" is greater than 1 and the end ledger doesn't fall on the boundary.
-		if config.LedgerBatchConfig.LedgersPerFile > 1 && config.EndLedger%config.LedgerBatchConfig.LedgersPerFile != 0 {
-			config.EndLedger = (config.EndLedger/config.LedgerBatchConfig.LedgersPerFile + 1) * config.LedgerBatchConfig.LedgersPerFile
-		}
+		config.EndLedger = config.LedgerBatchConfig.GetSequenceNumberEndBoundary(config.EndLedger)
 	}
 
 	logger.Infof("Computed effective export boundary ledger range: start=%d, end=%d", config.StartLedger, config.EndLedger)


Test patch:
diff --git a/exp/services/ledgerexporter/internal/config_test.go b/exp/services/ledgerexporter/internal/config_test.go
index 86f6cfb5..13316507 100644
--- a/exp/services/ledgerexporter/internal/config_test.go
+++ b/exp/services/ledgerexporter/internal/config_test.go
@@ -5,8 +5,9 @@ import (
 	"fmt"
 	"testing"
 
-	"github.com/stellar/go/historyarchive"
 	"github.com/stretchr/testify/require"
+
+	"github.com/stellar/go/historyarchive"
 )
 
 func TestNewConfigResumeEnabled(t *testing.T) {
@@ -154,7 +155,7 @@ func TestAdjustedLedgerRangeBoundedMode(t *testing.T) {
 			start:         0,
 			end:           1,
 			expectedStart: 2,
-			expectedEnd:   10,
+			expectedEnd:   9,
 		},
 		{
 			name:          "Round down start ledger and round up end ledger, 15 ledgers per file ",
@@ -162,7 +163,7 @@ func TestAdjustedLedgerRangeBoundedMode(t *testing.T) {
 			start:         4,
 			end:           10,
 			expectedStart: 2,
-			expectedEnd:   15,
+			expectedEnd:   14,
 		},
 		{
 			name:          "Round down start ledger and round up end ledger, 64 ledgers per file ",
@@ -170,7 +171,7 @@ func TestAdjustedLedgerRangeBoundedMode(t *testing.T) {
 			start:         400,
 			end:           500,
 			expectedStart: 384,
-			expectedEnd:   512,
+			expectedEnd:   511,
 		},
 		{
 			name:          "No change, 64 ledger per file",
@@ -178,7 +179,7 @@ func TestAdjustedLedgerRangeBoundedMode(t *testing.T) {
 			start:         64,
 			end:           128,
 			expectedStart: 64,
-			expectedEnd:   128,
+			expectedEnd:   191,
 		},
 	}