Repository navigation
perf: instrument and speed up deployment fingerprints - #2602
Conversation
A shared.main dry-run for semantic-shared PR #255 spent 340 seconds building fingerprints for 6,663 downstream impacts. Log time spent loading targets and external ancestors, converting specs, finding ancestor closures, and building current and proposed fingerprint graphs. Avoid deep-copying external target specs between graph snapshots. Clear their current values from the shared cache before calculating proposed values so changed ancestors still produce fresh hashes. Reuse parsed query ASTs when rendered specs are fingerprinted instead of parsing the same SQL on each pass. Add a repeatable 6,000-metric benchmark and regression tests. The synthetic run fell from about 4.0 to 2.1 seconds, with SQL parses falling from 18,000 to 6,000. Database access is mocked, so the production speedup is not yet measured. Tests: 30 deployment fingerprint tests and 44 model fingerprint tests passed.
The benchmark uses mocked database access and a synthetic graph. Keep its measurements in the PR description while retaining the production timing logs and regression tests in the source tree.
Describe why a downstream target must be rehashed against proposed ancestors when the shared cache is keyed by spec identity.
✅ Deploy Preview for thriving-cassata-78ae72 canceled.
|
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if rendered.rendered_query is not None: | ||
| rendered._query_ast = spec.query_ast |
There was a problem hiding this comment.
When extract_node_graph processes a metric, it caches an AST with the projection alias rewritten to the metric name. This change then reuses that AST for the version-1 fingerprint. The deployment path therefore hashes the rewritten query, while fingerprinting the same spec without prior dependency extraction hashes the original query. An unchanged metric can receive different fingerprints depending on which path processed it first.
2 participants
Summary
USG checks in https://github.netflix.net/corp/semantic-shared/pull/255 are taking more than 7 minutes, so I had to bump the timeout to 15 minutes in https://github.netflix.net/corp/semantic-shared/pull/322.
This PR does some low hanging fruit optimizations (avoid deep copying, avoid reparsing SQL). This improved perf on a synthetic tests from about 4.0 to 2.1 seconds. The PR also add logging to instrument the checks better, so we can find the bottleneck in #255.
Test Plan
make checkpassesmake testshows 100% unit test coverageDeployment Plan