feat(AF-941): bytes-scanned cost caps and estimated_bytes_scanned routing - #1104
Merged
Merged
Conversation
…ting Persist the warehouse pre-flight bytes estimate (V192) and let admins cap it per datasource and per grant (most restrictive wins), refused with 422 on engines without a bytes estimate. The cap rejects before routing, holds auto-approvals for review when no estimate exists (REQUIRE_REVIEW) and is re-checked before execution, groups and break-glass included. Adds the fail-closed estimated_bytes_scanned routing condition, a BYTES_SCANNED_CAP trace step and QUERY_BYTES_SCANNED_CAP_ENFORCED audit rows. Estimate prep runs in its own transaction so a lost insert race never strands a query.
Contributor
Frontend Test Results 1 files 314 suites 12m 28s ⏱️ Results for commit a5caa44. ♻️ This comment has been updated with latest results. |
Contributor
Coverage Report for Frontend Coverage (frontend)
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Contributor
Contributor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #941
What
Caps a governed warehouse query by bytes scanned, not only rows returned. It works in two layers:
estimated_bytes_scannedrouting condition that mirrorsestimated_rowsand fails closed when there is no estimate.datasources.max_bytes_scanned_per_queryplus a per-grantbytes_scanned_limit_override, with the most restrictive value winning. It is enforced when the query leavesPENDING_AIand again just before execution, which covers scheduled, recurring, grouped and break-glass runs, and caps lowered after approval.Decisions on the issue's open questions:
bytes_cap_missing_estimate.REQUIRE_REVIEW(the default) holds every automatic approval for a person, the same way a SQL-review BLOCK does.REJECTrefuses the query.BYTES_SCANNED_CAP_NOT_SUPPORTEDat config time.QUERY_BYTES_SCANNED_CAP_ENFORCED(null actor,stage=decision|execution) is written whenever the cap changed an outcome.Estimate persistence:
query_estimates.estimated_bytes_scannedis now persisted (V192). The AF-634 figure used to be dropped.estimated_rowspolicy could silently fail closed because it raced the estimate listener.REQUIRES_NEWtransaction, so losing the insert race to that listener can never roll back the decision.Screenshots
(The e2e stack has no warehouse, so for shots 2–4 a Postgres datasource was re-typed and the rejected query's cap state was seeded directly.)
Verification
Backend:
mvn -o verify -Pcoverageran 10,578 tests. The only 2 failures were index assertions inAdminAccessSimulationControllerIntegrationTestthat had shifted by the new trace step; those are now fixed.BytesScannedCapEnforcementIntegrationTestandSqlReviewEnforcementIntegrationTest, plusApplicationModulesTest,ApiPackageDependencyTestandMessagesParityTest. All green.Frontend: lint (0 errors), typecheck,
test:coverage(2698 tests; 96.1% lines, 87.7% branches) and build are all green.E2E: the full suite passed locally (387 tests).
admin-routing-policies.spec.tsgains two tests: a builder test for the new operand, and a check that the condition does not fire without an estimate.DatasourceSettingsPageandDatasourceCreateWizardPagecomponent tests andDatasourceControllerIntegrationTestinstead.Docs and website
03-data-model.md,04-api-spec.md,05-backend.md(new "Bytes-scanned cost caps" section),06-frontend.md,07-security.md.README.md, and theCLAUDE.mdstate-transition block.website/docs/configuration/{datasources,review-workflows,users-roles}/index.html. Their dates were already today's.help-corpus/regenerated.docs/09-deployment.mdis untouched.Review notes
Four reviewers ran: af-java-reviewer, af-reviewer, af-frontend-reviewer and af-content-reviewer. I ran the gates myself rather than using af-verifier.
Blockers, both fixed:
PENDING_AI(af-java-reviewer, af-reviewer). The estimate and cap preparation now run in aREQUIRES_NEWtransaction before the decision loads the query. A lost race only fails that inner transaction, and the winner's row is then read.REQUIRE_REVIEW(af-java-reviewer, af-reviewer). A group member with no estimate now runs only if a person approved the group; otherwise it fails witherror.bytes_cap.no_estimate_unreviewed.QueryCostEstimateService.estimateBytesScanned, which is bounded byaccessflow.proxy.estimate-timeout.Concerns and nits, fixed:
@ApiResponse422 texts now mention the new error (af-java-reviewer).Bunit symbol instead of the English word (af-java-reviewer).QueryAutoRejectedEventJavadoc is updated (af-reviewer).Surviving concerns, documented but not changed:
REQUIRE_REVIEW. This is documented in 05-backend.md.error_messagebut leaves the stampedbytes_scanned_capcolumns alone. This is a documented choice.Follow-ups, out of scope: Terraform provider and bootstrap-spec fields for the new datasource and grant settings.