Skip to content

Commit

Permalink
Code review fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
@tamirms
tamirms committed Nov 9, 2021
1 parent b02ecdb commit b236bcd
Show file tree
Hide file tree
Showing 10 changed files with 131 additions and 134 deletions.
21 changes: 11 additions & 10 deletions services/horizon/cmd/db.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,12 @@ var dbReingestRangeCmd = &cobra.Command{
if err != nil {
return err
}
return runDBReingestRange([][2]uint32{{argsUInt32[0], argsUInt32[1]}}, reingestForce, parallelWorkers, *config)
return runDBReingestRange(
[]history.LedgerRange{{StartSequence: argsUInt32[0], EndSequence: argsUInt32[1]}},
reingestForce,
parallelWorkers,
*config,
)
},
}

Expand Down Expand Up @@ -344,7 +349,7 @@ var dbFillGapsCmd = &cobra.Command{
if err != nil {
return err
}
var gaps []history.LedgerGap
var gaps []history.LedgerRange
if withRange {
gaps, err = runDBDetectGapsInRange(*config, uint32(start), uint32(end))
if err != nil {
Expand All @@ -359,15 +364,11 @@ var dbFillGapsCmd = &cobra.Command{
hlog.Infof("found gaps %v", gaps)
}

var ledgerRanges [][2]uint32
for _, gap := range gaps {
ledgerRanges = append(ledgerRanges, [2]uint32{gap.StartSequence, gap.EndSequence})
}
return runDBReingestRange(ledgerRanges, reingestForce, parallelWorkers, *config)
return runDBReingestRange(gaps, reingestForce, parallelWorkers, *config)
},
}

func runDBReingestRange(ledgerRanges [][2]uint32, reingestForce bool, parallelWorkers uint, config horizon.Config) error {
func runDBReingestRange(ledgerRanges []history.LedgerRange, reingestForce bool, parallelWorkers uint, config horizon.Config) error {
if reingestForce && parallelWorkers > 1 {
return errors.New("--force is incompatible with --parallel-workers > 1")
}
Expand Down Expand Up @@ -466,7 +467,7 @@ var dbDetectGapsCmd = &cobra.Command{
},
}

func runDBDetectGaps(config horizon.Config) ([]history.LedgerGap, error) {
func runDBDetectGaps(config horizon.Config) ([]history.LedgerRange, error) {
horizonSession, err := db.Open("postgres", config.DatabaseURL)
if err != nil {
return nil, err
Expand All @@ -475,7 +476,7 @@ func runDBDetectGaps(config horizon.Config) ([]history.LedgerGap, error) {
return q.GetLedgerGaps(context.Background())
}

func runDBDetectGapsInRange(config horizon.Config, start, end uint32) ([]history.LedgerGap, error) {
func runDBDetectGapsInRange(config horizon.Config, start, end uint32) ([]history.LedgerRange, error) {
horizonSession, err := db.Open("postgres", config.DatabaseURL)
if err != nil {
return nil, err
Expand Down
27 changes: 17 additions & 10 deletions services/horizon/internal/db2/history/ledger.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"encoding/hex"
"fmt"
"sort"
"time"

sq "github.com/Masterminds/squirrel"
Expand Down Expand Up @@ -132,11 +133,11 @@ func (q *Q) InsertLedger(ctx context.Context,

// GetLedgerGaps obtains ingestion gaps in the history_ledgers table.
// Returns the gaps and error.
func (q *Q) GetLedgerGaps(ctx context.Context) ([]LedgerGap, error) {
var gaps []LedgerGap
func (q *Q) GetLedgerGaps(ctx context.Context) ([]LedgerRange, error) {
var gaps []LedgerRange
query := `
SELECT sequence + 1 AS gap_start,
next_number - 1 AS gap_end
SELECT sequence + 1 AS start,
next_number - 1 AS end
FROM (
SELECT sequence,
LEAD(sequence) OVER (ORDER BY sequence) AS next_number
Expand All @@ -146,6 +147,9 @@ func (q *Q) GetLedgerGaps(ctx context.Context) ([]LedgerGap, error) {
if err := q.SelectRaw(ctx, &gaps, query); err != nil {
return nil, err
}
sort.Slice(gaps, func(i, j int) bool {
return gaps[i].StartSequence < gaps[j].StartSequence
})
return gaps, nil
}

Expand All @@ -165,14 +169,14 @@ func min(a, b uint32) uint32 {

// GetLedgerGapsInRange obtains ingestion gaps in the history_ledgers table within the given range.
// Returns the gaps and error.
func (q *Q) GetLedgerGapsInRange(ctx context.Context, start, end uint32) ([]LedgerGap, error) {
var result []LedgerGap
func (q *Q) GetLedgerGapsInRange(ctx context.Context, start, end uint32) ([]LedgerRange, error) {
var result []LedgerRange
var oldestLedger, latestLedger uint32

if err := q.ElderLedger(ctx, &oldestLedger); err != nil {
return nil, errors.Wrap(err, "Could not query elder ledger")
} else if oldestLedger == 0 {
return []LedgerGap{{
return []LedgerRange{{
StartSequence: start,
EndSequence: end,
}}, nil
Expand All @@ -183,11 +187,14 @@ func (q *Q) GetLedgerGapsInRange(ctx context.Context, start, end uint32) ([]Ledg
}

if start < oldestLedger {
result = append(result, LedgerGap{
result = append(result, LedgerRange{
StartSequence: start,
EndSequence: min(end, oldestLedger-1),
})
}
if end <= oldestLedger {
return result, nil
}

gaps, err := q.GetLedgerGaps(ctx)
if err != nil {
Expand All @@ -201,14 +208,14 @@ func (q *Q) GetLedgerGapsInRange(ctx context.Context, start, end uint32) ([]Ledg
if gap.StartSequence > end {
break
}
result = append(result, LedgerGap{
result = append(result, LedgerRange{
StartSequence: max(gap.StartSequence, start),
EndSequence: min(gap.EndSequence, end),
})
}

if latestLedger < end {
result = append(result, LedgerGap{
result = append(result, LedgerRange{
StartSequence: max(latestLedger+1, start),
EndSequence: end,
})
Expand Down
38 changes: 19 additions & 19 deletions services/horizon/internal/db2/history/ledger_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ func TestGetLedgerGaps(t *testing.T) {

gaps, err = q.GetLedgerGapsInRange(context.Background(), 1, 100)
tt.Assert.NoError(err)
tt.Assert.Equal([]LedgerGap{{StartSequence: 1, EndSequence: 100}}, gaps)
tt.Assert.Equal([]LedgerRange{{StartSequence: 1, EndSequence: 100}}, gaps)

// Lets insert a few gaps and make sure they are detected incrementally
insertLedgerWithSequence(tt, q, 4)
Expand All @@ -252,41 +252,41 @@ func TestGetLedgerGaps(t *testing.T) {

gaps, err = q.GetLedgerGapsInRange(context.Background(), 1, 2)
tt.Assert.NoError(err)
tt.Assert.Equal([]LedgerGap{{StartSequence: 1, EndSequence: 2}}, gaps)
tt.Assert.Equal([]LedgerRange{{StartSequence: 1, EndSequence: 2}}, gaps)

gaps, err = q.GetLedgerGapsInRange(context.Background(), 1, 3)
tt.Assert.NoError(err)
tt.Assert.Equal([]LedgerGap{{StartSequence: 1, EndSequence: 3}}, gaps)
tt.Assert.Equal([]LedgerRange{{StartSequence: 1, EndSequence: 3}}, gaps)

gaps, err = q.GetLedgerGapsInRange(context.Background(), 1, 6)
tt.Assert.NoError(err)
tt.Assert.Equal([]LedgerGap{{StartSequence: 1, EndSequence: 3}}, gaps)
tt.Assert.Equal([]LedgerRange{{StartSequence: 1, EndSequence: 3}}, gaps)

gaps, err = q.GetLedgerGapsInRange(context.Background(), 3, 5)
tt.Assert.NoError(err)
tt.Assert.Equal([]LedgerGap{{StartSequence: 3, EndSequence: 3}}, gaps)
tt.Assert.Equal([]LedgerRange{{StartSequence: 3, EndSequence: 3}}, gaps)

gaps, err = q.GetLedgerGapsInRange(context.Background(), 4, 6)
tt.Assert.NoError(err)
tt.Assert.Len(gaps, 0)

gaps, err = q.GetLedgerGapsInRange(context.Background(), 4, 8)
tt.Assert.NoError(err)
tt.Assert.Equal([]LedgerGap{{StartSequence: 8, EndSequence: 8}}, gaps)
tt.Assert.Equal([]LedgerRange{{StartSequence: 8, EndSequence: 8}}, gaps)

gaps, err = q.GetLedgerGapsInRange(context.Background(), 4, 10)
tt.Assert.NoError(err)
tt.Assert.Equal([]LedgerGap{{StartSequence: 8, EndSequence: 10}}, gaps)
tt.Assert.Equal([]LedgerRange{{StartSequence: 8, EndSequence: 10}}, gaps)

gaps, err = q.GetLedgerGapsInRange(context.Background(), 8, 10)
tt.Assert.NoError(err)
tt.Assert.Equal([]LedgerGap{{StartSequence: 8, EndSequence: 10}}, gaps)
tt.Assert.Equal([]LedgerRange{{StartSequence: 8, EndSequence: 10}}, gaps)

gaps, err = q.GetLedgerGapsInRange(context.Background(), 9, 11)
tt.Assert.NoError(err)
tt.Assert.Equal([]LedgerGap{{StartSequence: 9, EndSequence: 11}}, gaps)
tt.Assert.Equal([]LedgerRange{{StartSequence: 9, EndSequence: 11}}, gaps)

var expectedGaps []LedgerGap
var expectedGaps []LedgerRange

insertLedgerWithSequence(tt, q, 99)
insertLedgerWithSequence(tt, q, 100)
Expand All @@ -295,45 +295,45 @@ func TestGetLedgerGaps(t *testing.T) {

gaps, err = q.GetLedgerGaps(context.Background())
tt.Assert.NoError(err)
expectedGaps = append(expectedGaps, LedgerGap{8, 98})
expectedGaps = append(expectedGaps, LedgerRange{8, 98})
tt.Assert.Equal(expectedGaps, gaps)

gaps, err = q.GetLedgerGapsInRange(context.Background(), 10, 11)
tt.Assert.NoError(err)
tt.Assert.Equal([]LedgerGap{{StartSequence: 10, EndSequence: 11}}, gaps)
tt.Assert.Equal([]LedgerRange{{StartSequence: 10, EndSequence: 11}}, gaps)

gaps, err = q.GetLedgerGapsInRange(context.Background(), 4, 11)
tt.Assert.NoError(err)
tt.Assert.Equal([]LedgerGap{{StartSequence: 8, EndSequence: 11}}, gaps)
tt.Assert.Equal([]LedgerRange{{StartSequence: 8, EndSequence: 11}}, gaps)

gaps, err = q.GetLedgerGapsInRange(context.Background(), 1, 11)
tt.Assert.NoError(err)
tt.Assert.Equal([]LedgerGap{{StartSequence: 1, EndSequence: 3}, {StartSequence: 8, EndSequence: 11}}, gaps)
tt.Assert.Equal([]LedgerRange{{StartSequence: 1, EndSequence: 3}, {StartSequence: 8, EndSequence: 11}}, gaps)

gaps, err = q.GetLedgerGapsInRange(context.Background(), 10, 105)
tt.Assert.NoError(err)
tt.Assert.Equal([]LedgerGap{{StartSequence: 10, EndSequence: 98}, {StartSequence: 103, EndSequence: 105}}, gaps)
tt.Assert.Equal([]LedgerRange{{StartSequence: 10, EndSequence: 98}, {StartSequence: 103, EndSequence: 105}}, gaps)

gaps, err = q.GetLedgerGapsInRange(context.Background(), 100, 105)
tt.Assert.NoError(err)
tt.Assert.Equal([]LedgerGap{{StartSequence: 103, EndSequence: 105}}, gaps)
tt.Assert.Equal([]LedgerRange{{StartSequence: 103, EndSequence: 105}}, gaps)

gaps, err = q.GetLedgerGapsInRange(context.Background(), 105, 110)
tt.Assert.NoError(err)
tt.Assert.Equal([]LedgerGap{{StartSequence: 105, EndSequence: 110}}, gaps)
tt.Assert.Equal([]LedgerRange{{StartSequence: 105, EndSequence: 110}}, gaps)

// Yet another gap, this time to a single-ledger cluster
insertLedgerWithSequence(tt, q, 1000)

gaps, err = q.GetLedgerGaps(context.Background())
tt.Assert.NoError(err)
expectedGaps = append(expectedGaps, LedgerGap{103, 999})
expectedGaps = append(expectedGaps, LedgerRange{103, 999})
tt.Assert.Equal(expectedGaps, gaps)

// Yet another gap, this time the gap only contains a ledger
insertLedgerWithSequence(tt, q, 1002)
gaps, err = q.GetLedgerGaps(context.Background())
tt.Assert.NoError(err)
expectedGaps = append(expectedGaps, LedgerGap{1001, 1001})
expectedGaps = append(expectedGaps, LedgerRange{1001, 1001})
tt.Assert.Equal(expectedGaps, gaps)
}
6 changes: 3 additions & 3 deletions services/horizon/internal/db2/history/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -576,9 +576,9 @@ type LedgerCache struct {
queued map[int32]struct{}
}

type LedgerGap struct {
StartSequence uint32 `db:"gap_start"`
EndSequence uint32 `db:"gap_end"`
type LedgerRange struct {
StartSequence uint32 `db:"start"`
EndSequence uint32 `db:"end"`
}

// LedgersQ is a helper struct to aid in configuring queries that loads
Expand Down
Loading

0 comments on commit b236bcd

Please sign in to comment.