Skip to content

Conversation

@sluongng
Copy link
Contributor

@sluongng sluongng commented Feb 5, 2025

Switch to use codecV2 backed by the default buffer pool to handle all
Marshaling/Unmarshaling by grpc clients/servers. The default buffer pool
is already setup here.

server/util/grpc_client/grpc_client.go
254:            experimental.WithBufferPool(mem.DefaultBufferPool()),

server/util/grpc_server/grpc_server.go
240:            experimental.BufferPool(mem.DefaultBufferPool()),

This does not affect the manual Marshaling/Unmarshaling that we does via
server/util/proto.{Marshal,Unmarshal}(). These should still use V1 for
the time being.

References:

@sluongng sluongng force-pushed the sluongng/grpc-use-codecv2 branch from 33a1648 to 7824eb9 Compare February 5, 2025 15:52
@sluongng sluongng changed the title sluongng/grpc use codecv2 vtprotocodec: user codecV2 for grpc client/server Feb 5, 2025
Copy link
Member

@tylerwilliams tylerwilliams left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This LGTM but we should have @luluz66 take a look before submitting.

Should we switch server/util/proto.{Marshal,Unmarshal}() to V2 also (in another cl) ?

@sluongng sluongng force-pushed the sluongng/grpc-use-codecv2 branch from 7824eb9 to 8216231 Compare February 6, 2025 13:52
Switch to use codecV2 backed by the default buffer pool to handle all
Marshaling/Unmarshaling by grpc clients/servers. The default buffer pool
is already setup here.

```
server/util/grpc_client/grpc_client.go
254:		experimental.WithBufferPool(mem.DefaultBufferPool()),

server/util/grpc_server/grpc_server.go
240:		experimental.BufferPool(mem.DefaultBufferPool()),
```

This does not affect the manual Marshaling/Unmarshaling that we does via
`server/util/proto.{Marshal,Unmarshal}()`. These should still use V1 for
the time being.
@sluongng sluongng force-pushed the sluongng/grpc-use-codecv2 branch from 8216231 to 3ed10ac Compare February 10, 2025 10:32
@sluongng sluongng force-pushed the sluongng/grpc-use-codecv2 branch from 3ed10ac to 4341d9b Compare February 10, 2025 11:19
@sluongng sluongng requested a review from luluz66 February 10, 2025 14:47
@sluongng sluongng merged commit 12d5a13 into master Feb 10, 2025
10 checks passed
@sluongng sluongng deleted the sluongng/grpc-use-codecv2 branch February 10, 2025 21:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants