merlimat opened a new pull request, #4904:
URL: https://github.com/apache/bookkeeper/pull/4904
Descriptions of the changes in this PR:
### Motivation
BookKeeper generates its protocol, metadata and stream-storage messages, and
the stream-storage gRPC stubs, with LightProto 0.7.0. Since then LightProto has
gained:
- **Faster serialization and parsing.** The generated code no longer goes
through `sun.misc.Unsafe` on the hot path, which JDK 24+ penalizes on every
call. `clear()` is O(1), and large messages are written in place.
- **Correctness fixes:**
- `sint32` values with |v| ≥ 2^30 were written past their computed size.
- Bools are now read as protobuf reads them: any non-zero varint is `true`.
- Map entries with an omitted key or value hold protobuf's defaults.
- Messages from Netty leak-aware composite buffers parse correctly.
- Writing back a parsed message no longer sizes the output from the wire
size.
- **Open proto3 enums**, as in protobuf-java. An unknown number is kept and
written back instead of turning into the zero value, and every proto3 enum gets
an `UNRECOGNIZED` constant.
- **A cheaper gRPC marshaller.** It parses messages in place from the
transport buffers, and serializes without allocating a new array per message.
- **No `sun.misc.Unsafe` warning** when Netty itself doesn't use Unsafe.
### Changes
Bump `lightproto-maven-plugin.version` from 0.7.0 to 0.9.3. No source
changes are needed:
- BookKeeper doesn't use LightProto's JSON methods, which became opt-in in
0.7.2.
- Every `switch` over a proto3 enum in stream storage has a `default`
branch, so the new `UNRECOGNIZED` constant compiles and falls into the existing
error handling.
- No code writes into the arrays returned by `bytes` getters, which since
0.9.0 are shared with the message.
The version has to be 0.9.3 rather than 0.9.2. From 0.9.0 to 0.9.2, closing
the stream returned by the generated gRPC marshaller emptied it, even if it had
not been read yet:
- The stream-storage proxy (`ProxyHandlerRegistry` with
`IdentityInputStreamMarshaller`) forwards that stream to the next hop
unchanged, and grpc-java closes it as soon as `onMessage()` returns.
- A Netty frontend reads the stream before that close, so production setups
were fine.
- An in-process frontend reads it later, so the next hop got an empty
message. With 0.9.2, `TestStorageContainerStoreImpl` got empty responses and
`ProxyPingPongServiceTest` hung.
LightProto 0.9.3 keeps the stream readable until it has been read to the end
([streamnative/lightproto#54](https://github.com/streamnative/lightproto/pull/54)).
### Testing
- `mvn clean package -DskipTests -Ddistributedlog` passes.
- `apache-rat:check checkstyle:check spotbugs:check spotless:check` passes
on `bookkeeper-proto`, `bookkeeper-server` and the stream modules that generate
LightProto code.
- The `bookkeeper-server` protocol, metadata, cookie, DbLedgerStorage, auth
and `BookieInitializationTest` tests pass (232).
- The stream-storage tests pass with `-DstreamTests` (468): common,
including `ProxyPingPongServiceTest`; proto; statelib; clients; storage,
including `TestStorageContainerStoreImpl`; and server. No Netty leak reports.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
