Repository navigation
feat(components): implement production Cloud Spanner store client and emulator integration suite - #682
Conversation
… emulator integration suite
There was a problem hiding this comment.
Code Review
This pull request introduces a Cloud Spanner-backed implementation of the component store (SpannerStore) along with its corresponding unit tests and integration into the experiment state server. The review feedback highlights critical performance and concurrency risks in Spanner transactions, specifically pointing out that loading the entire active catalog inside read-write transactions in UpdateObservedGeneration, CompleteDeactivation, and SetComponentIntent will cause severe lock contention and table scans. Additionally, the reviewer recommends improving error handling by propagating protobuf unmarshalling errors in rowToComponentRecord, unmarshalConfigProto, and unmarshalAccessStatus instead of silently ignoring them.
There was a problem hiding this comment.
A few questions as I don't remember all technical details with components. Also please split similar PRs like this in the future - 2k is a lot and there was no need to implement everything here at once.
|
@andrzejfudro thanks, will try better. I've put UPD: @andrzejfudro PR 698 addresses this problem of a split. If you want, I can split this PR in 3 or if you deem this to be reviewed already by you, we can leave it as it is |
|
@VLukiantsev No need to split now. Just to share my opinion, I've seen the "Anti-Stubbing & Dead Code Prevention" explanation but I think it is perfectly valid to stub the methods if something is work-in-progress and PR grows too much. |
Summary
Implements the production Cloud Spanner store client in
pkg/components/store/spanner_store.gosatisfying thestore.Storeinterface for the XMC Components System. Author a comprehensive Cloud Spanner emulator integration test suite inpkg/components/store/spanner_store_test.goand wire the Spanner store intocmd/experiment_state_server/main.go.Motivation & Context
The XMC Components System requires a durable, ACID-compliant relational persistence layer backing the Component Catalog and Component Manager Service (CMS). The production database schema (
010_components_system.up.sql/ess-schema.sql) uses Cloud Spanner with generated columns (ActiveName), null-filtered unique indexes, and foreign-key interleaved dependency graphs. This change provides the production Spanner store driver implementing transactional intent registration, soft-deletion active name release, cascading deactivations, and generation snapshot resolution.Key Changes
pkg/components/store/spanner_store.go):store.Storeinterface with parameterizedspanner.Statementqueries andReadWriteTransactionisolation.SetComponentIntentwith DAG prospective cycle detection, active dependency validation, and atomic mutation buffers.GetComponent,GetComponentByID, andListComponentsusingReadOnlyTransactionand targeted row scanners for snapshot isolation.PrepareDeactivationandCompleteDeactivationwith cascade calculations, drain calculation, and targeted active dependent verification (queryActiveDependents).UpdateObservedGenerationwith single-row point updates (queryComponentWithDeps) to eliminate lock contention during controller reconciliation.ExecuteDeletionwith deletion protection checks (xmc.dev/deletion-protection), cascading soft-deletions (State = 5), and automaticActiveNameindex release.ResolveComponentSnapshotfor current and historical generation resolution fromExperimentComponentSnapshots.ValidateComponentRequirementwith semantic versioning constraint evaluation.pkg/components/store/spanner_store_test.go):cmd/experiment_state_server/server/prod.go, exposesprodServer.SpannerClient().cmd/experiment_state_server/main.go, updatesinitCMSto instantiatestore.NewSpannerStore(spannerClient)with fallback tostore.NewMemoryStore().Architectural Sizing & Indivisible Atomic Implementation Exception
SpannerStoreis a single cohesive Cloud Spanner database adapter satisfying the existing 11-methodstore.Storeinterface contract (var _ Store = (*SpannerStore)(nil)). Splitting across partial methods would break Go compile-time type assertions and server wiring incmd/experiment_state_server/main.go.codes.Unimplementedor runtime panics, introducing unreachable/dead code and runtime trapdoors into trunk.graph_validator.go,deactivation_engine.go,semver.go,snapshot_resolver.go).SpannerStorestrictly contains database persistence mapping.Assumptions
None — requirements were fully specified in task b/549524786 and validated against database schema migrations and store interfaces.
Verification & Testing
bazel test //pkg/components/... //cmd/experiment_state_server/...store_test,cms_test,bridge_test,linter_test,health_test, andserver_test).//pkg/components/store:store_test) passed in ~10s.Related Issues / Bugs
Fixes b/549524786
TAG=agy
CONV=454ed690-28d9-4de3-865b-a0055f77d835