risingwave Replace serde-protobuf and rust-protobuf with prost?

Replace serde-protobuf and rust-protobuf with prost?

Open ice1000 opened this issue 1 year ago • 3 comments

We're using 3 different protobuf crates at the very same time (🤯). Maybe we want to only use prost, as it's supported by:

https://github.com/tikv/grpc-rs
https://github.com/hyperium/tonic, the successor of tower-grpc

Plus, the code generated by prost is more friendly to move semantics.

P.S. PingCAP tried to migrate from rust-protobuf to prost in 2019, but the runtime performance didn't change much (*). However, there are still good reasons to migrate:

Less dependencies (faster build time)
Rust-protobuf generates bloated code that slows down compilation and takes lots of time to optimize (even faster build time)

*: I blame the migration strategy -- we wrapped the prost generated code with a rust-protobuf-like layer, and that's probably slowing down everything.

Aug 04 '22 20:08 ice1000

I believe the dependency can be removed as long as we go through the protobuf decoding thing in connectors. cc @tabVersion may take a look if you have time :)

Aug 05 '22 20:08 skyzh

Do we really use rust-protobuf in gRPC? I think it should have been replaced with prost long ago. 🥵

Aug 09 '22 05:08 BugenZhao

Do we really use rust-protobuf in gRPC? I think it should have been replaced with prost long ago. 🥵

No. The dependency is only introduced in tikv's rust-prometheus (as an optional feature, which we enabled) and risingwave-source. I think we can supersede the usage in risingwave-source.

Aug 09 '22 06:08 ice1000

This issue has been open for 60 days with no activity. Could you please update the status? Feel free to continue discussion or close as not planned.