Change cadence config to dynamic #851
Conversation
| _matchingDomainTaskListRoot + "enableSyncMatch", | ||
| _matchingDomainTaskListRoot + "updateAckInterval", | ||
| _matchingDomainTaskListRoot + "idleTasklistCheckInterval", | ||
| "testGetPropertyKey", |
There was a problem hiding this comment.
Instead of having keys as a flat array, can we make it a map of service to list of configs?
I think it will be much cleaner if we separate them out per service.
Another thing is I don't like we rely on the ordering of how keys are defined. Can we convert them into a map too, otherwise it will be very error prone.
Basically the keys definition would look something like:
map[serviceIdx]map[int]string
service/frontend/workflowHandler.go
Outdated
| if getRequest.GetMaximumPageSize() == 0 { | ||
| getRequest.MaximumPageSize = common.Int32Ptr(wh.config.DefaultHistoryMaxPageSize) | ||
| if getRequest.GetMaximumPageSize() <= 0 { | ||
| domainFilter := dynamicconfig.DomainFilter(getRequest.GetDomain()) |
There was a problem hiding this comment.
Can we define a strongly typed helper function on config which takes the domain when reading any config which supports filter on domain. Then we can have the code to create filter in one location and have a straightforward API for rest of the code to load the config.
There was a problem hiding this comment.
add GetIntPropertyWithDomainFilter
| ExecutionMgrNumConns: "history.executionMgrNumConns", | ||
| HistoryMgrNumConns: "history.historyMgrNumConns", | ||
| MaximumBufferedEventsBatch: "history.maximumBufferedEventsBatch", | ||
| ShardUpdateMinInterval: "history.shardUpdateMinInterval", | ||
| } | ||
|
|
||
| const ( | ||
| // The order of constants is important. It should match the order in the keys array above. |
There was a problem hiding this comment.
Probably this comment is not relevant anymore. Delete this.
service/frontend/workflowHandler.go
Outdated
| if getRequest.GetMaximumPageSize() == 0 { | ||
| getRequest.MaximumPageSize = common.Int32Ptr(wh.config.DefaultHistoryMaxPageSize) | ||
| if getRequest.GetMaximumPageSize() <= 0 { | ||
| getRequest.MaximumPageSize = common.Int32Ptr(int32( |
There was a problem hiding this comment.
This is still too verbose. Can we have a strongly typed API on config like:
GetHistoryMaxPageSize(domain)?
No description provided.