multiple instances support for grpc broker. #235
Conversation
|
/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() { |
morvencao
force-pushed
the
br_grpc_ha
branch
from
580601f to
29049f9
Compare
Signed-off-by: morvencao <[email protected]>
morvencao
force-pushed
the
br_grpc_ha
branch
2 times, most recently
from
1c1601c to
1e1e107
Compare
Signed-off-by: morvencao <[email protected]>
morvencao
force-pushed
the
br_grpc_ha
branch
5 times, most recently
from
acaa814 to
5f9e8f3
Compare
cmd/maestro/server/grpc_broker.go
Outdated
| } | ||
|
|
||
| klog.V(4).Infof("send the event to spec subscribers, %s", evt) | ||
|
|
||
| // WARNING: don't use "pbEvt, err := pb.ToProto(evt)" to convert cloudevent to protobuf | ||
| pbEvt := &pbv1.CloudEvent{} | ||
| if err = grpcprotocol.WritePBMessage(context.TODO(), binding.ToMessage(evt), pbEvt); err != nil { | ||
| return fmt.Errorf("failed to convert cloudevent to protobuf: %v", err) | ||
| klog.Errorf("failed to convert cloudevent to protobuf: %v", err) |
There was a problem hiding this comment.
also record the resource ID
Signed-off-by: morvencao <[email protected]>
morvencao
force-pushed
the
br_grpc_ha
branch
from
5f9e8f3 to
9172a13
Compare
ref: https://issues.redhat.com/browse/ACM-16035
/assign @skeeey @qiujian16 @clyang82