9uapaw commented on code in PR #4468:
URL: https://github.com/apache/hadoop/pull/4468#discussion_r902203432
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/logaggregation/AggregatedLogDeletionService.java:
##########
@@ -295,14 +314,15 @@ protected Configuration createConf() {
// as @Idempotent, it will automatically take care of RM restart/failover.
@VisibleForTesting
protected ApplicationClientProtocol createRMClient() throws IOException {
- return ClientRMProxy.createRMProxy(getConfig(),
- ApplicationClientProtocol.class);
+ return ClientRMProxy.createRMProxy(getConfig(),
ApplicationClientProtocol.class);
}
@VisibleForTesting
protected void stopRMClient() {
- if (task != null && task.getRMClient() != null) {
- RPC.stopProxy(task.getRMClient());
+ for (LogDeletionTask task : tasks) {
+ if (task != null && task.getRMClient() != null) {
+ RPC.stopProxy(task.getRMClient());
+ }
Review Comment:
There is only one common RMClient is created, however, you stop this
RMClient three times, which could be problematic, or at best redundant. I
presume the client itself is thread safe, thus safe to share between tasks, but
it should only be stopped once.
--
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]
