Bug fixes of multi_zone consumer group #170
Conversation
services/controllerhost/consumer.go
Outdated
| return strings.EqualFold(zone, cgDesc.GetActiveZone()) | ||
| } | ||
|
|
||
| if len(dConfig.ActiveZone) > 0 { | ||
| return strings.EqualFold(zone, dConfig.ActiveZone) | ||
| } | ||
|
|
||
| return true | ||
| return false |
There was a problem hiding this comment.
can you also add a log or metric when both dConfig.ActiveZone and cgDesc.GetActiveZone() are empty ?
just want to make sure we have mechanisms to verify correctness as we roll this out initially.
| @@ -385,19 +385,17 @@ func (mcp *Mcp) GetOutputHosts(ctx thrift.Context, inReq *c.GetOutputHostsReques | |||
| } | |||
|
|
|||
| response := func(outputHostIDs []string, err error) (*c.GetOutputHostsResult_, error) { | |||
| if len(outputHostIDs) < 1 { | |||
| // only count as failure if our answer contains no endpoints at all | |||
| if err != nil { | |||
There was a problem hiding this comment.
let's revert to the same code as before:
https://github.com/uber/cherami-server/blob/f47853600f4ebb2fc74a41e3a7d3e11ab6f07031/services/controllerhost/controllerhost.go
checking for err != nil still changes the behavior.
|
For the bug fix in controller, I reverted back to the original change but added another variable in the cache called consumeDisabled to distinguish the multi_zone scenario(a ok case) and the no output host case(an error case). Also I fixed another bug in replicator. In authoritative zone, we skipped the destination and cg reconciliation, so localCgs variable is always empty, which results in no cg extent reconciliation for authoritative zone. Now I'm moving the computation of localCgs to the outer scope. |
|
@venkat1109 could you take another look? |
| } | ||
|
|
||
| return true | ||
| context.log.Warn(`no active zone from dynamic config !`) |
There was a problem hiding this comment.
would be a good idea to emit a metric here
Fix a couple of bugs: