fix(perf): cache verified operation IDs to skip redundant preflight DB lookup#673
Open
michael-johnston wants to merge 2 commits intomainfrom
Open
fix(perf): cache verified operation IDs to skip redundant preflight DB lookup#673michael-johnston wants to merge 2 commits intomainfrom
michael-johnston wants to merge 2 commits intomainfrom
Conversation
…B lookup _perform_preflight_checks_for_sample_store_methods decorates every sample store method (measurement_requests_for_operation, complete_measurement_request_with_results_timeseries, etc.) and verifies that the supplied operation_id belongs to the space before allowing the call through. It did this by calling getResource(operation) on every decorated call — a full DB round-trip each time. For the common path for `ado show X operation` which goes through from_operation_id, this check is redundant as the operation was already fetched to locate the space, so ownership is proven by construction. The fix adds _verified_operation_ids: set[str] to DiscoverySpace.__init__ and guards the getResource call in the preflight decorator behind a set membership test. from_operation_id pre-populates the set with the operation_id immediately after the space is built, so any subsequent decorated call on that space skips the DB round-trip entirely. The full ownership check (getResource + uri comparison) is still executed for operation IDs not in the set, preserving correctness for spaces constructed via other paths (e.g. from_stored_configuration called directly) where ownership has not yet been established. Measured saving per decorated method call: ~653ms (this is saving in ado show X operation call)
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.
_perform_preflight_checks_for_sample_store_methods decorates every
sample store method (measurement_requests_for_operation,
complete_measurement_request_with_results_timeseries, etc.) and
verifies that the supplied operation_id belongs to the space before
allowing the call through. It did this by calling getResource(operation)
on every decorated call — a full DB round-trip each time.
For the common path for
ado show X operationwhich goes through from_operation_id, this check is redundant asthe operation was already fetched to locate the space, so ownership is
proven by construction.
The fix adds _verified_operation_ids: set[str] to DiscoverySpace.init
and guards the getResource call in the preflight decorator behind a set
membership test. from_operation_id pre-populates the set with the
operation_id immediately after the space is built, so any subsequent
decorated call on that space skips the DB round-trip entirely.
The full ownership check (getResource + uri comparison) is still
executed for operation IDs not in the set, preserving correctness for
spaces constructed via other paths (e.g. from_stored_configuration
called directly) where ownership has not yet been established.
Measured saving per decorated method call: ~653ms (this is saving in ado show X operation call)