Skip to content

Instantly share code, notes, and snippets.

@tbg
Last active December 6, 2022 10:01
Show Gist options
  • Save tbg/12aff2bf80e8e3107e51ffe390b7fa2c to your computer and use it in GitHub Desktop.
Save tbg/12aff2bf80e8e3107e51ffe390b7fa2c to your computer and use it in GitHub Desktop.
Advent of protobuf hackery
You have this proto message.
message RequestHeader {
// some fields
}
You want to add a field to it that is "there" only during crdb_test.
Cannot have codegen depend on the build tag.
Instead: use a casttype
Field not being there means
- does not bloat the sizeof(RequestHeader) when !crdb_test
- does not show up on the wire outside of crdb_test
- does not show up on the wire when zero, even under crdb_test (test ergonomics)
can't use primitive type -> will blow sizeof, but is good on the wire
can't use empty message with casttype -> good for sizeof, but shows up on the wire (proto tag)
actually, second option can be made work.
do the following:
- disable marshaler codegen for RequestHeader
- RequestHeaderPure: like RH, but without the extra field
- RequestHeaderCrdbTest: exactly like RH, but with generated marshaler
- then write custom marshaler methods for RequestHeader which delegate
- if not under test or field is zero, delegate to RequestHeaderPure
- otherwise, RequestHeaderCrdbTest
```go
// Marshal implements protoutil.Message.
func (r *RequestHeader) Marshal() ([]byte, error) {
if buildutil.CrdbTestBuild && r.KVNemesisSeq.Get() != 0 {
rt := r.crdbTest()
return rt.Marshal()
}
p := r.pure()
return p.Marshal()
}
```
This checks all of the boxes:
- when not crdb_test struct size is same (empty struct{} doesn't add anything). And we effectively marshal a RequestHeaderPure, which doesn't know about the struct and so adds no tag.
- when in crdb_test, struct size changes but that's ok. If field is zero, we marshal as RequestHeaderPure so tests that don't set this field (i.e. everything but KVNemesis) seems exactly the same value sizes, etc, so test ergonomics are good.
so effectively the field is invisible except if you're KVNemesis.
PS: we're not married to this - it seems too clever. It's the kind of thing likely to cause pain when we migrate off protobuf. We're ready to remove this when we see any problems with it.
We have four bytes of padding available on RequestHeader, so we can put an int32 there unconditionally. We can almost do the same in MVCCValueHeader (the other proto where we perform this trick) except we're one boolean short of having enough padding.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment