Last active
December 6, 2022 10:01
-
-
Save tbg/12aff2bf80e8e3107e51ffe390b7fa2c to your computer and use it in GitHub Desktop.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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