szetszwo commented on code in PR #6049:
URL: https://github.com/apache/ozone/pull/6049#discussion_r1504724961
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/replication/SendContainerRequestHandler.java:
##########
@@ -98,6 +103,13 @@ public void onNext(SendContainerRequest req) {
nextOffset += length;
} catch (Throwable t) {
onError(t);
+ } finally {
+ if (marshaller != null) {
Review Comment:
In the current code, the `marshaller` is never null. Indeed, it is a good
idea to initially the marshaller objects only if zero-copy is enabled.
```java
+++
b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/replication/GrpcReplicationService.java
@@ -61,19 +61,25 @@ public class GrpcReplicationService extends
private final boolean zeroCopyEnabled;
- private final ZeroCopyMessageMarshaller<SendContainerRequest>
- sendContainerZeroCopyMessageMarshaller = new
ZeroCopyMessageMarshaller<>(
- SendContainerRequest.getDefaultInstance());
+ private final ZeroCopyMessageMarshaller<SendContainerRequest>
sendContainerZeroCopyMessageMarshaller;
- private final ZeroCopyMessageMarshaller<CopyContainerRequestProto>
- copyContainerZeroCopyMessageMarshaller = new
ZeroCopyMessageMarshaller<>(
- CopyContainerRequestProto.getDefaultInstance());
+ private final ZeroCopyMessageMarshaller<CopyContainerRequestProto>
copyContainerZeroCopyMessageMarshaller;
public GrpcReplicationService(ContainerReplicationSource source,
ContainerImporter importer, boolean zeroCopyEnabled) {
this.source = source;
this.importer = importer;
this.zeroCopyEnabled = zeroCopyEnabled;
+
+ if (zeroCopyEnabled) {
+ this.sendContainerZeroCopyMessageMarshaller = new
ZeroCopyMessageMarshaller<>(
+ SendContainerRequest.getDefaultInstance());
+ this.copyContainerZeroCopyMessageMarshaller = new
ZeroCopyMessageMarshaller<>(
+ CopyContainerRequestProto.getDefaultInstance());
+ } else {
+ this.sendContainerZeroCopyMessageMarshaller = null;
+ this.copyContainerZeroCopyMessageMarshaller = null;
+ }
}
public ServerServiceDefinition bindServiceWithZeroCopy() {
```
```java
@@ -148,10 +154,11 @@ public void download(CopyContainerRequestProto request,
// output may have already been closed, ignore such errors
IOUtils.cleanupWithLogger(LOG, outputStream);
- InputStream popStream =
- copyContainerZeroCopyMessageMarshaller.popStream(request);
- if (popStream != null) {
- IOUtils.cleanupWithLogger(LOG, popStream);
+ if (copyContainerZeroCopyMessageMarshaller != null) {
+ InputStream popStream =
copyContainerZeroCopyMessageMarshaller.popStream(request);
+ if (popStream != null) {
+ IOUtils.cleanupWithLogger(LOG, popStream);
+ }
}
}
}
```
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
