Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

services/horizon/internal/db2: Add column and index to improve trade_type filter performance #4149

Merged
merged 6 commits into from
Dec 21, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions services/horizon/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,15 @@ file. This project adheres to [Semantic Versioning](http://semver.org/).

## Unreleased

* Generate Http Status code of 499 for Client Disconnects, should propagate into `horizon_http_requests_duration_seconds_count`
metric key with status=499 label. ([4098](horizon_http_requests_duration_seconds_count))
* Improve performance of `/trades?trade_type=liquidity_pool` requests. ([4149](https://github.com/stellar/go/pull/4149))
* Added `absBeforeEpoch` to ClaimableBalance API Resources. It will contain the Unix epoch representation of absolute before date. ([4148](https://github.com/stellar/go/pull/4148))

### DB Schema Migration

* DB migrations add a column and index to the `history_trades` table. This is very large table so migration may take a long time (depending on your DB hardware). Please test the migrations execution time on the copy of your production DB first.

## v2.12.1

### Fixes
Expand Down
11 changes: 8 additions & 3 deletions services/horizon/internal/actions_trade_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -315,10 +315,13 @@ func assertResponseTradeEqualsDBTrade(ht *HTTPT, row history.Trade, record horiz
ht.Assert.Equal(row.PriceN.Int64, record.Price.N)
ht.Assert.Equal(row.PriceD.Int64, record.Price.D)

if row.BaseLiquidityPoolID.Valid || row.CounterLiquidityPoolID.Valid {
ht.Assert.Equal(history.LiquidityPoolTrades, record.TradeType)
} else {
switch row.Type {
case history.OrderbookTradeType:
ht.Assert.Equal(history.OrderbookTrades, record.TradeType)
case history.LiquidityPoolTradeType:
ht.Assert.Equal(history.LiquidityPoolTrades, record.TradeType)
default:
ht.Assert.Fail("invalid trade type %v", row.Type)
}
}

Expand Down Expand Up @@ -833,6 +836,8 @@ func IngestTestTrade(
PriceN: int64(price.N),
PriceD: int64(price.D),
LedgerCloseTime: timestamp.ToTime(),

Type: history.OrderbookTradeType,
})
err = batch.Exec(ctx)
if err != nil {
Expand Down
1 change: 1 addition & 0 deletions services/horizon/internal/db2/history/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -704,6 +704,7 @@ type Trade struct {
BaseIsSeller bool `db:"base_is_seller"`
PriceN null.Int `db:"price_n"`
PriceD null.Int `db:"price_d"`
Type TradeType `db:"trade_type"`
}

// Transaction is a row of data from the `history_transactions` table
Expand Down
6 changes: 4 additions & 2 deletions services/horizon/internal/db2/history/trade.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,9 +171,9 @@ func createTradesSQL(page db2.PageQuery, query historyTradesQuery) (string, []in

switch query.tradeType {
case OrderbookTrades:
sql = sql.Where(sq.Eq{"htrd.base_liquidity_pool_id": nil, "htrd.counter_liquidity_pool_id": nil})
sql = sql.Where(sq.Eq{"htrd.trade_type": OrderbookTradeType})
case LiquidityPoolTrades:
sql = sql.Where(sq.Eq{"htrd.base_offer_id": nil, "htrd.counter_offer_id": nil})
sql = sql.Where(sq.Eq{"htrd.trade_type": LiquidityPoolTradeType})
case AllTrades:
default:
return "", nil, errors.Errorf("Invalid trade type: %v", query.tradeType)
Expand Down Expand Up @@ -308,6 +308,7 @@ var selectTradeFields = sq.Select(
"htrd.base_is_seller",
"htrd.price_n",
"htrd.price_d",
"htrd.trade_type",
)

var selectReverseTradeFields = sq.Select(
Expand All @@ -332,6 +333,7 @@ var selectReverseTradeFields = sq.Select(
"NOT(htrd.base_is_seller) as base_is_seller",
"htrd.price_d as price_n",
"htrd.price_n as price_d",
"htrd.trade_type",
)

func getCanonicalAssetOrder(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,16 @@ import (
"github.com/stellar/go/support/errors"
)

// TradeType is an enum which indicates the type of trade
type TradeType int16

const (
// OrderbookTradeType is a trade which exercises an offer on the orderbook.
OrderbookTradeType = TradeType(1)
// LiquidityPoolTradeType is a trade which exercises a liquidity pool.
LiquidityPoolTradeType = TradeType(2)
)

// InsertTrade represents the arguments to TradeBatchInsertBuilder.Add() which is used to insert
// rows into the history_trades table
type InsertTrade struct {
Expand All @@ -33,6 +43,8 @@ type InsertTrade struct {

BaseIsSeller bool `db:"base_is_seller"`

Type TradeType `db:"trade_type"`

PriceN int64 `db:"price_n"`
PriceD int64 `db:"price_d"`
}
Expand Down
9 changes: 9 additions & 0 deletions services/horizon/internal/db2/history/trade_scenario.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ func createInsertTrades(
CounterAmount: 896,
PriceN: 1,
PriceD: 3,
Type: OrderbookTradeType,
}

second := first
Expand All @@ -51,6 +52,7 @@ func createInsertTrades(
CounterAmount: 6,
PriceN: 1156,
PriceD: 3,
Type: OrderbookTradeType,
}

fourth := InsertTrade{
Expand All @@ -67,6 +69,7 @@ func createInsertTrades(
BaseIsSeller: true,
PriceN: 675,
PriceD: 981,
Type: LiquidityPoolTradeType,
}

fifth := InsertTrade{
Expand All @@ -83,6 +86,7 @@ func createInsertTrades(
BaseIsSeller: true,
PriceN: 43,
PriceD: 56,
Type: LiquidityPoolTradeType,
}

return []InsertTrade{
Expand Down Expand Up @@ -293,6 +297,7 @@ func TradeScenario(tt *test.T, q *Q) TradeFixtures {
BaseIsSeller: true,
PriceN: null.IntFrom(inserts[0].PriceN),
PriceD: null.IntFrom(inserts[0].PriceD),
Type: OrderbookTradeType,
},
{
HistoryOperationID: inserts[1].HistoryOperationID,
Expand All @@ -313,6 +318,7 @@ func TradeScenario(tt *test.T, q *Q) TradeFixtures {
BaseIsSeller: true,
PriceN: null.IntFrom(inserts[1].PriceN),
PriceD: null.IntFrom(inserts[1].PriceD),
Type: OrderbookTradeType,
},
{
HistoryOperationID: inserts[2].HistoryOperationID,
Expand All @@ -333,6 +339,7 @@ func TradeScenario(tt *test.T, q *Q) TradeFixtures {
BaseIsSeller: false,
PriceN: null.IntFrom(inserts[2].PriceN),
PriceD: null.IntFrom(inserts[2].PriceD),
Type: OrderbookTradeType,
},
{
HistoryOperationID: inserts[3].HistoryOperationID,
Expand All @@ -354,6 +361,7 @@ func TradeScenario(tt *test.T, q *Q) TradeFixtures {
LiquidityPoolFee: inserts[3].LiquidityPoolFee,
PriceN: null.IntFrom(inserts[3].PriceN),
PriceD: null.IntFrom(inserts[3].PriceD),
Type: LiquidityPoolTradeType,
},
{
HistoryOperationID: inserts[4].HistoryOperationID,
Expand All @@ -375,6 +383,7 @@ func TradeScenario(tt *test.T, q *Q) TradeFixtures {
LiquidityPoolFee: inserts[4].LiquidityPoolFee,
PriceN: null.IntFrom(inserts[4].PriceN),
PriceD: null.IntFrom(inserts[4].PriceD),
Type: LiquidityPoolTradeType,
},
}

Expand Down
23 changes: 23 additions & 0 deletions services/horizon/internal/db2/schema/bindata.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
-- +migrate Up

ALTER TABLE history_trades ADD trade_type smallint DEFAULT 1 CHECK(trade_type > 0);
UPDATE history_trades SET trade_type = 2 WHERE base_liquidity_pool_id IS NOT NULL OR counter_liquidity_pool_id IS NOT NULL;
CREATE INDEX htrd_by_trade_type ON history_trades USING BTREE(trade_type, history_operation_id, "order");

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would it be ok to drop the other liquidity pool id indexes on history_trades? not sure if they come into play on other queries, but they aren't used for deriving trade type query anymore, correct? htrd_by_base_liquidity_pool_id htrd_by_counter_liquidity_pool_id

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is an endpoint to fetch all trades occurring on a specific liquidity pool (e.g. https://horizon.stellar.org/liquidity_pools/001403bfa898ef65015bb585e07343811aca01e02cc2a6f126a6c842f6d5359c/trades ) . those indexes are used by that endpoint

-- +migrate Down

DROP INDEX htrd_by_trade_type;
ALTER TABLE history_trades DROP trade_type;
Original file line number Diff line number Diff line change
Expand Up @@ -299,9 +299,11 @@ func (p *TradeProcessor) extractTrades(
return nil, err
}
row.LiquidityPoolFee = null.IntFrom(int64(fee))
row.Type = history.LiquidityPoolTradeType
} else {
row.BaseOfferID = null.IntFrom(int64(trade.OfferId()))
sellerAccount = trade.SellerId().Address()
row.Type = history.OrderbookTradeType
}

if buyOfferExists {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,7 @@ func (s *TradeProcessorTestSuiteLedger) mockReadTradeTransactions(
BaseIsSeller: false,
PriceN: int64(s.sellPrices[0].D),
PriceD: int64(s.sellPrices[0].N),
Type: history.OrderbookTradeType,
},
{
HistoryOperationID: toid.New(int32(ledger.Header.LedgerSeq), 1, 3).ToInt64(),
Expand All @@ -257,6 +258,7 @@ func (s *TradeProcessorTestSuiteLedger) mockReadTradeTransactions(
BaseOfferID: null.IntFrom(int64(s.strictSendTrade.OfferId())),
PriceN: int64(s.sellPrices[1].N),
PriceD: int64(s.sellPrices[1].D),
Type: history.OrderbookTradeType,
},
{
HistoryOperationID: toid.New(int32(ledger.Header.LedgerSeq), 1, 4).ToInt64(),
Expand All @@ -273,6 +275,7 @@ func (s *TradeProcessorTestSuiteLedger) mockReadTradeTransactions(
CounterOfferID: null.IntFrom(int64(s.buyOfferTrade.OfferId())),
PriceN: int64(s.sellPrices[2].D),
PriceD: int64(s.sellPrices[2].N),
Type: history.OrderbookTradeType,
},
{
HistoryOperationID: toid.New(int32(ledger.Header.LedgerSeq), 1, 5).ToInt64(),
Expand All @@ -289,6 +292,7 @@ func (s *TradeProcessorTestSuiteLedger) mockReadTradeTransactions(
BaseOfferID: null.IntFrom(int64(s.sellOfferTrade.OfferId())),
PriceN: int64(s.sellPrices[3].N),
PriceD: int64(s.sellPrices[3].D),
Type: history.OrderbookTradeType,
},
{
HistoryOperationID: toid.New(int32(ledger.Header.LedgerSeq), 1, 6).ToInt64(),
Expand All @@ -305,6 +309,7 @@ func (s *TradeProcessorTestSuiteLedger) mockReadTradeTransactions(
CounterOfferID: null.IntFrom(int64(s.passiveSellOfferTrade.OfferId())),
PriceN: int64(s.sellPrices[4].D),
PriceD: int64(s.sellPrices[4].N),
Type: history.OrderbookTradeType,
},

{
Expand All @@ -323,6 +328,7 @@ func (s *TradeProcessorTestSuiteLedger) mockReadTradeTransactions(
BaseOfferID: null.IntFrom(int64(s.otherPassiveSellOfferTrade.OfferId())),
PriceN: int64(s.sellPrices[5].N),
PriceD: int64(s.sellPrices[5].D),
Type: history.OrderbookTradeType,
},
{
HistoryOperationID: toid.New(int32(ledger.Header.LedgerSeq), 1, 8).ToInt64(),
Expand All @@ -339,6 +345,7 @@ func (s *TradeProcessorTestSuiteLedger) mockReadTradeTransactions(
LiquidityPoolFee: null.IntFrom(int64(xdr.LiquidityPoolFeeV18)),
PriceN: int64(s.sellPrices[6].D),
PriceD: int64(s.sellPrices[6].N),
Type: history.LiquidityPoolTradeType,
},
{
HistoryOperationID: toid.New(int32(ledger.Header.LedgerSeq), 1, 9).ToInt64(),
Expand All @@ -355,6 +362,7 @@ func (s *TradeProcessorTestSuiteLedger) mockReadTradeTransactions(
LiquidityPoolFee: null.IntFrom(int64(xdr.LiquidityPoolFeeV18)),
PriceN: int64(s.sellPrices[7].N),
PriceD: int64(s.sellPrices[7].D),
Type: history.LiquidityPoolTradeType,
},
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ func TestTradeAggregations(t *testing.T) {
CounterAssetID: counterAssetId,
PriceN: 23456,
PriceD: 10000,
Type: history.OrderbookTradeType,
},
},
resolution: 60_000,
Expand Down Expand Up @@ -115,6 +116,7 @@ func TestTradeAggregations(t *testing.T) {
CounterAssetID: counterAssetId,
PriceN: 23456,
PriceD: 10000,
Type: history.OrderbookTradeType,
},
{
HistoryOperationID: 0,
Expand All @@ -130,6 +132,7 @@ func TestTradeAggregations(t *testing.T) {
CounterAssetID: counterAssetId,
PriceN: 13456,
PriceD: 10000,
Type: history.OrderbookTradeType,
},
},
resolution: 60_000,
Expand Down Expand Up @@ -169,6 +172,7 @@ func TestTradeAggregations(t *testing.T) {
CounterAssetID: counterAssetId,
PriceN: 23456,
PriceD: 10000,
Type: history.OrderbookTradeType,
},
{
HistoryOperationID: 0,
Expand All @@ -184,6 +188,7 @@ func TestTradeAggregations(t *testing.T) {
CounterAssetID: counterAssetId,
PriceN: 13456,
PriceD: 10000,
Type: history.OrderbookTradeType,
},
},
resolution: 86_400_000,
Expand Down
12 changes: 5 additions & 7 deletions services/horizon/internal/resourceadapter/trade.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,8 @@ func PopulateTrade(
if row.BaseAccount.Valid {
dest.BaseAccount = row.BaseAccount.String
}
var isLiquidityPoolTrade bool
if row.BaseLiquidityPoolID.Valid {
dest.BaseLiquidityPoolID = row.BaseLiquidityPoolID.String
isLiquidityPoolTrade = true
}
dest.BaseAssetType = row.BaseAssetType
dest.BaseAssetCode = row.BaseAssetCode
Expand All @@ -47,7 +45,6 @@ func PopulateTrade(
}
if row.CounterLiquidityPoolID.Valid {
dest.CounterLiquidityPoolID = row.CounterLiquidityPoolID.String
isLiquidityPoolTrade = true
}
dest.CounterAssetType = row.CounterAssetType
dest.CounterAssetCode = row.CounterAssetCode
Expand All @@ -60,10 +57,11 @@ func PopulateTrade(
dest.LiquidityPoolFeeBP = uint32(row.LiquidityPoolFee.Int64)
}

if isLiquidityPoolTrade {
dest.TradeType = "liquidity_pool"
} else {
dest.TradeType = "orderbook"
switch row.Type {
case history.OrderbookTradeType:
dest.TradeType = history.OrderbookTrades
case history.LiquidityPoolTradeType:
dest.TradeType = history.LiquidityPoolTrades
}

if row.HasPrice() {
Expand Down