You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
add global legacy / shadow / chasm transport-mode dynamic config, defaulting to legacy
register the namespace-replication CHASM library and history service, and route frontend mutations through the layered history client in shadow mode
run the full component and destination-tagged peer transport while hard-skipping source and receiver writes
compare deterministic payload fingerprints at both the frontend build boundary and peer receive boundary, with mismatch logging
preserve the legacy metadata write and replication queue as the only authoritative path
Tests
make proto
go test -tags test_dep ./common/namespace/nsreplication/... ./chasm/lib/namespacereplication/... ./service/frontend/...
go test -tags test_dep ./service/history ./service/worker
make fmt-imports
make lint-code-fast
Operational impact
The default remains legacy. shadow exercises CHASM compare-only and does not fail the user mutation if shadow transport fails. Authoritative chasm writes are intentionally rejected until PR4. This is the first stack point suitable for a simple multi-cell shadow transport E2E.
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
Seven unresolved findings remain, including one critical issue and multiple moderate rollout and correctness issues.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Wires namespace replication through CHASM in shadow mode while retaining legacy replication as authoritative.
Changes:
Adds transport-mode configuration and CHASM history/worker registration.
Routes frontend mutations through compare-only shadow transport.
Adds deterministic fingerprints, peer validation, generated protobufs, and tests.
File summaries
File
Reviewed changes and final findings
tests/xdc/namespace_replication_chasm_test.go
Adds shadow transport E2E coverage.
tests/testcore/functional_test_base.go
Supports additional test server options.
service/worker/fx.go
Registers the namespace replication library.
service/history/fx.go
Registers the history-side CHASM library.
service/frontend/workflow_handler.go
Injects the layered CHASM client.
service/frontend/workflow_handler_test.go
Updates handler fixtures.
service/frontend/service.go
Adds transport-mode configuration.
service/frontend/namespace_handler.go
Builds and triggers shadow mutations. Moderate (1 vote): decouple shadow triggering from the user-mutation critical path. Moderate (2 votes): reject unsupported chasm mode instead of silently falling back to legacy.
service/frontend/namespace_handler_test.go
Updates constructor fixtures.
service/frontend/namespace_handler_chasm_test.go
Tests shadow mutation routing.
service/frontend/fx.go
Provides the layered CHASM client and library.
service/frontend/admin_handler.go
Adds compare-only receiver handling. Moderate (1 vote): distinguish shadow validation from a real applied write.
med — Make the trigger idempotent across layered-client retries.
The frontend calls this RPC through a retrying layered client, but StartExecution uses the default BusinessIDConflictPolicyFail and BusinessIDReusePolicyAllowDuplicate. If the first RPC persists the component and loses its response, a retry while it is running returns ExecutionAlreadyStarted; if it has completed, the reuse policy can start a second execution for the same mutation. The frontend ignores the error, so this appears as a false shadow failure or duplicates the entire shadow fan-out and its persistence/visibility work.
Suggestion: Use an idempotent start/retry flow: on an existing execution, resolve the same component and poll it rather than returning an error or creating a new run.
med — Malformed mutation details can panic during peer transport.
Checking only that namespace_detail is present allows info, config, or replication_config to be nil. The shadow local task skips the metadata write, then NamespaceDetailToTaskAttributes dereferences those nested fields directly when a peer task runs, turning an invalid RPC into a task panic/failure instead of a bounded InvalidArgument response.
Suggestion: Validate all nested fields required by NamespaceDetailToTaskAttributes at this RPC boundary.
if req.GetMutation().GetNamespaceDetail() == nil {
return nil, serviceerror.NewInvalidArgument("mutation.namespace_detail is required")
chasm/lib/namespacereplication/library.go:40
Details
med — Register the active library with tdbg.
This change now persists namespace-replication CHASM components and adds a registration-only NewNilLibrary, but tools/tdbg/chasm_registry.go still omits this library while registering the other CHASM libraries. As a result, tdbg cannot resolve the new component/task types when inspecting these executions, which removes the debugging path for the feature. Add namespacereplication.NewNilLibrary() to that registry alongside the other registration-only libraries.
Suggestion: Update tools/tdbg/chasm_registry.go to register the namespace-replication nil library.
med — Shadow peer validation is reported as a real write.
OUTCOME_APPLIED is documented as an existing namespace being updated, but this branch intentionally performs no receiver write. The peer applier maps that response to PeerApplyResultApplied, so the source component records PEER_APPLY_OUTCOME_APPLIED and can expose a successful write even though shadow mode only validated the payload.
Suggestion: Add a distinct validation-only outcome (and map it separately) or avoid recording a write outcome for shadow requests.
med — Keep the shadow trigger off the user-mutation critical path.
This RPC uses the caller's context and is awaited synchronously. If the history service is unavailable, the layered client retries and waits until that request context expires before this function logs and ignores the error, so a compare-only failure can still consume the Register/Update deadline or cause the user RPC to be cancelled. The shadow path should be launched with an independently bounded context (or another non-blocking delivery mechanism) so its failure cannot affect user-mutation latency.
Suggestion: Decouple this trigger from the request context and bound its background lifetime; retain the warning log when the detached attempt fails.
nit — Document the RPC's current shadow-only semantics.
The next line still promises apply-if-higher writes, but this PR rejects non-shadow requests and shadow requests never mutate state. This public API comment therefore describes behavior that no supported request can reach.
Suggestion: Describe shadow validation and the temporary rejection of authoritative requests.
// calls this RPC against each peer cell after its local phase resolves.
// Apply semantics are apply-if-higher (config_version / failover_version).
The changed generated files contain displaced comments: this method's deprecation marker is inserted inside strconv.Itoa, and api/adminservice/v1/request_response.pb.go:115-121 attaches deprecation and scheduler comments to unrelated enum branches. The code still parses, but generated API documentation and deprecation metadata are malformed.
Suggestion: Regenerate both protobuf outputs from their source files and verify that declaration comments remain attached to EnumDescriptor and SchedulerTarget.
med — The blocking RPC test double can leak forever on a failed test.
The test double ignores the RPC context while waiting on release. If the test fails before closing that channel, the production timeout cannot stop this goroutine, so it remains blocked for the rest of the test process.
Suggestion: Wait for either release or context cancellation and return ctx.Err() when canceled.
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
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.
Stack: PR2 of 7
Depends on #12113.
Summary
legacy/shadow/chasmtransport-mode dynamic config, defaulting tolegacyTests
make protogo test -tags test_dep ./common/namespace/nsreplication/... ./chasm/lib/namespacereplication/... ./service/frontend/...go test -tags test_dep ./service/history ./service/workermake fmt-importsmake lint-code-fastOperational impact
The default remains
legacy.shadowexercises CHASM compare-only and does not fail the user mutation if shadow transport fails. Authoritativechasmwrites are intentionally rejected until PR4. This is the first stack point suitable for a simple multi-cell shadow transport E2E.