feat(dynamicconfig): add operational dynamic config store properties - #8091
Merged
jakobht merged 1 commit intoMay 19, 2026
Merged
Conversation
jakobht
requested review from
Assem-Uber,
Shaddoll,
abhishekj720,
adhityamamallan,
arzonus,
bowenxia,
c-warren,
demirkayaender,
dkrotx,
eleonoradgr,
fimanishi,
gazi-yestemirova,
macrotim,
natemort,
neil-xie,
shijiesheng and
timl3136
as code owners
May 13, 2026 11:44
jakobht
force-pushed
the
feat/operational-config-properties
branch
from
May 13, 2026 11:47
6ee8965 to
9516036
Compare
This was referenced May 13, 2026
Adds four new dynamic config keys to control the polling cadence and timeouts of the operational dynamic config store (added in a follow-up commit): - system.operationalConfigStorePollInterval (Duration, default 2s) - system.operationalConfigStoreFetchTimeout (Duration, default 2s) - system.operationalConfigStoreUpdateTimeout (Duration, default 2s) - system.operationalConfigStoreUpdateRetryAttempts (Int, default 1) The 2s default puts the operational store at the configstore client's minimum poll interval (configStoreMinPollInterval), enabling sub-2s regional convergence for safety-critical operational config that cannot tolerate the convergence latency of a typical eventually-consistent dynamic config system. The store itself is added in the follow-up commit; defining the keys first lets that commit wire them in without a placeholder defaults step. Signed-off-by: Jakob Haahr Taankvist <jht@uber.com>
jakobht
force-pushed
the
feat/operational-config-properties
branch
from
May 17, 2026 08:55
9516036 to
2913473
Compare
Code Review ✅ ApprovedIntroduces four new dynamic config properties to manage polling intervals and timeouts for the upcoming operational config store. No issues found. Rules ✅ All requirements metRepository Rules
1 rule not applicable. Show all rules by commenting OptionsAuto-apply is off → Gitar will not commit updates to this branch. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | Gitar |
This was referenced May 17, 2026
eleonoradgr
reviewed
May 19, 2026
| // Default value: 2 seconds | ||
| OperationalConfigStorePollInterval | ||
|
|
||
| // OperationalConfigStoreFetchTimeout is the per-call timeout used when |
Contributor
There was a problem hiding this comment.
I am not sure we need this to be configurable
eleonoradgr
approved these changes
May 19, 2026
jakobht
added a commit
that referenced
this pull request
May 19, 2026
…rimary database (#8092) > **Stacked on top of #8091** — that PR adds the `system.operationalConfigStore*` dynamic config keys this PR consumes. Please review #8091 first; until it merges, the diff here will include those changes too. **What changed?** Adds an always-instantiated `configstore.Client` to `Resource` (parallel to the existing optional `isolationGroupConfigStore`), exposed as `GetOperationalConfigStore()`. Distinguished from existing config stores by a new `persistence.OperationalDynamicConfig` `ConfigType` (append-only enum addition; existing values keep their iota positions). Polling cadence and timeouts are driven by the `system.operationalConfigStore*` keys defined in #8091. Same nil-fallback semantics as `isolationGroupConfigStore` — logs a warning if the persistence layer doesn't support a config store. Nothing reads from this store yet; a follow-up PR will inject it into services and add `UpdateOperationalDynamicConfig` admin endpoint + CLI. **Why?** To enable sub-2s regional convergence for safety-critical operational config (e.g. sharding-related rollout flags) where stale per-host values can produce a split-brain in shard ownership. Backing such config with the primary database — via the already-existing configstore client, but as a separate, always-on instance — gives a reliable source of truth regardless of which dynamic config client the operator has configured as their primary. **How did you test it?** - `go test -count=1 ./common/resource/... ./common/persistence/ ./common/dynamicconfig/configstore/... ./service/history/resource/... ./service/history/handler/...` — all pass. - New coverage on changed lines: `GetOperationalConfigStore` 100%, `createOperationalConfigStoreOrDefault` 87.5%. - `make build` and `make lint` clean. - Existing `TestStartStop` extended to assert the getter returns nil under the test's stub persistence; two focused new tests for `createOperationalConfigStoreOrDefault` (override path + nil-when-persistence-unsupported). **Potential risks** - New polling client per host — at the 2s default poll interval, ~0.5 RPS per host against the configstore partition. **Release notes** N/A **Documentation Changes** N/A. --------- Signed-off-by: Jakob Haahr Taankvist <jht@uber.com>
jakobht
added a commit
that referenced
this pull request
May 19, 2026
…Resource (#8099) > **Stacked on top of #8092**, which is stacked on #8091. Please review in order: #8091 → #8092 → this PR. Until #8091 and #8092 merge, the diff here will include those commits too. **What changed?** Adds `Resource.GetOperationalDynamicConfig() *dynamicconfig.Collection`, wrapping the operational configstore client added in #8092 so service code can read operational dynamic config values with the same typed-accessor ergonomics used for the primary dynamic config. The Collection is always non-nil: when the underlying configstore client is unavailable (persistence layer doesn't support a config store), it is backed by a `NopClient` so callers get default values rather than having to nil-check. This mirrors the existing fallback for the primary dynamic config client. The cluster-name filter is applied, matching how the primary Collection is constructed. **Why?** Services need a typed entry point to read operational dynamic config (the values written via the upcoming `UpdateOperationalDynamicConfig` admin endpoint). Exposing it on `Resource` lets each service consume it the same way it consumes the primary Collection, with no nil-handling boilerplate at the call site. **How did you test it?** - `go build ./...` clean. - `go test -count=1 -race ./common/resource/... ./common/dynamicconfig/... ./common/persistence/ ./service/history/resource/... ./service/history/handler/...` all pass. - Coverage on new lines: `GetOperationalDynamicConfig` 100%, `newOperationalDynamicConfigCollection` 100%. - Extended `TestStartStop` to assert the getter returns non-nil even when the underlying store is nil; two focused new tests on `newOperationalDynamicConfigCollection` (nil-store-falls-back-to-nop and uses-store-when-present). **Potential risks** - Adds one method to the `Resource` interface and `service/history/resource.Resource` mock — both regenerated mocks are in the diff. No production code consumes the new Collection yet. - The no-op fallback hides the "operational store unavailable" condition from consumers reading via the Collection — callers that need to detect this should use `GetOperationalConfigStore()` (returns nil when unavailable) instead. **Release notes** N/A — preparation for a follow-up feature; no user-visible change. **Documentation Changes** N/A. Signed-off-by: Jakob Haahr Taankvist <jht@uber.com>
jakobht
added a commit
that referenced
this pull request
May 20, 2026
> **Stacked on top of #8099**, which is stacked on #8092, which is stacked on #8091. Please review in order: #8091 → #8092 → #8099 → this PR. Until those merge, the diff here will include their commits too. Also bumps the `idls` submodule to pick up [cadence-idl#266](cadence-workflow/cadence-idl#266). **What changed?** Adds the four admin RPCs that read and write the cassandra-backed operational dynamic config store: `GetOperationalDynamicConfig`, `UpdateOperationalDynamicConfig`, `RestoreOperationalDynamicConfig`, `ListOperationalDynamicConfig`. Each write goes through `logOperationalConfigChange`, which emits a structured audit log line capturing the caller, operation, key, and payload. The store-unavailable case (persistence backend doesn't support a config store) returns `BadRequestError` with the request scope's error metric recorded. Existing `Get/Update/Restore/ListDynamicConfig` are refactored to share inner helpers (`getDynamicConfigValue`, `updateDynamicConfigValue`, `restoreDynamicConfigValue`, `listDynamicConfigValue`) with the new operational variants so both surfaces go through one implementation per operation. As a side effect of the shared `updateDynamicConfigValue`/`restoreDynamicConfigValue` path, the regular (non-operational) handlers now also emit the audit log line on writes. A small drive-by in `common/resource/resource_test_utils.go`: changes `IsolationGroupStore` and `OperationalConfigStore` field types from `*configstore.MockClient` to `configstore.Client` so a nil assignment produces a true-nil interface — required for the `store unavailable` tests to actually exercise the BadRequestError branch. **Why?** The cassandra-backed operational dynamic config store added in the prior PRs needs an admin surface so operators can manage values. Reusing the regular dynamic config helpers keeps the two surfaces consistent (same validation, same JSON encoding, same filter handling) and the audit logging gives a paper trail for every operator-driven change. **How did you test it?** - `go build ./...` clean. - `go test -count=1 ./service/frontend/admin/... ./common/types/mapper/... ./common/resource/...` all pass. - New table-tests in `handler_test.go` cover the four operational handlers: nil request, store unavailable (`*BadRequestError`), invalid config name, happy-path forwarding to the store, and filter-passing for the Get path. - Coverage on new functions: `Get/Update/Restore/ListOperationalDynamicConfig` 92-100%, `operationalConfigClient` 100%, shared inner helpers 81-100%. **Potential risks** - Adds four methods to the admin service interface — all wrapper layers (accesscontrolled, grpc, thrift, errorinjectors, metered, retryable, timeout) are regenerated in the diff. - The shared `updateDynamicConfigValue` / `restoreDynamicConfigValue` path means the existing `UpdateDynamicConfig` / `RestoreDynamicConfig` admin RPCs now emit `operational dynamic config change` audit log lines as well. The log volume increase is bounded by admin write traffic, which is low. - No production code path consumes the new operational store via these RPCs yet; the matching-side consumption lands in a follow-up. **Release notes** N/A — preparation for the matching-side cutover; no user-visible change yet. **Documentation Changes** N/A. --- **Detailed Description** This PR makes two distinct sets of wire-shape changes: 1. **Additive: four new admin RPCs** — `GetOperationalDynamicConfig`, `UpdateOperationalDynamicConfig`, `RestoreOperationalDynamicConfig`, `ListOperationalDynamicConfig`. These are net-new methods on the admin service; no existing RPC is modified. 2. **Removed (via idl submodule bump): deprecated `ActiveClusterSelectionStrategy` enum and three fields on `ActiveClusterSelectionPolicy`** (`Strategy`, `StickyRegion`, `ExternalEntityType`, `ExternalEntityKey`). These were marked `deprecated = true` in the proto in cadence-idl and removed in cadence-idl #264 (now merged). This PR bumps the submodule past that removal and drops the now-orphaned mapper code + internal-type fields. The replacement is `ClusterAttribute`, which has been the supported path for some time. **Impact Analysis** - **Backward Compatibility**: Old clients that still send the deprecated `Strategy`/`StickyRegion`/`ExternalEntityType`/`ExternalEntityKey` fields will have those fields silently dropped — proto/thrift no longer have them on the wire. Behaviorally those clients fall through to `ClusterAttribute` (which they should already be populating per the prior deprecation). The new operational dynamic config RPCs are additive and have no backward-compat concern. - **Forward Compatibility**: New servers serving old clients work as above (deprecated fields ignored). New clients calling old servers see "method not found" for the four operational RPCs until the server side rolls out. **Testing Plan** - **Unit Tests**: Yes — admin handler tests cover all four new RPCs (nil/missing-args/store-unavailable/happy-path). Mapper round-trip tests cover the (now-shrunk) `ActiveClusterSelectionPolicy`. - **Persistence Tests**: Yes — `common/persistence/serializer_test.go` and `executionManagerTest.go` exercise `ActiveClusterSelectionPolicy` serialization with `ClusterAttribute` only. - **Integration Tests**: Existing integration tests pass against the new admin RPCs via the regenerated client wrappers. No `host/` test added for the operational store itself yet — coverage gap noted, follow-up if needed. - **Compatibility Tests**: Fuzz tests in `common/types/mapper/proto/api_test.go` exercise the (now simplified) `ActiveClusterSelectionPolicy` mappers. **Rollout Plan** - The four new operational dynamic config admin RPCs are additive — no rollout coordination needed. - The `ActiveClusterSelectionStrategy` removal is the cadence-side ingestion of cadence-idl #264 (already merged into the IDL). Clients still sending the deprecated fields will see them ignored; `ClusterAttribute` is the supported replacement and has been live for a while. - Order of deployment: server-side first (this PR + its stack). Once deployed, the operational config CLI (#8101) and matching-side reader (#8104) can land in either order. - Rollback: safe — no schema migration. Reverting drops the new admin RPCs (clients calling them get method-not-found) and restores the deprecated `ActiveClusterSelectionPolicy` field mappings (no behavior change since `ClusterAttribute` continues to work). - Kill switch: the operational dynamic config store itself is configurable per-namespace via existing dynamic config flags. No new kill switch added in this PR. Signed-off-by: Jakob Haahr Taankvist <jht@uber.com>
arzonus
pushed a commit
to arzonus/cadence
that referenced
this pull request
May 20, 2026
…adence-workflow#8091) **What changed?** Adds four new dynamic config keys to control the polling cadence and timeouts of an upcoming operational dynamic config store: | Key | Type | Default | | --- | --- | --- | | `system.operationalConfigStorePollInterval` | Duration | 2s | | `system.operationalConfigStoreFetchTimeout` | Duration | 2s | | `system.operationalConfigStoreUpdateTimeout` | Duration | 2s | | `system.operationalConfigStoreUpdateRetryAttempts` | Int | 1 | This is analogous to the existing isolation-group config store properties ([`system.isolationGroupState*`](https://github.com/cadence-workflow/cadence/blob/e5ae9bd25148eac140f717820be4d79f38ea3c87/common/dynamicconfig/dynamicproperties/constants.go#L5923-L5937)), which serve the same role for the existing isolation-group configstore. The new keys exist for an additional, separate operational configstore added in the follow-up. This is a key-definitions-only PR — nothing reads these values yet. **Why?** The 2s default puts the operational store at the configstore client's [minimum poll interval](https://github.com/cadence-workflow/cadence/blob/e5ae9bd25148eac140f717820be4d79f38ea3c87/common/dynamicconfig/configstore/config_store_client.go#L58-L60). The motivation is to enable sub-2s regional convergence for safety-critical operational config (e.g. sharding-related rollout flags). **How did you test it?** - `go test -count=1 -race ./common/dynamicconfig/...` — passes. **Potential risks** - None expected. **Release notes** N/A **Documentation Changes** N/A. Signed-off-by: Jakob Haahr Taankvist <jht@uber.com>
arzonus
pushed a commit
to arzonus/cadence
that referenced
this pull request
May 20, 2026
…rimary database (cadence-workflow#8092) > **Stacked on top of cadence-workflow#8091** — that PR adds the `system.operationalConfigStore*` dynamic config keys this PR consumes. Please review cadence-workflow#8091 first; until it merges, the diff here will include those changes too. **What changed?** Adds an always-instantiated `configstore.Client` to `Resource` (parallel to the existing optional `isolationGroupConfigStore`), exposed as `GetOperationalConfigStore()`. Distinguished from existing config stores by a new `persistence.OperationalDynamicConfig` `ConfigType` (append-only enum addition; existing values keep their iota positions). Polling cadence and timeouts are driven by the `system.operationalConfigStore*` keys defined in cadence-workflow#8091. Same nil-fallback semantics as `isolationGroupConfigStore` — logs a warning if the persistence layer doesn't support a config store. Nothing reads from this store yet; a follow-up PR will inject it into services and add `UpdateOperationalDynamicConfig` admin endpoint + CLI. **Why?** To enable sub-2s regional convergence for safety-critical operational config (e.g. sharding-related rollout flags) where stale per-host values can produce a split-brain in shard ownership. Backing such config with the primary database — via the already-existing configstore client, but as a separate, always-on instance — gives a reliable source of truth regardless of which dynamic config client the operator has configured as their primary. **How did you test it?** - `go test -count=1 ./common/resource/... ./common/persistence/ ./common/dynamicconfig/configstore/... ./service/history/resource/... ./service/history/handler/...` — all pass. - New coverage on changed lines: `GetOperationalConfigStore` 100%, `createOperationalConfigStoreOrDefault` 87.5%. - `make build` and `make lint` clean. - Existing `TestStartStop` extended to assert the getter returns nil under the test's stub persistence; two focused new tests for `createOperationalConfigStoreOrDefault` (override path + nil-when-persistence-unsupported). **Potential risks** - New polling client per host — at the 2s default poll interval, ~0.5 RPS per host against the configstore partition. **Release notes** N/A **Documentation Changes** N/A. --------- Signed-off-by: Jakob Haahr Taankvist <jht@uber.com>
arzonus
pushed a commit
to arzonus/cadence
that referenced
this pull request
May 20, 2026
…Resource (cadence-workflow#8099) > **Stacked on top of cadence-workflow#8092**, which is stacked on cadence-workflow#8091. Please review in order: cadence-workflow#8091 → cadence-workflow#8092 → this PR. Until cadence-workflow#8091 and cadence-workflow#8092 merge, the diff here will include those commits too. **What changed?** Adds `Resource.GetOperationalDynamicConfig() *dynamicconfig.Collection`, wrapping the operational configstore client added in cadence-workflow#8092 so service code can read operational dynamic config values with the same typed-accessor ergonomics used for the primary dynamic config. The Collection is always non-nil: when the underlying configstore client is unavailable (persistence layer doesn't support a config store), it is backed by a `NopClient` so callers get default values rather than having to nil-check. This mirrors the existing fallback for the primary dynamic config client. The cluster-name filter is applied, matching how the primary Collection is constructed. **Why?** Services need a typed entry point to read operational dynamic config (the values written via the upcoming `UpdateOperationalDynamicConfig` admin endpoint). Exposing it on `Resource` lets each service consume it the same way it consumes the primary Collection, with no nil-handling boilerplate at the call site. **How did you test it?** - `go build ./...` clean. - `go test -count=1 -race ./common/resource/... ./common/dynamicconfig/... ./common/persistence/ ./service/history/resource/... ./service/history/handler/...` all pass. - Coverage on new lines: `GetOperationalDynamicConfig` 100%, `newOperationalDynamicConfigCollection` 100%. - Extended `TestStartStop` to assert the getter returns non-nil even when the underlying store is nil; two focused new tests on `newOperationalDynamicConfigCollection` (nil-store-falls-back-to-nop and uses-store-when-present). **Potential risks** - Adds one method to the `Resource` interface and `service/history/resource.Resource` mock — both regenerated mocks are in the diff. No production code consumes the new Collection yet. - The no-op fallback hides the "operational store unavailable" condition from consumers reading via the Collection — callers that need to detect this should use `GetOperationalConfigStore()` (returns nil when unavailable) instead. **Release notes** N/A — preparation for a follow-up feature; no user-visible change. **Documentation Changes** N/A. Signed-off-by: Jakob Haahr Taankvist <jht@uber.com>
arzonus
pushed a commit
to arzonus/cadence
that referenced
this pull request
May 20, 2026
…adence-workflow#8091) **What changed?** Adds four new dynamic config keys to control the polling cadence and timeouts of an upcoming operational dynamic config store: | Key | Type | Default | | --- | --- | --- | | `system.operationalConfigStorePollInterval` | Duration | 2s | | `system.operationalConfigStoreFetchTimeout` | Duration | 2s | | `system.operationalConfigStoreUpdateTimeout` | Duration | 2s | | `system.operationalConfigStoreUpdateRetryAttempts` | Int | 1 | This is analogous to the existing isolation-group config store properties ([`system.isolationGroupState*`](https://github.com/cadence-workflow/cadence/blob/e5ae9bd25148eac140f717820be4d79f38ea3c87/common/dynamicconfig/dynamicproperties/constants.go#L5923-L5937)), which serve the same role for the existing isolation-group configstore. The new keys exist for an additional, separate operational configstore added in the follow-up. This is a key-definitions-only PR — nothing reads these values yet. **Why?** The 2s default puts the operational store at the configstore client's [minimum poll interval](https://github.com/cadence-workflow/cadence/blob/e5ae9bd25148eac140f717820be4d79f38ea3c87/common/dynamicconfig/configstore/config_store_client.go#L58-L60). The motivation is to enable sub-2s regional convergence for safety-critical operational config (e.g. sharding-related rollout flags). **How did you test it?** - `go test -count=1 -race ./common/dynamicconfig/...` — passes. **Potential risks** - None expected. **Release notes** N/A **Documentation Changes** N/A. Signed-off-by: Jakob Haahr Taankvist <jht@uber.com>
arzonus
pushed a commit
to arzonus/cadence
that referenced
this pull request
May 20, 2026
…rimary database (cadence-workflow#8092) > **Stacked on top of cadence-workflow#8091** — that PR adds the `system.operationalConfigStore*` dynamic config keys this PR consumes. Please review cadence-workflow#8091 first; until it merges, the diff here will include those changes too. **What changed?** Adds an always-instantiated `configstore.Client` to `Resource` (parallel to the existing optional `isolationGroupConfigStore`), exposed as `GetOperationalConfigStore()`. Distinguished from existing config stores by a new `persistence.OperationalDynamicConfig` `ConfigType` (append-only enum addition; existing values keep their iota positions). Polling cadence and timeouts are driven by the `system.operationalConfigStore*` keys defined in cadence-workflow#8091. Same nil-fallback semantics as `isolationGroupConfigStore` — logs a warning if the persistence layer doesn't support a config store. Nothing reads from this store yet; a follow-up PR will inject it into services and add `UpdateOperationalDynamicConfig` admin endpoint + CLI. **Why?** To enable sub-2s regional convergence for safety-critical operational config (e.g. sharding-related rollout flags) where stale per-host values can produce a split-brain in shard ownership. Backing such config with the primary database — via the already-existing configstore client, but as a separate, always-on instance — gives a reliable source of truth regardless of which dynamic config client the operator has configured as their primary. **How did you test it?** - `go test -count=1 ./common/resource/... ./common/persistence/ ./common/dynamicconfig/configstore/... ./service/history/resource/... ./service/history/handler/...` — all pass. - New coverage on changed lines: `GetOperationalConfigStore` 100%, `createOperationalConfigStoreOrDefault` 87.5%. - `make build` and `make lint` clean. - Existing `TestStartStop` extended to assert the getter returns nil under the test's stub persistence; two focused new tests for `createOperationalConfigStoreOrDefault` (override path + nil-when-persistence-unsupported). **Potential risks** - New polling client per host — at the 2s default poll interval, ~0.5 RPS per host against the configstore partition. **Release notes** N/A **Documentation Changes** N/A. --------- Signed-off-by: Jakob Haahr Taankvist <jht@uber.com>
arzonus
pushed a commit
to arzonus/cadence
that referenced
this pull request
May 20, 2026
…Resource (cadence-workflow#8099) > **Stacked on top of cadence-workflow#8092**, which is stacked on cadence-workflow#8091. Please review in order: cadence-workflow#8091 → cadence-workflow#8092 → this PR. Until cadence-workflow#8091 and cadence-workflow#8092 merge, the diff here will include those commits too. **What changed?** Adds `Resource.GetOperationalDynamicConfig() *dynamicconfig.Collection`, wrapping the operational configstore client added in cadence-workflow#8092 so service code can read operational dynamic config values with the same typed-accessor ergonomics used for the primary dynamic config. The Collection is always non-nil: when the underlying configstore client is unavailable (persistence layer doesn't support a config store), it is backed by a `NopClient` so callers get default values rather than having to nil-check. This mirrors the existing fallback for the primary dynamic config client. The cluster-name filter is applied, matching how the primary Collection is constructed. **Why?** Services need a typed entry point to read operational dynamic config (the values written via the upcoming `UpdateOperationalDynamicConfig` admin endpoint). Exposing it on `Resource` lets each service consume it the same way it consumes the primary Collection, with no nil-handling boilerplate at the call site. **How did you test it?** - `go build ./...` clean. - `go test -count=1 -race ./common/resource/... ./common/dynamicconfig/... ./common/persistence/ ./service/history/resource/... ./service/history/handler/...` all pass. - Coverage on new lines: `GetOperationalDynamicConfig` 100%, `newOperationalDynamicConfigCollection` 100%. - Extended `TestStartStop` to assert the getter returns non-nil even when the underlying store is nil; two focused new tests on `newOperationalDynamicConfigCollection` (nil-store-falls-back-to-nop and uses-store-when-present). **Potential risks** - Adds one method to the `Resource` interface and `service/history/resource.Resource` mock — both regenerated mocks are in the diff. No production code consumes the new Collection yet. - The no-op fallback hides the "operational store unavailable" condition from consumers reading via the Collection — callers that need to detect this should use `GetOperationalConfigStore()` (returns nil when unavailable) instead. **Release notes** N/A — preparation for a follow-up feature; no user-visible change. **Documentation Changes** N/A. Signed-off-by: Jakob Haahr Taankvist <jht@uber.com>
arzonus
pushed a commit
to arzonus/cadence
that referenced
this pull request
May 20, 2026
…rkflow#8100) > **Stacked on top of cadence-workflow#8099**, which is stacked on cadence-workflow#8092, which is stacked on cadence-workflow#8091. Please review in order: cadence-workflow#8091 → cadence-workflow#8092 → cadence-workflow#8099 → this PR. Until those merge, the diff here will include their commits too. Also bumps the `idls` submodule to pick up [cadence-idl#266](cadence-workflow/cadence-idl#266). **What changed?** Adds the four admin RPCs that read and write the cassandra-backed operational dynamic config store: `GetOperationalDynamicConfig`, `UpdateOperationalDynamicConfig`, `RestoreOperationalDynamicConfig`, `ListOperationalDynamicConfig`. Each write goes through `logOperationalConfigChange`, which emits a structured audit log line capturing the caller, operation, key, and payload. The store-unavailable case (persistence backend doesn't support a config store) returns `BadRequestError` with the request scope's error metric recorded. Existing `Get/Update/Restore/ListDynamicConfig` are refactored to share inner helpers (`getDynamicConfigValue`, `updateDynamicConfigValue`, `restoreDynamicConfigValue`, `listDynamicConfigValue`) with the new operational variants so both surfaces go through one implementation per operation. As a side effect of the shared `updateDynamicConfigValue`/`restoreDynamicConfigValue` path, the regular (non-operational) handlers now also emit the audit log line on writes. A small drive-by in `common/resource/resource_test_utils.go`: changes `IsolationGroupStore` and `OperationalConfigStore` field types from `*configstore.MockClient` to `configstore.Client` so a nil assignment produces a true-nil interface — required for the `store unavailable` tests to actually exercise the BadRequestError branch. **Why?** The cassandra-backed operational dynamic config store added in the prior PRs needs an admin surface so operators can manage values. Reusing the regular dynamic config helpers keeps the two surfaces consistent (same validation, same JSON encoding, same filter handling) and the audit logging gives a paper trail for every operator-driven change. **How did you test it?** - `go build ./...` clean. - `go test -count=1 ./service/frontend/admin/... ./common/types/mapper/... ./common/resource/...` all pass. - New table-tests in `handler_test.go` cover the four operational handlers: nil request, store unavailable (`*BadRequestError`), invalid config name, happy-path forwarding to the store, and filter-passing for the Get path. - Coverage on new functions: `Get/Update/Restore/ListOperationalDynamicConfig` 92-100%, `operationalConfigClient` 100%, shared inner helpers 81-100%. **Potential risks** - Adds four methods to the admin service interface — all wrapper layers (accesscontrolled, grpc, thrift, errorinjectors, metered, retryable, timeout) are regenerated in the diff. - The shared `updateDynamicConfigValue` / `restoreDynamicConfigValue` path means the existing `UpdateDynamicConfig` / `RestoreDynamicConfig` admin RPCs now emit `operational dynamic config change` audit log lines as well. The log volume increase is bounded by admin write traffic, which is low. - No production code path consumes the new operational store via these RPCs yet; the matching-side consumption lands in a follow-up. **Release notes** N/A — preparation for the matching-side cutover; no user-visible change yet. **Documentation Changes** N/A. --- **Detailed Description** This PR makes two distinct sets of wire-shape changes: 1. **Additive: four new admin RPCs** — `GetOperationalDynamicConfig`, `UpdateOperationalDynamicConfig`, `RestoreOperationalDynamicConfig`, `ListOperationalDynamicConfig`. These are net-new methods on the admin service; no existing RPC is modified. 2. **Removed (via idl submodule bump): deprecated `ActiveClusterSelectionStrategy` enum and three fields on `ActiveClusterSelectionPolicy`** (`Strategy`, `StickyRegion`, `ExternalEntityType`, `ExternalEntityKey`). These were marked `deprecated = true` in the proto in cadence-idl and removed in cadence-idl cadence-workflow#264 (now merged). This PR bumps the submodule past that removal and drops the now-orphaned mapper code + internal-type fields. The replacement is `ClusterAttribute`, which has been the supported path for some time. **Impact Analysis** - **Backward Compatibility**: Old clients that still send the deprecated `Strategy`/`StickyRegion`/`ExternalEntityType`/`ExternalEntityKey` fields will have those fields silently dropped — proto/thrift no longer have them on the wire. Behaviorally those clients fall through to `ClusterAttribute` (which they should already be populating per the prior deprecation). The new operational dynamic config RPCs are additive and have no backward-compat concern. - **Forward Compatibility**: New servers serving old clients work as above (deprecated fields ignored). New clients calling old servers see "method not found" for the four operational RPCs until the server side rolls out. **Testing Plan** - **Unit Tests**: Yes — admin handler tests cover all four new RPCs (nil/missing-args/store-unavailable/happy-path). Mapper round-trip tests cover the (now-shrunk) `ActiveClusterSelectionPolicy`. - **Persistence Tests**: Yes — `common/persistence/serializer_test.go` and `executionManagerTest.go` exercise `ActiveClusterSelectionPolicy` serialization with `ClusterAttribute` only. - **Integration Tests**: Existing integration tests pass against the new admin RPCs via the regenerated client wrappers. No `host/` test added for the operational store itself yet — coverage gap noted, follow-up if needed. - **Compatibility Tests**: Fuzz tests in `common/types/mapper/proto/api_test.go` exercise the (now simplified) `ActiveClusterSelectionPolicy` mappers. **Rollout Plan** - The four new operational dynamic config admin RPCs are additive — no rollout coordination needed. - The `ActiveClusterSelectionStrategy` removal is the cadence-side ingestion of cadence-idl cadence-workflow#264 (already merged into the IDL). Clients still sending the deprecated fields will see them ignored; `ClusterAttribute` is the supported replacement and has been live for a while. - Order of deployment: server-side first (this PR + its stack). Once deployed, the operational config CLI (cadence-workflow#8101) and matching-side reader (cadence-workflow#8104) can land in either order. - Rollback: safe — no schema migration. Reverting drops the new admin RPCs (clients calling them get method-not-found) and restores the deprecated `ActiveClusterSelectionPolicy` field mappings (no behavior change since `ClusterAttribute` continues to work). - Kill switch: the operational dynamic config store itself is configurable per-namespace via existing dynamic config flags. No new kill switch added in this PR. Signed-off-by: Jakob Haahr Taankvist <jht@uber.com>
jakobht
added a commit
that referenced
this pull request
May 21, 2026
> **Stacked on top of #8100**, which is stacked on #8099 → #8092 → #8091. Please review in order: #8091 → #8092 → #8099 → #8100 → this PR. Until those merge, the diff here will include their commits too. **What changed?** Adds four new commands under `cadence admin config` that call the operational dynamic config admin RPCs added in #8100: - `operational-get` (alias `og`) — read a value from the cassandra-backed store - `operational-update` (alias `ou`) — write one or more values - `operational-restore` (alias `or`) — remove a value (or a filtered subset) - `operational-list` (alias `ol`) — list all stored entries Flag shapes (`--name`, `--filter`, `--value`) and the JSON value/filter formats are identical to the existing `get` / `update` / `restore` / `list` commands so operators don't need to learn a new payload format. **Why?** Operators need a CLI surface to manage values in the cassandra-backed operational config store — same ergonomics as the existing dynamic config commands, just pointed at the new admin RPCs. **How did you test it?** - `go build ./...` clean. - `go test -count=1 ./tools/cli/...` all pass. - New table-tests cover each command: missing required flag, server error, and happy-path forwarding (with filter parsing for get/restore and value parsing for update). - Coverage on new actions: 80-86%. **Potential risks** - Adds four sibling commands to the existing `cadence admin config` tree; no existing command names changed. - Currently a thin wrapper over the admin RPCs added in #8100; the operational store needs to be available on the persistence backend for the commands to do anything (else the server returns `BadRequestError`). **Release notes** N/A — operator-facing CLI surface for the upcoming operational dynamic config rollout. **Documentation Changes** N/A. Signed-off-by: Jakob Haahr Taankvist <jht@uber.com>
jakobht
added a commit
that referenced
this pull request
May 21, 2026
…ingle reader (#8104) > **Stacked on #8100 (admin handler) and merges in #8103 (the source-of-truth flag).** The merge commit is preserved so reviewers can see #8103's dependency explicitly. Review order: #8091 → #8092 → #8099 → #8100 → #8101 → #8103 → this PR. **What changed?** Adds `membership.PercentageOnboarded`, a tiny interface (`Value() int`) backed by a mockable struct, that is the single read path for `matching.percentageOnboardedToShardManager`. Bootstrap constructs it once in `cmd/server/cadence/server.go` and threads it via `resource.Params` to every consumer — the matching engine, the shard-distributor resolver, and the spectator `Enabled` callback. No call site can bypass it. The reader handles three concerns in one place: - **Migration** from the generic dynamic config to the cassandra-backed operational store: reads both sources on every call, emits `percentage_onboarded_to_shard_manager` tagged with `onboarding_source` (`dynamicconfig` | `operational`) + `onboarding_active` (`active` | `shadow`), returns the value from whichever source `matching.percentageOnboardedReadFromOperationalStore` selects. - **Emergency-offboarding kill switch**: returns 0 when on, so every per-task-list `TaskListExcludedFromShardDistributor` check excludes the task list and the spectator stops considering the feature enabled. - **Source-truth metrics**: emitted gauges always reflect the source values (not the post-emergency override), so dashboards still see the underlying dc / operational reads even when the effective return is 0. Cleanups: - Drops the dead `PercentageOnboardedToShardManager` and `EmergencyOffboardingFromShardManager` fields from `service/matching/config.Config` (no production reader after this change). - Drops the emergency early-exit branches in `service/matching/handler/engine.go` and `common/membership/sharddistributorresolver.go` — both collapse into the single percentage read. - Adds `configstore.NewNopClient` and moves `resolveOperationalConfigStore` from `common/resource/resource_impl.go` to the bootstrap, so `params.OperationalConfigStore` is always non-nil and `matching/service.go` wires the operational `dynamicconfig.Collection` with the same one-liner pattern used for the generic dynamic config. **Why?** Three independent callers were each reading the percentage directly from dynamic config and ANDing it with `EmergencyOffboardingFromShardManager` in their own way. With the migration, that becomes a four-way logic that must agree across all three sites. Funneling it through one reader removes the drift risk and gets per-source comparison metrics for free. **How did you test it?** - `go build ./...` clean. - `go test -count=1 -race ./common/membership/... ./common/resource/... ./service/matching/... ./service/frontend/admin/... ./cmd/server/... ./common/dynamicconfig/...` all pass. - New `common/membership/percentage_onboarded_test.go` covers all four `useOperational × emergency` combinations, asserts both the return value and that the per-source gauges are emitted with correct `onboarding_source` + `onboarding_active` tags reflecting source values (not the override). - All consumer fixtures updated to use the generated `MockPercentageOnboarded`. **Potential risks** - The bootstrap change moves the operational configstore resolution out of `common/resource/resource_impl.go`. Tests that build `resource.Params{}` directly (instead of going through bootstrap) will see `params.OperationalConfigStore == nil` unless they set it explicitly. The in-repo tests that needed updates are included; out-of-tree consumers (if any) would need similar updates. - `matching.Config` loses the `PercentageOnboardedToShardManager` and `EmergencyOffboardingFromShardManager` fields. Any out-of-tree consumer that read them via reflection would break — none in this repo do. - Behavior change: the regular `matching.percentageOnboardedToShardManager` admin RPCs continue to work; the migration flag is `false` by default so production behavior is unchanged until an operator flips it. **Release notes** N/A — preparation for the shard-manager onboarding migration; no user-visible change until the flag is flipped. **Documentation Changes** N/A. Signed-off-by: Jakob Haahr Taankvist <jht@uber.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
2 participants
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.
What changed?
Adds four new dynamic config keys to control the polling cadence and timeouts of an upcoming operational dynamic config store:
system.operationalConfigStorePollIntervalsystem.operationalConfigStoreFetchTimeoutsystem.operationalConfigStoreUpdateTimeoutsystem.operationalConfigStoreUpdateRetryAttemptsThis is analogous to the existing isolation-group config store properties (
system.isolationGroupState*), which serve the same role for the existing isolation-group configstore. The new keys exist for an additional, separate operational configstore added in the follow-up.This is a key-definitions-only PR — nothing reads these values yet.
Why?
The 2s default puts the operational store at the configstore client's minimum poll interval. The motivation is to enable sub-2s regional convergence for safety-critical operational config (e.g. sharding-related rollout flags).
How did you test it?
go test -count=1 -race ./common/dynamicconfig/...— passes.Potential risks
Release notes
N/A
Documentation Changes
N/A.