ZanderXu commented on code in PR #4527:
URL: https://github.com/apache/hadoop/pull/4527#discussion_r934277370
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java:
##########
@@ -1153,9 +1087,51 @@ public void run() {
+ connections.size());
}
+ /**
+ * A thread to write rpc requests to the socket.
+ */
+ private class RpcRequestSender implements Runnable {
+ @Override
+ public void run() {
+ while (!shouldCloseConnection.get()) {
+ ResponseBuffer buf = null;
+ try {
+ Pair<Call, ResponseBuffer> pair = rpcRequestQueue.take();
+ if (shouldCloseConnection.get()) {
+ return;
+ }
+ buf = pair.getRight();
+ synchronized (ipcStreams.out) {
+ if (LOG.isDebugEnabled()) {
+ Call call = pair.getLeft();
+ LOG.debug(getName() + " sending #" + call.id
+ + " " + call.rpcRequest);
Review Comment:
Maybe we can use `{}`, such as:
```
LOG.debug("{} sending #{} {}.", getName(), call.id, call.rpcRequest);
```
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java:
##########
@@ -1420,6 +1350,7 @@ public void stop() {
// wake up all connections
for (Connection conn : connections.values()) {
conn.interrupt();
+ conn.rpcRequestThread.interrupt();
Review Comment:
Why don't we use `conn.close()` first? Because it might be blocked by
`IOUtils.closeStream(ipcStreams)` or `socket.close()`?
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java:
##########
@@ -455,6 +385,10 @@ private class Connection extends Thread {
Consumer<Connection> removeMethod) {
this.remoteId = remoteId;
this.server = remoteId.getAddress();
+ this.rpcRequestThread = new Thread(new RpcRequestSender(),
+ "IPC Parameter Sending Thread for " + remoteId);
+ this.rpcRequestThread.setDaemon(true);
+ this.rpcRequestThread.start();
Review Comment:
Maybe we should start `rpcRequestThread` after `setupConnection()`, because
if we started it here, the socket might not be available.
--
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]
