Add resolveNamespaceID - #263
Open
JayChung0258 wants to merge 1 commit into
Open
Conversation
Given a field somewhere in a replication message, resolveNamespaceID walks up the parent chain and returns the namespace id of the nearest message that owns a namespace. Nothing calls it yet. Only four message types count as owners, matched by a type switch. Other messages carry a NamespaceId that names a different namespace, so matching on the field name would pick the wrong one. The clearest example is StartChildWorkflowExecutionInitiatedEventAttributes, which holds the child's NamespaceId right beside the parent's SearchAttributes. Walking up rather than remembering the last NamespaceId seen on the way down keeps the answer independent of traversal order, which the visit library does not define. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
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.
The main idea
To translate a search attribute per namespace, the code first has to answer a question it cannot answer today: standing on a
SearchAttributesfield somewhere in a replication message, which namespace does this belong to?The attribute itself does not say. It is a map key with an opaque payload under it. The namespace is further out, on the message that owns the workflow.
This PR adds the one function that answers that. Nothing calls it yet.
What was changed
resolveNamespaceID(vwp, fallback)walks up the parent chain from any field and returns theNamespaceIdof the nearest message that owns a namespace:fallbackis returned when the walk reaches nothing. That happens in two places, both handled in later PRs: inside a data blob, whose events are visited in a fresh traversal with no link back out, and on the two raw history responses, which carry history but no namespace of their own.Two decisions worth reviewing
Owners are matched by a type switch over four types, not by looking for a field named
NamespaceId.Because plenty of other messages have a
NamespaceIdthat means something else.history.StartChildWorkflowExecutionInitiatedEventAttributesis the one that would actually bite: it holds the child'sNamespaceId(field 18) right next to the parent'sSearchAttributes(field 17), so a field name match would translate a parent's history event with the child's mapping.WorkflowExecutionInfohas the same shape viaParentNamespaceId.A type switch also means the compiler checks this. If one of these messages renames its field upstream, the build breaks instead of translation quietly stopping.
The question I would like a second opinion on: are these four complete? I got them by going through every search attribute and data blob field reachable in
adminservice,replication,persistenceandhistory. If a new message type shows up later with search attributes and an owner that is not in this list, resolution returns the fallback and translation is skipped rather than being wrong. A later PR adds a counter for that case.The walk goes up, not down. Remembering the last
NamespaceIdseen while descending would be simpler to write, but the visit library does not define an order:visit.ValuesUnsafepops the front of its worklist and swaps in the last element, so it is neither breadth first nor depth first. One replication frame can carry tasks for several namespaces, so state kept during a descent can be latched from the wrong task. Walking up from the field has no such problem.Checklist
Toward CGSCE-639.
How was this tested:
Six tests, all walking real messages so they exercise the same parent chain the translator will see: one hop, two and three hops, the child
NamespaceIdbeing stepped over,ParentNamespaceIdbeing ignored, an owner with an empty id carrying on outward, and no owner returning the fallback.Mutation checked: adding
StartChildWorkflowExecutionInitiatedEventAttributesto the type switch turnsTestResolveNamespaceIDIgnoresChildNamespaceIDred, which is that test's only job.make generate-test-certs go test -race -timeout=12m -tags test_dep -count=1 ./... make lintBoth clean.
No.
🤖 Generated with Claude Code