Skip to content

Commit

Permalink
[GraphQL/Limits] Reimplement QueryLimitsChecker (#18666)
Browse files Browse the repository at this point in the history
## Description

Rewriting query limits checker to land a number of improvements and
fixes:

- Avoid issues with overflows by counting down from a predefined budget,
rather than counting up to the limit and protecting multiplications
using `checked_mul`.

- Improve detection of paginated fields: 
- Previously we treated all connections-related fields as appearing as
many times as the page size (including the field that introduced the
connection, and the `pageInfo` field). This was over-approximated the
output size by a large margin. The new approach counts exactly the
number of nodes in the output: The connection's root field, and any
non-`edges` or `nodes` field will not get multiplied by the page size.
- The checker now also detects connections-related fields even if they
are obscured by fragment or inline fragment spreads.

- Tighter `__schema` query detection: Previously we would skip requests
that started with a `__schema` introspection query. Now it's required to
be the only operation in the request (not just the first).

- Fix metrics collection after limits are hit: Previously, if a limit
was hit, we would not observe validation-related metrics in prometheus.
Now we will always record such metrics, and if a limit has been hit, it
will register as being "at" the limit.

## Test plan

```
sui-graphql-e2e-tests$ cargo nextest run --features pg_integration -- limits/
```

## Stack

- #18660 
- #18661 
- #18662
- #18663 
- #18664 

---

## 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: Output node estimation has been made more accurate -- the
estimate should now track the theoretical max number of nodes on the
JSON `data` output.
- [ ] CLI: 
- [ ] Rust SDK:
  • Loading branch information
amnn committed Jul 16, 2024
1 parent e397c2f commit 4ba06ed
Show file tree
Hide file tree
Showing 4 changed files with 588 additions and 479 deletions.
103 changes: 73 additions & 30 deletions crates/sui-graphql-e2e-tests/tests/limits/output_node_estimation.exp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
processed 14 tasks
processed 16 tasks

task 1 'run-graphql'. lines 6-14:
Response: {
Expand All @@ -16,7 +16,7 @@ Response: {
"depth": 3,
"variables": 0,
"fragments": 0,
"queryPayload": 132
"queryPayload": 254
}
}
}
Expand All @@ -37,11 +37,11 @@ Response: {
"extensions": {
"usage": {
"inputNodes": 4,
"outputNodes": 80,
"outputNodes": 42,
"depth": 4,
"variables": 0,
"fragments": 0,
"queryPayload": 163
"queryPayload": 359
}
}
}
Expand All @@ -68,11 +68,11 @@ Response: {
"extensions": {
"usage": {
"inputNodes": 6,
"outputNodes": 1640,
"outputNodes": 842,
"depth": 6,
"variables": 0,
"fragments": 0,
"queryPayload": 206
"queryPayload": 484
}
}
}
Expand Down Expand Up @@ -108,11 +108,11 @@ Response: {
"extensions": {
"usage": {
"inputNodes": 10,
"outputNodes": 1720,
"outputNodes": 922,
"depth": 6,
"variables": 0,
"fragments": 0,
"queryPayload": 306
"queryPayload": 735
}
}
}
Expand Down Expand Up @@ -142,11 +142,11 @@ Response: {
"extensions": {
"usage": {
"inputNodes": 10,
"outputNodes": 1640,
"outputNodes": 882,
"depth": 6,
"variables": 0,
"fragments": 0,
"queryPayload": 308
"queryPayload": 733
}
}
}
Expand All @@ -171,7 +171,7 @@ Response: {
"depth": 4,
"variables": 0,
"fragments": 0,
"queryPayload": 145
"queryPayload": 323
}
}
}
Expand All @@ -196,7 +196,7 @@ Response: {
"depth": 4,
"variables": 0,
"fragments": 0,
"queryPayload": 143
"queryPayload": 322
}
}
}
Expand All @@ -219,16 +219,16 @@ Response: {
"extensions": {
"usage": {
"inputNodes": 14,
"outputNodes": 3320,
"outputNodes": 1762,
"depth": 8,
"variables": 0,
"fragments": 0,
"queryPayload": 501
"queryPayload": 1077
}
}
}

task 9 'run-graphql'. lines 144-170:
task 9 'run-graphql'. lines 144-171:
Response: {
"data": {
"transactionBlocks": {
Expand All @@ -244,16 +244,16 @@ Response: {
"extensions": {
"usage": {
"inputNodes": 13,
"outputNodes": 3300,
"outputNodes": 1742,
"depth": 7,
"variables": 0,
"fragments": 0,
"queryPayload": 533
"queryPayload": 1030
}
}
}

task 10 'run-graphql'. lines 172-221:
task 10 'run-graphql'. lines 173-222:
Response: {
"data": {
"transactionBlocks": {
Expand All @@ -274,16 +274,16 @@ Response: {
"extensions": {
"usage": {
"inputNodes": 24,
"outputNodes": 86340,
"outputNodes": 46424,
"depth": 11,
"variables": 0,
"fragments": 0,
"queryPayload": 1395
"queryPayload": 2093
}
}
}

task 11 'run-graphql'. lines 223-248:
task 11 'run-graphql'. lines 224-249:
Response: {
"data": {
"transactionBlocks": {
Expand All @@ -300,34 +300,64 @@ Response: {
"extensions": {
"usage": {
"inputNodes": 12,
"outputNodes": 33300,
"outputNodes": 17302,
"depth": 11,
"variables": 0,
"fragments": 0,
"queryPayload": 704
"queryPayload": 1029
}
}
}

task 12 'run-graphql'. lines 250-260:
task 12 'run-graphql'. lines 251-272:
Response: {
"data": {
"fragmentSpread": {
"nodes": [
{
"digest": "EoFwLKRy23XKLkWZbBLiqjTV2vsKPsmpW6dV2caK8ZDH"
}
]
},
"inlineFragment": {
"nodes": [
{
"digest": "EoFwLKRy23XKLkWZbBLiqjTV2vsKPsmpW6dV2caK8ZDH"
}
]
}
},
"extensions": {
"usage": {
"inputNodes": 8,
"outputNodes": 44,
"depth": 4,
"variables": 0,
"fragments": 1,
"queryPayload": 562
}
}
}

task 13 'run-graphql'. lines 274-286:
Response: {
"data": null,
"extensions": {
"usage": {
"inputNodes": 4,
"outputNodes": 80,
"outputNodes": 62,
"depth": 4,
"variables": 0,
"fragments": 0,
"queryPayload": 154
"queryPayload": 394
}
},
"errors": [
{
"message": "'first' and 'last' must not be used together",
"locations": [
{
"line": 3,
"line": 4,
"column": 3
}
],
Expand All @@ -341,17 +371,17 @@ Response: {
]
}

task 13 'run-graphql'. lines 262-272:
task 14 'run-graphql'. lines 288-298:
Response: {
"data": null,
"extensions": {
"usage": {
"inputNodes": 4,
"outputNodes": 80,
"outputNodes": 42,
"depth": 4,
"variables": 0,
"fragments": 0,
"queryPayload": 147
"queryPayload": 141
}
},
"errors": [
Expand All @@ -366,3 +396,16 @@ Response: {
}
]
}

task 15 'run-graphql'. lines 300-310:
Response: {
"data": null,
"errors": [
{
"message": "Estimated output nodes exceeds 100000",
"extensions": {
"code": "BAD_USER_INPUT"
}
}
]
}
Loading

0 comments on commit 4ba06ed

Please sign in to comment.