Copilot commented on code in PR #2746:
URL: https://github.com/apache/tika/pull/2746#discussion_r3041397694
##########
tika-grpc/src/main/java/org/apache/tika/pipes/grpc/TikaGrpcServerImpl.java:
##########
@@ -488,5 +488,14 @@ public void shutdown() {
LOG.error("Error shutting down Ignite server", e);
}
}
+ if (pipesClient != null) {
+ LOG.info("Shutting down the pipes client");
+ try {
+ pipesClient.close();
+ }
+ catch (IOException e) {
Review Comment:
The `catch` is formatted as `}
catch (...)` which is inconsistent with the rest of this file (`} catch
(...)`) and will likely be reformatted/flagged by Spotless. Keep `catch` on the
same line as the closing brace.
```suggestion
} catch (IOException e) {
```
##########
tika-grpc/src/main/java/org/apache/tika/pipes/grpc/TikaGrpcServerImpl.java:
##########
@@ -488,5 +488,14 @@ public void shutdown() {
LOG.error("Error shutting down Ignite server", e);
}
}
+ if (pipesClient != null) {
+ LOG.info("Shutting down the pipes client");
+ try {
+ pipesClient.close();
+ }
+ catch (IOException e) {
+ LOG.error("Error closing the pipes client", e);
+ }
+ }
Review Comment:
`TikaGrpcServer.stop()` currently calls `serviceImpl.shutdown()` before
`server.shutdown().awaitTermination(...)`. With this change, closing
`pipesClient` here can interrupt in-flight RPCs during a graceful shutdown
window. Consider deferring `pipesClient.close()` until after the gRPC `Server`
has terminated (or adjusting the shutdown sequencing) to avoid spurious request
failures during shutdown.
##########
tika-grpc/src/main/java/org/apache/tika/pipes/grpc/TikaGrpcServerImpl.java:
##########
@@ -488,5 +488,14 @@ public void shutdown() {
LOG.error("Error shutting down Ignite server", e);
}
}
+ if (pipesClient != null) {
+ LOG.info("Shutting down the pipes client");
+ try {
+ pipesClient.close();
+ }
+ catch (IOException e) {
+ LOG.error("Error closing the pipes client", e);
Review Comment:
After successfully closing `pipesClient`, the field should be set to null
(ideally in a `finally`) to make shutdown idempotent and avoid repeated close
attempts or use-after-close from other code paths. This also matches the
pattern used above for `igniteStoreServer`.
```suggestion
} catch (IOException e) {
LOG.error("Error closing the pipes client", e);
} finally {
pipesClient = null;
```
--
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]
