[Dataflow Streaming] Optimize serialization of small commits #34355
Conversation
|
R: @scwhittle |
|
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control. If you'd like to restart, comment |
|
Run PreCommit Java |
|
Beyond profile difference, did you see cpu usage drop with the PR? kind of hard to tell from profiles other than that it moved locations. Some benefits does seem like it would reduce memory since we don't have serialized commit in addition to grpc message frames which might get sent. Downside is that we are now serializing beneath the synchronized block for the stream which seems like it could cause some more contention if there are multiple commit threads since they just pull randomly from commit stream pool and may overlap. If this also helps cpu maybe that is moot. |
| COMMIT_METHOD_DESCRIPTOR = | ||
| MethodDescriptor.<CommitRequest, StreamingCommitResponse>newBuilder() | ||
| .setFullMethodName(getCommitWorkStreamMethod().getFullMethodName()) | ||
| .setIdempotent(getCommitWorkStreamMethod().isIdempotent()) |
There was a problem hiding this comment.
can you do getCommitWorkStreamMethod().toBuilder() and just override what you need? then it's simpler and we don't have to worry about missing over new things added.
Prior to this change all commit chunks were serialized first and then sent to GRPC layer as StreamingCommitRequestChunk.
With this change small commits that don't span chunks are sent as it is without the intermediate serialization using StreamingCommitRequestChunkOverlay to GRPC layer. #33578
Example old profile:
Example new prrofile: