-
Notifications
You must be signed in to change notification settings - Fork 11.2k
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
[GraphQL/TransactionBlock] Scan Limits #18413
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
3 Skipped Deployments
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initial round of comments -- main thing to watch out for is the fact that it looks like in this implementation the scan limit is given in terms of checkpoints, when it should be in terms of transactions.
63e235c
to
f114258
Compare
f114258
to
d685824
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quick scan through to get caught up before our pairing session today!
crates/sui-graphql-rpc/src/types/transaction_block/tx_connection.rs
Outdated
Show resolved
Hide resolved
b176ef8
to
505b2a7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We got there, congrats @wlmyng :)
Mostly small nits left about docs etc, but there is one important detail to correct about avoiding underflow/overflow.
Let's also hold off on landing this until we get the green light from @gegaowp that the breaking changes pick is safe to build on top of.
Well done, again 🎉
crates/sui-graphql-rpc/src/types/transaction_block/tx_lookups.rs
Outdated
Show resolved
Hide resolved
crates/sui-graphql-rpc/src/types/transaction_block/tx_lookups.rs
Outdated
Show resolved
Hide resolved
crates/sui-graphql-rpc/src/types/transaction_block/tx_lookups.rs
Outdated
Show resolved
Hide resolved
crates/sui-graphql-rpc/src/types/transaction_block/tx_lookups.rs
Outdated
Show resolved
Hide resolved
crates/sui-graphql-rpc/src/types/transaction_block/tx_lookups.rs
Outdated
Show resolved
Hide resolved
comments elucidating why we check the transaction's first or last cursor against the scan limited cursor of the current page do the same for the backwards pagination case when a cursor is specified, bump scan_limit by one so we don't count it among the txs scanned. otherwise we'll be short one
322f0c6
to
a7f22d4
Compare
There was a bug related to bounds for the first page, when it contains the genesis checkpoint. This was fixed by refactoring `TxBounds` to systematically use a half-open interval (inclusive lowerbounds and exclusive upperbounds) when calculating and combining intervals to be used on the DB, regardless of where the bounds came from and whether they were inclusive or exclusive at source. Also added a new test to exercise this behaviour -- all existing tests also pass, unchanged.
## Description Implement learnings from GraphQL performance benchmarks: - Implement transaction block pagination as a two step process: First fetch the relevant transaction sequence numbers, then fetch their contents. - Every "atomic" filter on transaction blocks is served by a single `tx_` table, with two indices on it, both of which are prepped to perform index-only scans. - The primary index is used to apply the filter directly. - The secondary index applies the filter after limiting results to one sender. - Compound filters are served by joining multiple atomic filters together. - The "scan limit" concept is introduced to limit the amount of work done when dealing with compound filters (see below). ### Scan Limits - If a filter is compound, a scan limit must be provided, and controls how many transactions are considered as candidates when building a page of results. - There is an upperbound on the scan limit, currently 100M, which is enough for over a week of transactions at 100TPS. - When scan limits are enabled, pagination behaviour changes: Pages can be returned with fewer results than the page size (including no results), but still have a previous or next page, because there were no valid candidates in the area scanned but there is more room to scan on either side. - The start and end cursor for the page may no longer point to an element in the results, because they point to the first and last candidate transaction. ## Test plan ``` sui$ cargo build -p sui-indexer sui$ cargo nextest run -p sui-graphql-rpc sui$ cargo nextest run -p sui-graphql-e2e-tests --features pg_integration ``` --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] Indexer: - [ ] JSON-RPC: - [x] GraphQL: Introduce `scanLimit` for paginating `TransactionBlocks`. Queries that include multiple complex filters (filters on the function called, affected objects, recipient), need to include a scan limit which controls the number of transactions that are looked at as candidates. - [ ] CLI: - [ ] Rust SDK: --------- Co-authored-by: Ashok Menon <ashok@mystenlabs.com>
## Description Implement learnings from GraphQL performance benchmarks: - Implement transaction block pagination as a two step process: First fetch the relevant transaction sequence numbers, then fetch their contents. - Every "atomic" filter on transaction blocks is served by a single `tx_` table, with two indices on it, both of which are prepped to perform index-only scans. - The primary index is used to apply the filter directly. - The secondary index applies the filter after limiting results to one sender. - Compound filters are served by joining multiple atomic filters together. - The "scan limit" concept is introduced to limit the amount of work done when dealing with compound filters (see below). ### Scan Limits - If a filter is compound, a scan limit must be provided, and controls how many transactions are considered as candidates when building a page of results. - There is an upperbound on the scan limit, currently 100M, which is enough for over a week of transactions at 100TPS. - When scan limits are enabled, pagination behaviour changes: Pages can be returned with fewer results than the page size (including no results), but still have a previous or next page, because there were no valid candidates in the area scanned but there is more room to scan on either side. - The start and end cursor for the page may no longer point to an element in the results, because they point to the first and last candidate transaction. ## Test plan ``` sui$ cargo build -p sui-indexer sui$ cargo nextest run -p sui-graphql-rpc sui$ cargo nextest run -p sui-graphql-e2e-tests --features pg_integration ``` --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] Indexer: - [ ] JSON-RPC: - [x] GraphQL: Introduce `scanLimit` for paginating `TransactionBlocks`. Queries that include multiple complex filters (filters on the function called, affected objects, recipient), need to include a scan limit which controls the number of transactions that are looked at as candidates. - [ ] CLI: - [ ] Rust SDK: --------- Co-authored-by: Ashok Menon <ashok@mystenlabs.com>
Description
Implement learnings from GraphQL performance benchmarks:
tx_
table, with two indices on it, both of which are prepped to perform index-only scans.Scan Limits
Test plan
Release notes
Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required.
For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates.
scanLimit
for paginatingTransactionBlocks
. Queries that include multiple complex filters (filters on the function called, affected objects, recipient), need to include a scan limit that controls the number of transactions that are looked at as candidates.