Refactor Spanner embedding generation to run ML.PREDICT outside write transaction#656
Conversation
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| Security | 1 medium |
| CodeStyle | 1 minor |
🟢 Metrics 4 complexity
Metric Results Complexity 4
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
There was a problem hiding this comment.
Code Review
This pull request refactors the embedding generation process in _generate_embeddings_partitioned by running ML.PREDICT as a read-only query outside of the read-write transaction, followed by writing the results in a separate, short transaction to reduce transaction overhead. Additionally, the unused embedding_label_index has been removed from the schema, configuration, Spanner client, and tests. Feedback suggests filtering out any None embeddings returned by ML.PREDICT before performing the write transaction to avoid unnecessary write operations.
1ba5999 to
c92a0bd
Compare
… transaction * Uncouple ML.PREDICT remote inference from Spanner read-write transactions to prevent 409 aborted transactions due to long-held locks. * Execute prediction as a read-only query outside transaction, and commit updates in a fast write transaction (<50ms). * Clean up unused secondary index on embedding table DDL and update test suite mocks.
c92a0bd to
c2b6d86
Compare
gmechali
left a comment
There was a problem hiding this comment.
Thank you Dan! (pls wait for Xiao's review too)
…o spanner-embeddings-fix
…o spanner-embeddings-fix # Conflicts: # pipeline/workflow/ingestion-helper/app_test.py # pipeline/workflow/ingestion-helper/utils/embeddings.py
…o spanner-embeddings-fix
Summary
This change refactors the Spanner embedding ingestion helper to separate remote machine learning inference (
ML.PREDICT) from Spanner read-write transactions.Problem
Previously, the embedding generator executed
INSERT OR UPDATE ... SELECT FROM ML.PREDICT(...)within a Spanner read-write transaction. BecauseML.PREDICTmakes external network calls to Vertex AI, the write transaction remained open for a prolonged period (~85 seconds in a typical bulk ingestion batch of 1,000 nodes).During bulk imports (such as the
undatadataset), Spanner frequently runs high-priority background range splits and directory migrations (_MoveRange). The long-held write locks from our transaction repeatedly conflicted with these range moves, causing Spanner to abort the user transaction with a 409 conflict:Transaction was aborted. It was wounded by a higher priority transaction with tag "_MoveRange" due to conflict on keys with prefix [<internal>], column state in table split.Solution
ML.PREDICTquery as a read-only query outside the read-write transaction using a database snapshot. This fetches the generated embeddings into memory without holding Spanner locks.NodeEmbeddingLabelIndexonembedding_label.app_test.pyandutils/embeddings_test.pyto match the two-stage execution model."