szetszwo commented on code in PR #1345:
URL: https://github.com/apache/ratis/pull/1345#discussion_r2789342000
##########
ratis-grpc/src/main/java/org/apache/ratis/grpc/client/GrpcClientProtocolClient.java:
##########
@@ -82,6 +82,7 @@ public class GrpcClientProtocolClient implements Closeable {
private final ManagedChannel clientChannel;
private final ManagedChannel adminChannel;
+ private final int maxMessageSize;
Review Comment:
Let's use `SizeInBytes`. The String will look when printing it out.
##########
ratis-grpc/src/main/java/org/apache/ratis/grpc/client/GrpcClientProtocolClient.java:
##########
@@ -121,7 +123,7 @@ public class GrpcClientProtocolClient implements Closeable {
}
private ManagedChannel buildChannel(String address, SslContext sslContext,
- SizeInBytes flowControlWindow, SizeInBytes maxMessageSize) {
+ SizeInBytes flowControlWindow, SizeInBytes maxMessageSizeConfig) {
Review Comment:
Since it becomes a field, let's remove the parameter.
##########
ratis-grpc/src/main/java/org/apache/ratis/grpc/client/GrpcClientProtocolClient.java:
##########
@@ -337,8 +348,15 @@ CompletableFuture<RaftClientReply>
onNext(RaftClientRequest request) {
if (f == null) {
return JavaUtils.completeExceptionally(new
AlreadyClosedException(getName() + " is closed."));
}
+ final RaftClientRequestProto proto;
+ try {
+ proto = toRaftClientRequestProto(request);
Review Comment:
This is a good idea to validate to request first. Let's check it in the
very beginning of this method.
##########
ratis-grpc/src/main/java/org/apache/ratis/grpc/client/GrpcClientProtocolClient.java:
##########
@@ -236,6 +238,15 @@ public RaftPeer getTarget() {
return target;
}
+ private RaftClientRequestProto toRaftClientRequestProto(RaftClientRequest
request) throws IOException {
+ final RaftClientRequestProto proto =
ClientProtoUtils.toRaftClientRequestProto(request);
+ if (proto.getSerializedSize() > maxMessageSize) {
+ throw new IOException(getName() + ": Message size:" +
proto.getSerializedSize()
+ + " exceeds maximum:" + maxMessageSize);
Review Comment:
- Use IllegalArgumentException
- Use "request serialized size" instead of "Message size"
- Include the request.
--
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]
