diff --git a/CHANGELOG.md b/CHANGELOG.md index b110ec22..b4e8f015 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ file. This project adheres to [Semantic Versioning](http://semver.org/). ## Unreleased +### Fixed +- Reduce PostgreSQL temp-file I/O for the `/trades` endpoint when filtered by account, offer, or liquidity pool. The account-filter query pattern was the dominant contributor to slow-query time and replication lag on pubnet. The subquery UNION now uses `UNION ALL` (the two branches are disjoint by protocol invariant — an account cannot match its own offer, a trade matches two distinct offers, and an LP trade has the pool on exactly one side) and each branch now applies the outer `LIMIT` so only the top rows per branch are materialized rather than the entire filtered set. + +### DB Schema Migration +- Add composite index `htrd_by_base_account_op_order` on `history_trades(base_account_id, history_operation_id, "order")` and `htrd_by_counter_account_op_order` on `(counter_account_id, history_operation_id, "order")` to give the planner a density-independent, spill-free plan for account-filtered trade queries. At pubnet scale each index build takes multiple hours; the migration uses `CREATE INDEX CONCURRENTLY` to avoid blocking writes. The existing single-column indexes `htrd_by_base_account` and `htrd_by_counter_account` are retained until follow-up observation confirms the composites cover all usage. + ## 26.0.0 **This release adds support for Protocol 26** diff --git a/internal/db2/history/trade.go b/internal/db2/history/trade.go index 70d4a5dd..1eec73e8 100644 --- a/internal/db2/history/trade.go +++ b/internal/db2/history/trade.go @@ -204,8 +204,8 @@ func createTradesSQL(page db2.PageQuery, oldestLedger int32, query historyTrades secondSelect = sql.Where("htrd.counter_liquidity_pool_id = ?", query.poolID) } - firstSelect = appendOrdering(firstSelect, oldestLedger, op, idx, page.Order) - secondSelect = appendOrdering(secondSelect, oldestLedger, op, idx, page.Order) + firstSelect = appendOrdering(firstSelect, oldestLedger, op, idx, page.Order).Limit(page.Limit) + secondSelect = appendOrdering(secondSelect, oldestLedger, op, idx, page.Order).Limit(page.Limit) firstSQL, firstArgs, err := firstSelect.ToSql() if err != nil { return "", nil, errors.Wrap(err, "error building a firstSelect query") @@ -215,7 +215,13 @@ func createTradesSQL(page db2.PageQuery, oldestLedger int32, query historyTrades return "", nil, errors.Wrap(err, "error building a secondSelect query") } - rawSQL := fmt.Sprintf("(%s) UNION (%s) ", firstSQL, secondSQL) + // UNION ALL is safe here: the two branches filter on base_/counter_ columns of the same + // field (account, offer, or liquidity_pool), and stellar-core's invariants guarantee those + // columns never hold the same non-null value in a single trade row — an account cannot + // match its own offer, a trade matches two distinct offers, and an LP trade has one side + // as the pool and the other as an account (so exactly one of base_/counter_liquidity_pool_id + // is non-null). This lets PostgreSQL skip the dedup sort UNION would otherwise force. + rawSQL := fmt.Sprintf("(%s) UNION ALL (%s) ", firstSQL, secondSQL) args := append(firstArgs, secondArgs...) // Order the final UNION: switch page.Order { diff --git a/internal/db2/history/trade_test.go b/internal/db2/history/trade_test.go index fdd55343..01b23346 100644 --- a/internal/db2/history/trade_test.go +++ b/internal/db2/history/trade_test.go @@ -1,6 +1,7 @@ package history import ( + "strings" "testing" "github.com/stellar/go-stellar-sdk/toid" @@ -41,6 +42,34 @@ func filterByAccount(trades []Trade, account string) []Trade { return result } +// TestCreateTradesSQLUsesUnionAll guards against a regression where the account, +// offer, or liquidity-pool filter variant's subquery UNION is switched back to +// plain UNION (which forces a server-side dedup sort and was the dominant +// temp-file I/O source on pubnet). The two branches are disjoint by protocol +// invariant, so UNION ALL is semantically equivalent and significantly cheaper. +func TestCreateTradesSQLUsesUnionAll(t *testing.T) { + cases := []struct { + name string + query historyTradesQuery + }{ + {"account", historyTradesQuery{accountID: 1, tradeType: AllTrades, orderPreserved: true}}, + {"offer", historyTradesQuery{offerID: 1, tradeType: AllTrades, orderPreserved: true}}, + {"pool", historyTradesQuery{poolID: 1, tradeType: AllTrades, orderPreserved: true}}, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + sql, _, err := createTradesSQL(descPQ, 0, c.query) + if err != nil { + t.Fatalf("createTradesSQL failed: %v", err) + } + if !strings.Contains(sql, "UNION ALL") { + t.Errorf("generated SQL must use UNION ALL (not UNION) for the %s filter; got: %s", c.name, sql) + } + }) + } +} + func TestSelectTrades(t *testing.T) { tt := test.Start(t) defer tt.Finish() diff --git a/internal/db2/schema/bindata.go b/internal/db2/schema/bindata.go index 882683d0..7a97420e 100644 --- a/internal/db2/schema/bindata.go +++ b/internal/db2/schema/bindata.go @@ -68,6 +68,7 @@ // migrations/69_add_asset_contracts_table.sql (671B) // migrations/6_create_assets_table.sql (366B) // migrations/70_replace_timestamp_trade_aggregations_brin_index.sql (317B) +// migrations/71_trades_account_composite_indexes.sql (457B) // migrations/7_modify_trades_table.sql (2.303kB) // migrations/8_add_aggregators.sql (907B) // migrations/8_create_asset_stats_table.sql (441B) @@ -1500,6 +1501,26 @@ func migrations70_replace_timestamp_trade_aggregations_brin_indexSql() (*asset, return a, nil } +var _migrations71_trades_account_composite_indexesSql = []byte("\x1f\x8b\x08\x00\x00\x00\x00\x00\x00\xff\x94\x90\x41\x4a\xc6\x30\x10\x46\xf7\x39\xc5\xf0\xaf\x14\xed\x09\xba\x92\x36\x85\x80\x24\x92\xa6\x50\x57\x21\x6d\x82\xcd\xc2\x4c\x99\x44\xa4\xb7\x17\x8b\x88\x16\x91\xfe\xdb\xf0\xf2\xcd\xe3\x55\x15\xdc\xbd\xc6\x17\x72\x25\xc0\xb0\x42\xc2\x42\x2e\x65\x37\x97\x88\x89\xb1\x46\xf3\x07\xc3\x41\xc8\x96\x8f\xd0\x28\xd9\x0c\x5a\x73\x69\x1e\x9f\x41\x74\x20\x95\x01\x3e\x8a\xde\xf4\xb0\x14\xf2\x76\xda\xec\xe4\x72\xb0\x6e\x9e\xf1\x2d\x15\x8b\xab\x45\xf2\x81\x18\x80\x92\xb0\xc4\x5c\x90\x36\x5b\xc8\xf9\x90\xe1\xe6\x17\x1a\xfd\xfd\x37\x80\x6b\x20\xf7\x79\x7e\x7f\xbd\xec\x13\x97\xdb\xfa\x7a\x99\x7d\x3a\xd0\x49\x9f\x23\x7d\x4a\xe9\x67\xbd\x16\xdf\xd3\xb1\x5f\xab\xd5\xd3\x97\xb0\xe8\xce\xfa\xd5\xff\x7f\xfb\xb3\x71\xcd\x3e\x02\x00\x00\xff\xff\xb3\xd8\x1e\xf0\xc9\x01\x00\x00") + +func migrations71_trades_account_composite_indexesSqlBytes() ([]byte, error) { + return bindataRead( + _migrations71_trades_account_composite_indexesSql, + "migrations/71_trades_account_composite_indexes.sql", + ) +} + +func migrations71_trades_account_composite_indexesSql() (*asset, error) { + bytes, err := migrations71_trades_account_composite_indexesSqlBytes() + if err != nil { + return nil, err + } + + info := bindataFileInfo{name: "migrations/71_trades_account_composite_indexes.sql", size: 0, mode: os.FileMode(0), modTime: time.Unix(0, 0)} + a := &asset{bytes: bytes, info: info, digest: [32]uint8{0x2f, 0xa9, 0x70, 0x57, 0x35, 0xca, 0x14, 0x5c, 0xe1, 0x6c, 0xdf, 0x31, 0x5, 0x18, 0x3b, 0xad, 0xa0, 0x4, 0x21, 0x85, 0xf8, 0x4e, 0x6f, 0x3, 0x0, 0x60, 0xfc, 0x17, 0x9f, 0x9d, 0xe5, 0x98}} + return a, nil +} + var _migrations7_modify_trades_tableSql = []byte("\x1f\x8b\x08\x00\x00\x00\x00\x00\x00\xff\xc4\x54\x4d\x8f\xda\x30\x14\xbc\xe7\x57\x3c\xed\x29\x51\xc3\xaa\xad\xda\xbd\x6c\x55\x09\x58\x97\x46\x65\xc3\x36\x04\xa9\xb7\xc8\x89\xdf\x06\xab\xc1\x8e\x6c\xa7\x88\x7f\x5f\x05\x08\xcd\x27\xb0\xbb\x87\x5e\x93\x99\x79\x6f\xec\xf1\x8c\x46\xf0\x6e\xc3\x53\x45\x0d\xc2\x2a\xb7\x46\x23\x60\x4a\xe6\x60\xd6\x08\x32\x63\x60\x14\x65\xa8\xc1\xd0\x38\xc3\x5b\xc8\x0b\x03\x14\x04\x6e\x41\x0a\x04\x2e\x20\xcf\x68\x82\xd6\x43\xb0\x78\x82\x70\x3c\x99\x13\x58\x73\x6d\xa4\xda\x45\x07\xde\xbd\x35\x0d\xc8\x38\x24\xbd\x3f\xc1\xb6\x00\xe0\xf4\x51\xe6\xa8\xa8\xe1\x52\x44\x9c\xc1\xc4\x9b\x79\x7e\x08\xfe\x22\x04\x7f\x35\x9f\xbb\x7b\xe4\x8d\x54\x0c\xd5\x0d\x78\x7e\x48\x66\x24\x68\xfd\xcd\x90\xa5\xa8\xa2\x24\x93\x1a\x59\x44\x0d\x84\xde\x23\x59\x86\xe3\xc7\xa7\x16\x50\x3e\x3f\xa3\x1a\x1c\x12\x53\x8d\x11\x4d\x12\x59\x08\xd3\x03\x82\x80\x7c\x23\x01\xf1\xa7\x64\x79\xda\xfc\x88\xd6\x36\x67\x4e\x5d\x44\x6b\xbc\x5a\xa2\xc4\x76\x04\x36\xa5\x6c\x87\x3e\xfd\x4e\xa6\x3f\xc0\xae\x43\xbe\xc2\xfb\x23\x71\xbf\x09\xaa\x37\x3b\x38\xe9\xbc\xc1\xc4\x49\xe3\xac\x8f\x16\xea\x9f\x95\xbd\x41\xae\x23\x8d\x59\x86\x0a\x26\x8b\xc5\x9c\x8c\xfd\xc3\xbf\x3d\xd7\x6e\x1e\xf3\x97\xce\xd2\x8e\xe5\xdc\x5b\x55\x04\x57\xbe\xf7\x73\x45\xc0\xf3\x1f\xc8\x2f\x58\x1b\xc5\xa2\x9c\x33\x58\xf8\xed\x54\xae\x96\x9e\x3f\x83\xd8\x28\x44\xb0\xfb\xc2\xe9\x56\x41\x74\x4e\xf1\xae\x8b\x52\xae\x22\xc3\x37\x18\x65\x52\xfe\x2e\xf2\xc1\x09\x93\x30\x20\xa4\x69\xc1\xed\x38\x70\x3b\xb1\xee\x1d\x5a\xd1\xae\x1a\xd9\x39\xa5\x3e\xc5\xeb\x1d\x5c\xb5\x60\xbc\x8b\xf6\xcf\xee\xd2\x79\x57\x6f\xb3\xbc\x37\xab\x5e\x4d\x0f\x72\x2b\x1a\xe5\x24\x70\x8b\xaa\xea\x25\x85\x5c\x68\x53\xe2\xaa\xde\x92\x02\x6f\x87\x7b\x09\x12\xaa\x13\xca\xf0\xd5\xfd\x14\xf3\x94\x0b\x33\xd0\x4f\x5c\x18\x4c\x51\x0d\xd5\x4e\x2f\xf7\x10\xf2\xc1\xdf\x71\xb1\x3b\x47\x96\x19\x3b\x5e\xa7\xd9\xe5\x08\xc9\x9a\x2a\x9a\x18\x54\xf0\x87\xaa\x1d\x17\xa9\x7d\xf7\xc9\x19\xe6\x70\xad\x0b\x54\x3d\xac\xcf\x77\x67\x58\x89\x64\x7d\x93\x3e\x7c\xec\xe7\x1c\x5e\x77\x6b\xfd\xaa\x03\xea\x90\x5a\x01\xc8\x22\x5d\x9b\x97\x1a\x6b\xb0\x5e\x60\xad\xc1\xbb\xda\x5c\xc5\x3a\x6b\xaf\x09\x2a\x0d\xfe\x87\x62\x7a\xc5\x13\x6c\x8b\x94\x1a\xe5\x55\x5d\x92\x68\xe5\xd1\x6d\xc7\xc6\xed\xa6\x6f\x60\xda\xe1\xe4\x2e\xcd\xeb\x04\xc5\xed\xde\xa6\xdb\x17\x0c\xe7\xfe\x6f\x00\x00\x00\xff\xff\x2a\xff\xe8\x4a\xff\x08\x00\x00") func migrations7_modify_trades_tableSqlBytes() ([]byte, error) { @@ -1739,6 +1760,7 @@ var _bindata = map[string]func() (*asset, error){ "migrations/69_add_asset_contracts_table.sql": migrations69_add_asset_contracts_tableSql, "migrations/6_create_assets_table.sql": migrations6_create_assets_tableSql, "migrations/70_replace_timestamp_trade_aggregations_brin_index.sql": migrations70_replace_timestamp_trade_aggregations_brin_indexSql, + "migrations/71_trades_account_composite_indexes.sql": migrations71_trades_account_composite_indexesSql, "migrations/7_modify_trades_table.sql": migrations7_modify_trades_tableSql, "migrations/8_add_aggregators.sql": migrations8_add_aggregatorsSql, "migrations/8_create_asset_stats_table.sql": migrations8_create_asset_stats_tableSql, @@ -1857,6 +1879,7 @@ var _bintree = &bintree{nil, map[string]*bintree{ "69_add_asset_contracts_table.sql": {migrations69_add_asset_contracts_tableSql, map[string]*bintree{}}, "6_create_assets_table.sql": {migrations6_create_assets_tableSql, map[string]*bintree{}}, "70_replace_timestamp_trade_aggregations_brin_index.sql": {migrations70_replace_timestamp_trade_aggregations_brin_indexSql, map[string]*bintree{}}, + "71_trades_account_composite_indexes.sql": {migrations71_trades_account_composite_indexesSql, map[string]*bintree{}}, "7_modify_trades_table.sql": {migrations7_modify_trades_tableSql, map[string]*bintree{}}, "8_add_aggregators.sql": {migrations8_add_aggregatorsSql, map[string]*bintree{}}, "8_create_asset_stats_table.sql": {migrations8_create_asset_stats_tableSql, map[string]*bintree{}}, diff --git a/internal/db2/schema/migrations/71_trades_account_composite_indexes.sql b/internal/db2/schema/migrations/71_trades_account_composite_indexes.sql new file mode 100644 index 00000000..1155522d --- /dev/null +++ b/internal/db2/schema/migrations/71_trades_account_composite_indexes.sql @@ -0,0 +1,12 @@ +-- +migrate Up notransaction + +CREATE INDEX CONCURRENTLY IF NOT EXISTS htrd_by_base_account_op_order + ON history_trades (base_account_id, history_operation_id, "order"); + +CREATE INDEX CONCURRENTLY IF NOT EXISTS htrd_by_counter_account_op_order + ON history_trades (counter_account_id, history_operation_id, "order"); + +-- +migrate Down notransaction + +DROP INDEX IF EXISTS htrd_by_counter_account_op_order; +DROP INDEX IF EXISTS htrd_by_base_account_op_order;