Watch: inefficient event distribution

[gyuho]

Separated from #3848.

inefficiency of event distribution through watch

problem: when reading value and sending out event through gRPC, we now unmarshal it first, then marshal it to byte slice again to distribute each time. This is inefficient.

[ajityagaty]

@gyuho @xiang90 Would it be possible to provide more pointers for this one?

[xiang90]

@ajityagaty

Let's say we have 1000 clients watch on the same key foo. If there is a change on key foo, we need to broadcast the event to all 1000 clients. The core content, the key change, is same for 1000 clients. Right now, we still allocation space and encode the key changes 1000 times which is inefficient. Ideally, we should just do the key change encoding once.

[ajityagaty]

@xiang90 Thanks for the explanation ! Could you also point me to the code path?

[shuqilou]

Do you have token some measures to resolve such inefficient watch mechanism?

[xiang90]

@shuqilou This can be an easy fix. We have not measure how efficient it is, it is actually not common to have 1000s of watchers watching on the same key. It would be great if you can:

1. measure the overhead
2. fix it

[xiang90]

I talked this issue with @awalterschulze : here is his suggestion

Disclaimer: This is a hack, but its not the worst hack, since clients won't notice.
Solution: You can concatenate two messages and unmarshal them as one.
Explanation:

message WatchResponse1 {
   int64 watch_id = 1;
}
message WatchResponse2 {
  repeated mvccpb.Event events = 2;
}

Marshal these two messages and concatenate their bytes and then send it.
Now this can be unmarshaled by

message WatchResponse {
  int64 watch_id = 1;

  repeated mvccpb.Event events = 2;
}

So this means that you can Marshal WatchResponse2 once and simply Marshal WatchResponse1 for every client.
The Marshaled bytes will look the same. You can even test this.
This is because each field is marshaled separately and simply concatenated. This is how the > serialization format works.

You can also swap the order in which you send these.
So first send the bytes for WatchResponse2 and then Marshal and send WatchResponse1.
The Unmarshalers of all protobuf libraries are capable (or should be by spec) of dealing with > receiving fields in different orders.

Also you don't even have to declare two messages right.
Because unset fields are not sent.
You can simply set the fields you want to Marshal.
So have one message

message WatchResponse {
  int64 watch_id = 1;

  repeated mvccpb.Event events = 2;
}

Set watch_id to nil or zero, depending on whether you are using proto2 or proto3 and then set the > events and Marshal.
Set events to nil and set watch_id to its unique value and Marshal
Concatenate the bytes and should have the same bytes you would have had, had you done this in > one go.

nullable=false in proto2, might introduce problems with this approach as watch_id will then always > be set.
But if you send the Marshaled events first and then append the Marshaled watch_id, the watch_id > value will be overridden with the new value. This is called merging of messages. I am not a fan of > this, but it works.

Side note repeated fields when not packed are also each marshaled separately.
So you could even marshal these separately and simply append them, but I don't think that will be > useful in this case.
But maybe if you have maybe duplicate events you could use this to optimize the serialization.

Also

Just embed your message into another struct which has the cached bytes.

type cached struct {
   WatchResponse *WatchResponse
   cache []byte
}

func (this *cached) Marshal() ([]byte, error) {
  // do the marshaling in the way you want by calling proto.Marshal / MarshalTo in appropriate ways
}

func (this *cached) IsProto() {}

You will need to implement the proto.Message interface.

If you are using the gogo/protobuf/codec
It probably has a bug gogo/protobuf#205
But when its fixed you might want to rather implement the MarshalTo method to get extra speed.

[xiang90]

@mqliang @sinsharat @ajityagaty @shuqilou

Hope this can help to explain this issue more.

[xiang90]

move to 3.3. with the help of gRPC proxy, watch load is not a huge issue.

[xiang90]

after second thought, we should just close this one. gRPC proxy should just solve this problem.