From 1cd3bbe735310433d542513f0b2683d23c2347a0 Mon Sep 17 00:00:00 2001 From: urvisavla Date: Fri, 6 Sep 2024 15:18:14 -0700 Subject: [PATCH] services/horizon: Limit sql queries for history endpoints to retention window (#5448) --- services/horizon/internal/actions/effects.go | 4 +- services/horizon/internal/actions/helpers.go | 55 +++--- .../horizon/internal/actions/helpers_test.go | 168 +++++++++++++++--- services/horizon/internal/actions/ledger.go | 4 +- .../horizon/internal/actions/operation.go | 4 +- services/horizon/internal/actions/trade.go | 9 +- .../horizon/internal/actions/transaction.go | 4 +- 7 files changed, 186 insertions(+), 62 deletions(-) diff --git a/services/horizon/internal/actions/effects.go b/services/horizon/internal/actions/effects.go index e04997aca5..d8620fc11f 100644 --- a/services/horizon/internal/actions/effects.go +++ b/services/horizon/internal/actions/effects.go @@ -51,12 +51,12 @@ type GetEffectsHandler struct { } func (handler GetEffectsHandler) GetResourcePage(w HeaderWriter, r *http.Request) ([]hal.Pageable, error) { - pq, err := GetPageQuery(handler.LedgerState, r, DefaultTOID) + pq, err := GetPageQuery(handler.LedgerState, r) if err != nil { return nil, err } - err = validateCursorWithinHistory(handler.LedgerState, pq) + err = validateAndAdjustCursor(handler.LedgerState, &pq) if err != nil { return nil, err } diff --git a/services/horizon/internal/actions/helpers.go b/services/horizon/internal/actions/helpers.go index 2575b13251..99071bfba7 100644 --- a/services/horizon/internal/actions/helpers.go +++ b/services/horizon/internal/actions/helpers.go @@ -3,6 +3,7 @@ package actions import ( "context" "encoding/hex" + "errors" "fmt" "net/http" "net/url" @@ -20,7 +21,6 @@ import ( "github.com/stellar/go/services/horizon/internal/db2" "github.com/stellar/go/services/horizon/internal/ledger" hProblem "github.com/stellar/go/services/horizon/internal/render/problem" - "github.com/stellar/go/support/errors" "github.com/stellar/go/support/ordered" "github.com/stellar/go/support/render/problem" "github.com/stellar/go/toid" @@ -45,8 +45,6 @@ type Opt int const ( // DisableCursorValidation disables cursor validation in GetPageQuery DisableCursorValidation Opt = iota - // DefaultTOID sets a default cursor value in GetPageQuery based on the ledger state - DefaultTOID Opt = iota ) // HeaderWriter is an interface for setting HTTP response headers @@ -171,7 +169,7 @@ func getLimit(r *http.Request, name string, def uint64, max uint64) (uint64, err if asI64 <= 0 { err = errors.New("invalid limit: non-positive value provided") } else if asI64 > int64(max) { - err = errors.Errorf("invalid limit: value provided that is over limit max of %d", max) + err = fmt.Errorf("invalid limit: value provided that is over limit max of %d", max) } if err != nil { @@ -185,14 +183,10 @@ func getLimit(r *http.Request, name string, def uint64, max uint64) (uint64, err // using the results from a call to GetPagingParams() func GetPageQuery(ledgerState *ledger.State, r *http.Request, opts ...Opt) (db2.PageQuery, error) { disableCursorValidation := false - defaultTOID := false for _, opt := range opts { if opt == DisableCursorValidation { disableCursorValidation = true } - if opt == DefaultTOID { - defaultTOID = true - } } cursor, err := getCursor(ledgerState, r, ParamCursor) @@ -221,13 +215,6 @@ func GetPageQuery(ledgerState *ledger.State, r *http.Request, opts ...Opt) (db2. return db2.PageQuery{}, err } - if cursor == "" && defaultTOID { - if pageQuery.Order == db2.OrderAscending { - pageQuery.Cursor = toid.AfterLedger( - ordered.Max(0, ledgerState.CurrentStatus().HistoryElder-1), - ).String() - } - } return pageQuery, nil } @@ -553,19 +540,37 @@ func validateAssetParams(aType, code, issuer, prefix string) error { return nil } -// validateCursorWithinHistory compares the requested page of data against the +// validateAndAdjustCursor compares the requested page of data against the // ledger state of the history database. In the event that the cursor is // guaranteed to return no results, we return a 410 GONE http response. -func validateCursorWithinHistory(ledgerState *ledger.State, pq db2.PageQuery) error { - // an ascending query should never return a gone response: An ascending query - // prior to known history should return results at the beginning of history, - // and an ascending query beyond the end of history should not error out but - // rather return an empty page (allowing code that tracks the procession of - // some resource more easily). - if pq.Order != "desc" { - return nil +// For ascending queries, we adjust the cursor to ensure it starts at +// the oldest available ledger. +func validateAndAdjustCursor(ledgerState *ledger.State, pq *db2.PageQuery) error { + err := validateCursorWithinHistory(ledgerState, *pq) + + if pq.Order == db2.OrderAscending { + // an ascending query should never return a gone response: An ascending query + // prior to known history should return results at the beginning of history, + // and an ascending query beyond the end of history should not error out but + // rather return an empty page (allowing code that tracks the procession of + // some resource more easily). + + // set/modify the cursor for ascending queries to start at the oldest available ledger if it + // precedes the oldest ledger. This avoids inefficient queries caused by index bloat from deleted rows + // that are removed as part of reaping to maintain the retention window. + if pq.Cursor == "" || errors.Is(err, &hProblem.BeforeHistory) { + pq.Cursor = toid.AfterLedger( + ordered.Max(0, ledgerState.CurrentStatus().HistoryElder-1), + ).String() + return nil + } } + return err +} +// validateCursorWithinHistory checks if the cursor is within the known history range. +// If the cursor is before the oldest available ledger, it returns BeforeHistory error. +func validateCursorWithinHistory(ledgerState *ledger.State, pq db2.PageQuery) error { var cursor int64 var err error @@ -596,7 +601,7 @@ func countNonEmpty(params ...interface{}) (int, error) { for _, param := range params { switch param := param.(type) { default: - return 0, errors.Errorf("unexpected type %T", param) + return 0, fmt.Errorf("unexpected type %T", param) case int32: if param != 0 { count++ diff --git a/services/horizon/internal/actions/helpers_test.go b/services/horizon/internal/actions/helpers_test.go index 05e840577e..e393cc357e 100644 --- a/services/horizon/internal/actions/helpers_test.go +++ b/services/horizon/internal/actions/helpers_test.go @@ -16,6 +16,7 @@ import ( horizonContext "github.com/stellar/go/services/horizon/internal/context" "github.com/stellar/go/services/horizon/internal/db2" "github.com/stellar/go/services/horizon/internal/ledger" + hProblem "github.com/stellar/go/services/horizon/internal/render/problem" "github.com/stellar/go/services/horizon/internal/test" "github.com/stellar/go/support/db" "github.com/stellar/go/support/errors" @@ -126,11 +127,21 @@ func TestValidateCursorWithinHistory(t *testing.T) { { cursor: "0", order: "asc", - valid: true, + valid: false, }, { cursor: "0-1234", order: "asc", + valid: false, + }, + { + cursor: "1", + order: "asc", + valid: true, + }, + { + cursor: "1-1234", + order: "asc", valid: true, }, } @@ -291,11 +302,10 @@ func TestGetPageQuery(t *testing.T) { tt.Assert.Error(err) } -func TestGetPageQueryCursorDefaultTOID(t *testing.T) { - ascReq := makeTestActionRequest("/foo-bar/blah?limit=2", testURLParams()) - descReq := makeTestActionRequest("/foo-bar/blah?limit=2&order=desc", testURLParams()) - +func TestPageQueryCursorDefaultOrder(t *testing.T) { ledgerState := &ledger.State{} + + // truncated history ledgerState.SetHorizonStatus(ledger.HorizonStatus{ HistoryLatest: 7000, HistoryLatestClosedAt: time.Now(), @@ -303,30 +313,30 @@ func TestGetPageQueryCursorDefaultTOID(t *testing.T) { ExpHistoryLatest: 7000, }) - pq, err := GetPageQuery(ledgerState, ascReq, DefaultTOID) + req := makeTestActionRequest("/foo-bar/blah?limit=2", testURLParams()) + + // default asc, w/o cursor + pq, err := GetPageQuery(ledgerState, req) assert.NoError(t, err) + assert.Empty(t, pq.Cursor) + assert.NoError(t, validateAndAdjustCursor(ledgerState, &pq)) assert.Equal(t, toid.AfterLedger(299).String(), pq.Cursor) assert.Equal(t, uint64(2), pq.Limit) assert.Equal(t, "asc", pq.Order) - pq, err = GetPageQuery(ledgerState, descReq, DefaultTOID) - assert.NoError(t, err) - assert.Equal(t, "", pq.Cursor) - assert.Equal(t, uint64(2), pq.Limit) - assert.Equal(t, "desc", pq.Order) + cursor := toid.AfterLedger(200).String() + reqWithCursor := makeTestActionRequest(fmt.Sprintf("/foo-bar/blah?cursor=%s&limit=2", cursor), testURLParams()) - pq, err = GetPageQuery(ledgerState, ascReq) + // default asc, w/ cursor + pq, err = GetPageQuery(ledgerState, reqWithCursor) assert.NoError(t, err) - assert.Empty(t, pq.Cursor) + assert.Equal(t, cursor, pq.Cursor) + assert.NoError(t, validateAndAdjustCursor(ledgerState, &pq)) + assert.Equal(t, toid.AfterLedger(299).String(), pq.Cursor) assert.Equal(t, uint64(2), pq.Limit) assert.Equal(t, "asc", pq.Order) - pq, err = GetPageQuery(ledgerState, descReq) - assert.NoError(t, err) - assert.Empty(t, pq.Cursor) - assert.Equal(t, "", pq.Cursor) - assert.Equal(t, "desc", pq.Order) - + // full history ledgerState.SetHorizonStatus(ledger.HorizonStatus{ HistoryLatest: 7000, HistoryLatestClosedAt: time.Now(), @@ -334,18 +344,130 @@ func TestGetPageQueryCursorDefaultTOID(t *testing.T) { ExpHistoryLatest: 7000, }) - pq, err = GetPageQuery(ledgerState, ascReq, DefaultTOID) + // default asc, w/o cursor + pq, err = GetPageQuery(ledgerState, req) assert.NoError(t, err) + assert.Empty(t, pq.Cursor) + assert.NoError(t, validateAndAdjustCursor(ledgerState, &pq)) assert.Equal(t, toid.AfterLedger(0).String(), pq.Cursor) assert.Equal(t, uint64(2), pq.Limit) assert.Equal(t, "asc", pq.Order) - pq, err = GetPageQuery(ledgerState, descReq, DefaultTOID) + // default asc, w/ cursor + pq, err = GetPageQuery(ledgerState, reqWithCursor) assert.NoError(t, err) - assert.Equal(t, "", pq.Cursor) + assert.Equal(t, cursor, pq.Cursor) + assert.NoError(t, validateAndAdjustCursor(ledgerState, &pq)) + assert.Equal(t, cursor, pq.Cursor) assert.Equal(t, uint64(2), pq.Limit) - assert.Equal(t, "desc", pq.Order) + assert.Equal(t, "asc", pq.Order) + +} + +func TestGetPageQueryWithoutCursor(t *testing.T) { + ledgerState := &ledger.State{} + + validateCursor := func(limit uint64, order string, expectedCursor string) { + req := makeTestActionRequest(fmt.Sprintf("/foo-bar/blah?limit=%d&order=%s", limit, order), testURLParams()) + pq, err := GetPageQuery(ledgerState, req) + assert.NoError(t, err) + assert.Empty(t, pq.Cursor) + assert.NoError(t, validateAndAdjustCursor(ledgerState, &pq)) + assert.Equal(t, expectedCursor, pq.Cursor) + assert.Equal(t, limit, pq.Limit) + assert.Equal(t, order, pq.Order) + } + + // truncated history + ledgerState.SetHorizonStatus(ledger.HorizonStatus{ + HistoryLatest: 7000, + HistoryLatestClosedAt: time.Now(), + HistoryElder: 300, + ExpHistoryLatest: 7000, + }) + + validateCursor(2, "asc", toid.AfterLedger(299).String()) + validateCursor(2, "desc", "") + + // full history + ledgerState.SetHorizonStatus(ledger.HorizonStatus{ + HistoryLatest: 7000, + HistoryLatestClosedAt: time.Now(), + HistoryElder: 0, + ExpHistoryLatest: 7000, + }) + + validateCursor(2, "asc", toid.AfterLedger(0).String()) + validateCursor(2, "desc", "") +} + +func TestValidateAndAdjustCursor(t *testing.T) { + ledgerState := &ledger.State{} + + validateCursor := func(cursor string, limit uint64, order string, expectedCursor string, expectedError error) { + pq := db2.PageQuery{Cursor: cursor, + Limit: limit, + Order: order, + } + err := validateAndAdjustCursor(ledgerState, &pq) + if expectedError != nil { + assert.EqualError(t, expectedError, err.Error()) + } else { + assert.NoError(t, err) + } + assert.Equal(t, expectedCursor, pq.Cursor) + assert.Equal(t, limit, pq.Limit) + assert.Equal(t, order, pq.Order) + } + + // full history + ledgerState.SetHorizonStatus(ledger.HorizonStatus{ + HistoryLatest: 7000, + HistoryLatestClosedAt: time.Now(), + HistoryElder: 0, + ExpHistoryLatest: 7000, + }) + + // invalid cursor + validateCursor("blah", 2, "asc", "blah", problem.BadRequest) + validateCursor("blah", 2, "desc", "blah", problem.BadRequest) + + validateCursor(toid.AfterLedger(0).String(), 2, "asc", toid.AfterLedger(0).String(), nil) + validateCursor(toid.AfterLedger(200).String(), 2, "asc", toid.AfterLedger(200).String(), nil) + validateCursor(toid.AfterLedger(7001).String(), 2, "asc", toid.AfterLedger(7001).String(), nil) + + validateCursor(toid.AfterLedger(0).String(), 2, "desc", toid.AfterLedger(0).String(), nil) + validateCursor(toid.AfterLedger(200).String(), 2, "desc", toid.AfterLedger(200).String(), nil) + validateCursor(toid.AfterLedger(7001).String(), 2, "desc", toid.AfterLedger(7001).String(), nil) + + // truncated history + ledgerState.SetHorizonStatus(ledger.HorizonStatus{ + HistoryLatest: 7000, + HistoryLatestClosedAt: time.Now(), + HistoryElder: 300, + ExpHistoryLatest: 7000, + }) + // invalid cursor + validateCursor("blah", 2, "asc", "blah", problem.BadRequest) + validateCursor("blah", 2, "desc", "blah", problem.BadRequest) + + // asc order + validateCursor(toid.AfterLedger(0).String(), 2, "asc", toid.AfterLedger(299).String(), nil) + validateCursor(toid.AfterLedger(200).String(), 2, "asc", toid.AfterLedger(299).String(), nil) + validateCursor(toid.AfterLedger(298).String(), 2, "asc", toid.AfterLedger(299).String(), nil) + validateCursor(toid.AfterLedger(299).String(), 2, "asc", toid.AfterLedger(299).String(), nil) + validateCursor(toid.AfterLedger(300).String(), 2, "asc", toid.AfterLedger(300).String(), nil) + validateCursor(toid.AfterLedger(301).String(), 2, "asc", toid.AfterLedger(301).String(), nil) + validateCursor(toid.AfterLedger(7001).String(), 2, "asc", toid.AfterLedger(7001).String(), nil) + + // desc order + validateCursor(toid.AfterLedger(0).String(), 2, "desc", toid.AfterLedger(0).String(), hProblem.BeforeHistory) + validateCursor(toid.AfterLedger(200).String(), 2, "desc", toid.AfterLedger(200).String(), hProblem.BeforeHistory) + validateCursor(toid.AfterLedger(299).String(), 2, "desc", toid.AfterLedger(299).String(), hProblem.BeforeHistory) + validateCursor(toid.AfterLedger(300).String(), 2, "desc", toid.AfterLedger(300).String(), nil) + validateCursor(toid.AfterLedger(320).String(), 2, "desc", toid.AfterLedger(320).String(), nil) + validateCursor(toid.AfterLedger(7001).String(), 2, "desc", toid.AfterLedger(7001).String(), nil) } func TestGetString(t *testing.T) { diff --git a/services/horizon/internal/actions/ledger.go b/services/horizon/internal/actions/ledger.go index 0b3d51b8e1..0836ba9b10 100644 --- a/services/horizon/internal/actions/ledger.go +++ b/services/horizon/internal/actions/ledger.go @@ -17,12 +17,12 @@ type GetLedgersHandler struct { } func (handler GetLedgersHandler) GetResourcePage(w HeaderWriter, r *http.Request) ([]hal.Pageable, error) { - pq, err := GetPageQuery(handler.LedgerState, r, DefaultTOID) + pq, err := GetPageQuery(handler.LedgerState, r) if err != nil { return nil, err } - err = validateCursorWithinHistory(handler.LedgerState, pq) + err = validateAndAdjustCursor(handler.LedgerState, &pq) if err != nil { return nil, err } diff --git a/services/horizon/internal/actions/operation.go b/services/horizon/internal/actions/operation.go index 9ccadb272e..6b200cba29 100644 --- a/services/horizon/internal/actions/operation.go +++ b/services/horizon/internal/actions/operation.go @@ -72,12 +72,12 @@ type GetOperationsHandler struct { func (handler GetOperationsHandler) GetResourcePage(w HeaderWriter, r *http.Request) ([]hal.Pageable, error) { ctx := r.Context() - pq, err := GetPageQuery(handler.LedgerState, r, DefaultTOID) + pq, err := GetPageQuery(handler.LedgerState, r) if err != nil { return nil, err } - err = validateCursorWithinHistory(handler.LedgerState, pq) + err = validateAndAdjustCursor(handler.LedgerState, &pq) if err != nil { return nil, err } diff --git a/services/horizon/internal/actions/trade.go b/services/horizon/internal/actions/trade.go index b29ad64715..57d97593cf 100644 --- a/services/horizon/internal/actions/trade.go +++ b/services/horizon/internal/actions/trade.go @@ -159,12 +159,12 @@ type GetTradesHandler struct { func (handler GetTradesHandler) GetResourcePage(w HeaderWriter, r *http.Request) ([]hal.Pageable, error) { ctx := r.Context() - pq, err := GetPageQuery(handler.LedgerState, r, DefaultTOID) + pq, err := GetPageQuery(handler.LedgerState, r) if err != nil { return nil, err } - err = validateCursorWithinHistory(handler.LedgerState, pq) + err = validateAndAdjustCursor(handler.LedgerState, &pq) if err != nil { return nil, err } @@ -287,10 +287,7 @@ func (handler GetTradeAggregationsHandler) GetResource(w HeaderWriter, r *http.R if err != nil { return nil, err } - err = validateCursorWithinHistory(handler.LedgerState, pq) - if err != nil { - return nil, err - } + qp := TradeAggregationsQuery{} if err = getParams(&qp, r); err != nil { return nil, err diff --git a/services/horizon/internal/actions/transaction.go b/services/horizon/internal/actions/transaction.go index 6a2fd1c6e2..ed579f9d6f 100644 --- a/services/horizon/internal/actions/transaction.go +++ b/services/horizon/internal/actions/transaction.go @@ -98,12 +98,12 @@ type GetTransactionsHandler struct { func (handler GetTransactionsHandler) GetResourcePage(w HeaderWriter, r *http.Request) ([]hal.Pageable, error) { ctx := r.Context() - pq, err := GetPageQuery(handler.LedgerState, r, DefaultTOID) + pq, err := GetPageQuery(handler.LedgerState, r) if err != nil { return nil, err } - err = validateCursorWithinHistory(handler.LedgerState, pq) + err = validateAndAdjustCursor(handler.LedgerState, &pq) if err != nil { return nil, err }