Unable to get pooling methods to gen

[joe-elliott]

I am currently attempting to put together a POC for using vtprotobuf in an application I work on. However the most promising feature (pooling) does not seem to exist in the generated code.

My command line:

protoc \
  -Ipkg/.patched-proto \
  --go_out=paths=source_relative:./pkg/tempopb/ \
  --go-grpc_out=paths=source_relative:./pkg/tempopb/ \
  --go-vtproto_out=paths=source_relative:./pkg/tempopb/ \
  --go-vtproto_opt=features=marshal+unmarshal+size+pool \
  pkg/.patched-proto/trace/v1/trace.proto

The output files seem to be generated correctly and there are no errors:

But I'm not seeing ResetVT, ReturnVTToPool, or *FromVTPool generated. I have tried with 0.2.0 as well as tip of main. I have also tried not specifying --go-vtproto_opt without luck.

I am seeing MarshalVT, MarshalToVT, SizeVT, UnmarshalVT, ... generated.

Thanks for your time!

[joe-elliott]

ok, I see now that I am supposed to use this option:

https://github.com/planetscale/vtprotobuf/blob/main/testproto/pool/pool.proto#L7

Struggling to reference the ext.proto file correctly, but messing around with it.

[joe-elliott]

Ok, I was able to get this to work by copying the ext.proto file into my project and then reference it from my proto. go mod vendor doesn't seem to want to bring this file in otherwise.

My file structure:

./pkg/tempopb/vtproto/ext.proto
./pkg/tempopb/tempo.proto

And then I am referencing it from tempo.proto like this:

import "pkg/tempopb/vtproto/ext.proto";

Is there a better way to do this that doesn't involve forking ext.proto into my repo?

[npordash]

@joe-elliott You can use --go-vtproto_opt=pool=<package>.<struct> arguments to protoc to specify exactly which generated structs should have pooling enabled. This way you don't need to use any proto extensions.

[vmg]

Yes, that's how we run the compilation in Vitess itself. Just use the --go_vtproto_opt CLI flag to enable specific options without having to modify the original .proto files. That will allow for fully backwards compatible ProtoBufs that can work with and without VTProtoBuf (in case you need to rollback or encounter bugs). Cheers!

[joe-elliott]

awesome, thx all!

[steve-gray]

I've been unable to get these methods to compile, I've tried for type X in package y (no namespace/prefixing to do)

--go-vtproto_opt=pool=y.x --go-vtproto_opt=features=pool+marshal+unmarshal+size

but no pool helpers - it does something though, it causes the Server/Client interface types to double-generate in both the .vt.pb files and the regular .pb files. Any chance of a super minimal example @vmg to help, or if @joe-elliott you could share a working example?

I'm happy to return it back as a PR into the README.md if I can see it working.

[vmg]

@steve-gray Vitess has been using that flag to generate our protos for a year now, and it's been working well for us. Here's a link to our Makefile:

https://github.com/vitessio/vitess/blob/main/Makefile#L243-L253

You can see that the objects passed to the flag must contain their full package namespace. Does this help fix your issues? How can we make the documentation easier to follow?

[jzelinskie]

Under each "available feature" in the README you could include how to use it in the protoc option.
I came here after hitting the same issue.