[AI-8th] feat: jraft-rheakv support gRPC communication#1261
[AI-8th] feat: jraft-rheakv support gRPC communication#1261jervyclaw wants to merge 4 commits intosofastack:masterfrom
Conversation
This commit adds gRPC communication support for jraft-rheakv. Key changes: 1. Added GrpcSerializationTransfer interface and GrpcSerializationTransferException in rpc-grpc-impl module for transfer serialization framework. 2. Added GrpcProtobufTransferHelper utility class to convert between protobuf and Java bean objects. 3. Updated GrpcServer to use transfer helpers for request/response conversion. 4. Updated GrpcClient to use transfer helpers for request/response conversion. 5. Created new rheakv-grpc module with: - Protobuf definitions for rhea store and pd commands - Transfer classes for all store commands (Get, Put, Delete, Scan, etc.) - Transfer classes for all pd commands (StoreHeartbeat, RegionHeartbeat, etc.) - GrpcProtoRegistryHelper to register all transfers and serializers Usage: Call GrpcProtoRegistryHelper.registryAll() before starting the StoreEngine when using gRPC as the RPC implementation. This fixes the issue where rheakv commands (GetRequest, PutRequest, etc.) were not recognized by gRPC because they are Hessian-serializable Java objects rather than protobuf messages. The transfer layer bridges this gap by converting between protobuf and Java beans using Hessian serialization. Co-authored-by: JervyClaw AI Agent <jervyclaw@gmail.com>
P1 Issue 1 - Add rheakv-grpc to reactor modules: - Add <module>rheakv-grpc</module> to jraft-rheakv/pom.xml modules list so it's included in normal Maven builds. P1 Issue 2 - Register CASAll RPC types in gRPC registry: - Add CASEntry, CASAllRequest, CASAllResponse protobuf messages to rheakv.proto - Regenerate RheakvRpc.java with protoc 3.17.3 - Create CASAllRequestProtobufTransfer and CASAllResponseProtobufTransfer - Register CASAllRequest/CASAllResponse in GrpcProtoRegistryHelper (both marshaller and protobuf transfer registrations) P2 Issue - Propagate codec failures in NodeExecute conversion: - NodeExecuteRequestProtobufTransfer was catching CodecException and continuing with e.printStackTrace(), returning partially populated requests. Now re-throws as GrpcSerializationTransferException in both protoBufTransJavaBean and javaBeanTransProtobufBean paths.
Formatter changes: - Added Apache license header to RheakvRpc.java and RheakvPDRpc.java - Reformatted protobuf-generated files to comply with 120-char line limit - Fixed line wrapping in CASAllRequestProtobufTransfer.java - Fixed line wrapping in CASAllResponseProtobufTransfer.java Run 'mvn clean install' with Java 17 to apply formatting before commit.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a43031400d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| throws CodecException { | ||
| RheakvPDRpc.RegionHeartbeatRequest.Builder builder = RheakvPDRpc.RegionHeartbeatRequest.newBuilder() | ||
| .setBaseRequest(BaseRequestProtobufTransfer.javaBeanTransProtobufBean(regionHeartbeatRequest)) | ||
| .setStoreId(regionHeartbeatRequest.getStoreId()).setLeastKeysOnSplit(regionHeartbeatRequest.getStoreId()); |
There was a problem hiding this comment.
RegionHeartbeatRequestTransfer.javaBeanTransProtobufBean writes leastKeysOnSplit from getStoreId() instead of getLeastKeysOnSplit(), so every outbound PD heartbeat carries the wrong split threshold. This can drive incorrect split scheduling decisions because PD receives a store ID value in the least-keys field.
Useful? React with 👍 / 👎.
| byte[] bytes = SerializerManager.getSerializer(SerializerManager.Hessian2).serialize( | ||
| regionHeartbeatResponse.getValue()); | ||
| builder.addValue(ByteString.copyFrom(bytes)); |
There was a problem hiding this comment.
RegionHeartbeatResponseTransfer.javaBeanTransProtobufBean serializes the entire List<Instruction> into a single bytes entry, but protoBufTransJavaBean deserializes each list element as a single Instruction. For non-empty responses this mismatch causes decode failures (or corrupted data) on the receiver because one serialized list blob is treated as one instruction object.
Useful? React with 👍 / 👎.
| .setRegionEpoch( | ||
| RheakvRpc.RegionEpoch.newBuilder().setConfVer(baseRequest.getRegionEpoch().getConfVer()) | ||
| .setVersion(baseRequest.getRegionEpoch().getVersion()).build()).build(); |
There was a problem hiding this comment.
BaseRequestProtobufTransfer.javaBeanTransProtobufBean dereferences baseRequest.getRegionEpoch() unconditionally, which throws when requests intentionally omit epoch (for example RangeSplitRequest from DefaultRheaKVCliService only sets regionId/newRegionId). Under gRPC this makes such requests fail with NPE before they are sent.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6c5c1db4c6
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| public RheakvRpc.PutResponse javaBeanTransProtobufBean(final PutResponse putResponse) throws CodecException { | ||
| final RheakvRpc.BaseResponse baseResponse = BaseResponseProtobufTransfer.javaBeanTransProtobufBean(putResponse); | ||
| final RheakvRpc.PutResponse.Builder response = RheakvRpc.PutResponse.newBuilder().setBaseResponse(baseResponse); | ||
| response.setValue(putResponse.getValue()); |
There was a problem hiding this comment.
This setter auto-unboxes Boolean to primitive bool, so it throws when putResponse.getValue() is null. That null case is normal on failures because DefaultRegionKVService#setFailure populates error but leaves value unset; in gRPC mode, serializing such error responses will fail with NullPointerException instead of returning the error to clients. The same pattern appears in other boolean response transfers (e.g. delete/merge/range-split), so these serializers should skip value when it is null.
Useful? React with 👍 / 👎.
| throws CodecException { | ||
| return RheakvPDRpc.CreateRegionIdResponse.newBuilder() | ||
| .setBaseResponse(BaseResponseProtobufTransfer.javaBeanTransProtobufBean(createRegionIdResponse)) | ||
| .setValue(createRegionIdResponse.getValue()).build(); |
There was a problem hiding this comment.
CreateRegionIdResponse#getValue() is a boxed Long, but protobuf setValue expects primitive int64; when PD returns an error (for example NOT_LEADER/exception in DefaultPlacementDriverService), value remains null and this line throws during serialization. That drops the intended error response for clients. GetStoreIdResponseTransfer has the same unguarded unboxing pattern.
Useful? React with 👍 / 👎.
| .javaBeanTransProtobufBean(keyLockResponse); | ||
| final RheakvRpc.KeyLockResponse.Builder response = RheakvRpc.KeyLockResponse.newBuilder().setBaseResponse( | ||
| baseResponse); | ||
| response.setValue(lockOwnerProtobufTransfer.javaBeanTransProtobufBean(keyLockResponse.getValue())); |
There was a problem hiding this comment.
Failed lock/unlock requests return error without a lock owner, so keyLockResponse.getValue() can be null; passing that into LockOwnerProtobufTransfer dereferences owner.getId() and throws, preventing the error response from being sent. This makes lock RPC failures crash serialization instead of propagating BaseResponse.error (same issue in KeyUnLockResponseProtobufTransfer).
Useful? React with 👍 / 👎.
1 participant
Summary
Add gRPC communication support for jraft-rheakv module.
Key features:
This enables gRPC as an alternative to the existing RPC implementation for rheakv.