multiple instances support for grpc broker. #235
Conversation
Signed-off-by: morvencao <[email protected]>
|
/ok-to-test |
|
/hold depends on #232 |
morvencao
force-pushed
the
br_grpc_ha
branch
5 times, most recently
from
1ce55ab to
580601f
Compare
pkg/controllers/event_handler.go
Outdated
| // check if all instances have processed the event | ||
| if !compareStrings(activeInstances, eventInstances) { | ||
| klog.V(10).Infof("Event %s is not processed by all instances, handled by %v, active instances %v", event.ID, eventInstances, activeInstances) | ||
| return nil |
There was a problem hiding this comment.
do we need requeue here (return an error)? or there are some processes can retrigger this?
There was a problem hiding this comment.
In this case, reconcileDate is not set, the 'SyncEvent' will trigger event be processed again?
If we return an error here, the current instance will process the event again, but the event may not be handled by other instances. so return an error doesn't help.
There was a problem hiding this comment.
I'm thinking maybeactiveInstances is_subset_of eventInstances make more sense?
There was a problem hiding this comment.
If we return an error here, the current instance will process the event again, but the event may not be handled by other instances. so return an error doesn't help.
so this means the ReconciledDate will be marked finally by the the "last" instance, right?
There was a problem hiding this comment.
yes, I added some comments to explain different cases.
| if svcErr != nil { | ||
| // if the resource is not found, it indicates the resource has been handled by | ||
| // other instances, so we can mark the event as reconciled and ignore it. | ||
| if svcErr.Is404() { |
Signed-off-by: morvencao <[email protected]>
morvencao
force-pushed
the
br_grpc_ha
branch
from
580601f to
29049f9
Compare
ref: https://issues.redhat.com/browse/ACM-16035
/assign @skeeey @qiujian16 @clyang82