Commit 9633bb7
authored
Migrate stream gRPC protos to LightProto (#4783)
* Migrate stream gRPC protos to LightProto
Migrates the remaining stream protos (the ones with gRPC services and
their imports) from `protobuf-java` to LightProto:
- `stream/proto/src/main/proto/`: cluster, common, kv, kv_rpc, kv_store,
storage, stream — all switched to lightproto-only generation. The
protobuf-maven-plugin (and protoc-gen-grpc-java) are removed; the
lightproto plugin produces both message classes and gRPC service stubs
natively (the same pattern oxia-java uses).
- `stream/tests-common/src/main/proto/rpc.proto`: same migration. The
unused `proto2_coder_test_messages.proto` is deleted (it relied on
proto2 `extensions` which lightproto doesn't support, and had no Java
references).
Java sources and tests across stream/{api,statelib,storage/api,storage/impl,
clients/java/{base,kv,all},server,common,tests-common}, tools/stream and
tests/integration/cluster were updated to the lightproto API:
- `Foo.newBuilder().setX(v).build()` → `new Foo().setX(v)` (no .build())
- `Foo.newBuilder(other)` → `new Foo().copyFrom(other)`
- nested-message setters: `outer.setInner(inner)` → `outer.setInner().copyFrom(inner)`
- repeated-message setters: `addRequests(req)` → `addRequest().copyFrom(req)`
(singular `addX()` returns the new instance to mutate)
- bytes: `UnsafeByteOperations.unsafeWrap(b)` → just `b`; `getX().toByteArray()`
→ just `getX()` (returns byte[] directly)
- parsing: static `Foo.parseFrom(byte[])` → `Foo f = new Foo(); f.parseFrom(b)`
- `getXMap()` / `getXList()` getter renames where lightproto uses different
pluralisation (e.g. `getRoEndpointCount` → `getRoEndpointsCount`,
`getRequests(i)` → `getRequestAt(i)`)
- boolean accessors `getX()` → `isX()` for primitive booleans
- enums: drop `UNRECOGNIZED` checks (lightproto enums don't have it)
- `InvalidProtocolBufferException` / `CodedOutputStream` removed; lightproto
parses to/from `ByteBuf` directly and throws `RuntimeException` on bad input
LightProto's plugin only emits async + blocking stubs — it does not emit
`*FutureStub`. Where the existing client code relied on `ListenableFuture`
APIs, four hand-written `*FutureStub` adapters were added under
`stream/clients/java/base/src/main/java/org/apache/bookkeeper/clients/grpc/`
(`RootRangeServiceFutureStub`, `MetaRangeServiceFutureStub`,
`StorageContainerServiceFutureStub`, `TableServiceFutureStub`). They wrap
`AbstractStub` and call `ClientCalls.futureUnaryCall` against the
lightproto-generated `MethodDescriptor`s, so the rest of the client code
needed only an import swap.
A handful of tests had `assertSame(req, receivedReq)` /
`assertTrue(a == b)` checks that previously worked because gRPC's
in-process transport happened to pass protobuf-java messages by reference.
LightProto's gRPC marshaller always serializes/deserializes (matching the
oxia/pulsar implementation), so those identity checks were switched to
`assertEquals` / `.equals(...)`.
* Slice rKey in routing header to avoid ByteBuf aliasing bug
`bkctl table put foo bar` and `Table#put(key, value)` route requests by
passing the key as both the partition key and the local key. The simple
table flow ends up storing the *same* ByteBuf reference in
`request.key` and `request.header.rKey`. LightProto's serializer for
`bytes` fields backed by ByteBuf calls `dst.writeBytes(src)`, which
advances `src`'s readerIndex; once the request body's `key` field has
been written, the same ByteBuf has zero readable bytes left, so when
the routing header's `rKey` field is serialized next it writes nothing.
The routing header therefore reaches the server with an empty rKey,
the put goes to the wrong storage container, and a cross-language
reader (e.g. the Python integration test) computing the correct
routing key against the actual key bytes finds no value.
Fix: pass `pKey.slice()` to `RoutingHeader#setRKey` in both
`PByteBufSimpleTableImpl` and `PByteBufTableRangeImpl` so the routing
header gets its own readerIndex independent of any other ByteBuf
fields aliasing the same buffer.
* Slice every ByteBuf passed to a lightproto setX(ByteBuf)
The previous fix only sliced pKey before setRKey. There's a second
manifestation of the same underlying issue: the integration test
TableClientTest reuses the same lKey/value ByteBuf across two
consecutive put() calls. After the first request is serialized,
lightproto's writeBytes(src) has consumed lKey's readerIndex, so the
second request's setKey(lKey) records _keyLen = 0, and the wire format
omits the key field entirely. The second put silently writes under an
empty key, prevKv comes back null, and the test fails.
Wrap every user-supplied ByteBuf with .slice() inside KvUtils before
handing it to a lightproto setter (newPutRequest, newRangeRequest,
newDeleteRequest, newIncrementRequest, populateProtoCompare,
populateProtoPutRequest, populateProtoDeleteRequest,
populateProtoRangeRequest). The slice gives lightproto its own
readerIndex over the same backing data, so mutating it during
serialization doesn't affect the caller's buffer or any other field
backed by the same ByteBuf.
The right long-term fix is in lightproto itself — setX(ByteBuf) should
either slice internally or use the non-mutating dst.writeBytes(src,
idx, len). Worth filing upstream.1 parent c286628 commit 9633bb7
103 files changed
Lines changed: 2261 additions & 2194 deletions
File tree
- stream
- clients/java
- all/src
- main/java/org/apache/bookkeeper/clients
- admin
- test/java/org/apache/bookkeeper/clients
- admin
- base/src
- main/java/org/apache/bookkeeper/clients
- grpc
- impl
- channel
- container
- internal
- utils
- test/java/org/apache/bookkeeper/clients
- grpc
- impl
- channel
- container
- internal
- api
- kv/src
- main/java/org/apache/bookkeeper/clients/impl/kv
- test/java/org/apache/bookkeeper/clients/impl/kv
- common/src/test/java/org/apache/bookkeeper/common/grpc
- proxy
- stats
- proto
- src
- main
- java/org/apache/bookkeeper/stream/protocol
- util
- proto
- test/java/org/apache/bookkeeper/stream/protocol/util
- server/src/main/java/org/apache/bookkeeper/stream
- cluster
- server
- grpc
- statelib/src
- main/java/org/apache/bookkeeper/statelib/impl
- mvcc
- op/proto
- rocksdb/checkpoint
- test/java/org/apache/bookkeeper/statelib/impl
- mvcc/op/proto
- rocksdb/checkpoint
- storage/impl/src
- main/java/org/apache/bookkeeper/stream/storage/impl
- grpc
- kv
- metadata
- stream
- routing
- service
- test/java/org/apache/bookkeeper/stream/storage/impl
- grpc
- kv
- metadata
- stream
- routing
- sc
- service
- tests-common
- src/main
- java/org/apache/bookkeeper/tests/rpc
- proto
- tests/integration/cluster/src/test/java/org/apache/bookkeeper/tests/integration/stream
- tools/stream/src/main/java/org/apache/bookkeeper/stream/cli/commands
- namespace
- table
Some content is hidden
Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
Lines changed: 3 additions & 4 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
34 | 34 | | |
35 | 35 | | |
36 | 36 | | |
| 37 | + | |
37 | 38 | | |
38 | 39 | | |
39 | 40 | | |
| |||
43 | 44 | | |
44 | 45 | | |
45 | 46 | | |
46 | | - | |
47 | | - | |
48 | 47 | | |
49 | 48 | | |
50 | 49 | | |
| |||
63 | 62 | | |
64 | 63 | | |
65 | 64 | | |
66 | | - | |
| 65 | + | |
67 | 66 | | |
68 | 67 | | |
69 | 68 | | |
| |||
74 | 73 | | |
75 | 74 | | |
76 | 75 | | |
77 | | - | |
| 76 | + | |
78 | 77 | | |
79 | 78 | | |
80 | 79 | | |
| |||
Lines changed: 2 additions & 3 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
34 | 34 | | |
35 | 35 | | |
36 | 36 | | |
| 37 | + | |
37 | 38 | | |
38 | 39 | | |
39 | 40 | | |
| |||
43 | 44 | | |
44 | 45 | | |
45 | 46 | | |
46 | | - | |
47 | | - | |
48 | 47 | | |
49 | 48 | | |
50 | 49 | | |
| |||
66 | 65 | | |
67 | 66 | | |
68 | 67 | | |
69 | | - | |
| 68 | + | |
70 | 69 | | |
71 | 70 | | |
72 | 71 | | |
| |||
Lines changed: 33 additions & 45 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
56 | 56 | | |
57 | 57 | | |
58 | 58 | | |
59 | | - | |
60 | | - | |
61 | | - | |
62 | | - | |
63 | | - | |
64 | | - | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
65 | 84 | | |
66 | 85 | | |
67 | 86 | | |
| |||
82 | 101 | | |
83 | 102 | | |
84 | 103 | | |
85 | | - | |
86 | | - | |
87 | | - | |
88 | | - | |
89 | | - | |
| 104 | + | |
90 | 105 | | |
91 | 106 | | |
92 | 107 | | |
| |||
105 | 120 | | |
106 | 121 | | |
107 | 122 | | |
108 | | - | |
109 | | - | |
110 | | - | |
111 | | - | |
112 | | - | |
113 | | - | |
| 123 | + | |
114 | 124 | | |
115 | 125 | | |
116 | 126 | | |
117 | | - | |
118 | | - | |
119 | | - | |
120 | | - | |
121 | | - | |
122 | | - | |
| 127 | + | |
123 | 128 | | |
124 | 129 | | |
125 | 130 | | |
| |||
146 | 151 | | |
147 | 152 | | |
148 | 153 | | |
149 | | - | |
150 | | - | |
151 | | - | |
152 | | - | |
153 | | - | |
| 154 | + | |
154 | 155 | | |
155 | 156 | | |
156 | 157 | | |
| |||
171 | 172 | | |
172 | 173 | | |
173 | 174 | | |
174 | | - | |
175 | | - | |
176 | | - | |
177 | | - | |
178 | | - | |
179 | | - | |
| 175 | + | |
180 | 176 | | |
181 | 177 | | |
182 | 178 | | |
183 | | - | |
184 | | - | |
185 | | - | |
186 | | - | |
187 | | - | |
188 | | - | |
| 179 | + | |
189 | 180 | | |
190 | 181 | | |
191 | 182 | | |
| |||
216 | 207 | | |
217 | 208 | | |
218 | 209 | | |
219 | | - | |
220 | | - | |
221 | | - | |
222 | | - | |
223 | | - | |
| 210 | + | |
| 211 | + | |
224 | 212 | | |
225 | 213 | | |
226 | 214 | | |
| |||
Lines changed: 26 additions & 14 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
43 | 43 | | |
44 | 44 | | |
45 | 45 | | |
46 | | - | |
47 | | - | |
48 | | - | |
49 | | - | |
50 | | - | |
51 | | - | |
52 | | - | |
53 | | - | |
54 | | - | |
55 | | - | |
56 | | - | |
57 | | - | |
58 | | - | |
59 | | - | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
60 | 72 | | |
61 | 73 | | |
62 | 74 | | |
| |||
Lines changed: 56 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
Lines changed: 96 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
0 commit comments