Skip to content

YARN-11274. Impove Nodemanager#NodeStatusUpdaterImpl Log. #4783

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ayushtkn merged 4 commits into from
Sep 10, 2022
Merged

YARN-11274. Impove Nodemanager#NodeStatusUpdaterImpl Log. #4783

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e7ecf87
YARN-11274. Impove Nodemanager#NodeStatusUpdaterImpl Log.
Aug 22, 2022
2723c22
Merge branch 'apache:trunk' into YARN-11274
slfan1989 Aug 30, 2022
58db839
YARN-11274. Fix CheckStyle.
Sep 9, 2022
f085326
YARN-11274. Fix CheckStyle.
Sep 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 21 additions & 30 deletions ...anager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/NodeStatusUpdaterImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ protected void serviceInit(Configuration conf) throws Exception {
// Update configured resources via plugins.
updateConfiguredResourcesViaPlugins(totalResource);

LOG.info("Nodemanager resources is set to: " + totalResource);
LOG.info("Nodemanager resources is set to: {}.", totalResource);

metrics.addResource(totalResource);

Expand Down Expand Up @@ -247,9 +247,8 @@ protected void serviceInit(Configuration conf) throws Exception {
LOG.debug("{} :{}", YARN_NODEMANAGER_DURATION_TO_TRACK_STOPPED_CONTAINERS,
durationToTrackStoppedContainers);
super.serviceInit(conf);
LOG.info("Initialized nodemanager with :" +
" physical-memory=" + memoryMb + " virtual-memory=" + virtualMemoryMb +
" virtual-cores=" + virtualCores);
LOG.info("Initialized nodemanager with : physical-memory={} virtual-memory={} " +
"virtual-cores={}.", memoryMb, virtualMemoryMb, virtualCores);

this.logAggregationEnabled =
conf.getBoolean(YarnConfiguration.LOG_AGGREGATION_ENABLED,
Expand All @@ -264,7 +263,7 @@ protected void serviceStart() throws Exception {

// NodeManager is the last service to start, so NodeId is available.
this.nodeId = this.context.getNodeId();
LOG.info("Node ID assigned is : " + this.nodeId);
LOG.info("Node ID assigned is : {}.", this.nodeId);
this.httpPort = this.context.getHttpPort();
this.nodeManagerVersionId = YarnVersionInfo.getVersion();
try {
Expand Down Expand Up @@ -312,10 +311,9 @@ private void unRegisterNM() {
request.setNodeId(this.nodeId);
try {
resourceTracker.unRegisterNodeManager(request);
LOG.info("Successfully Unregistered the Node " + this.nodeId
+ " with ResourceManager.");
LOG.info("Successfully Unregistered the Node {} with ResourceManager.", this.nodeId);
} catch (Exception e) {
LOG.warn("Unregistration of the Node " + this.nodeId + " failed.", e);
LOG.warn("Unregistration of the Node {} failed.", this.nodeId, e);
}
}

Expand Down Expand Up @@ -399,7 +397,7 @@ nodeManagerVersionId, containerReports, getRunningApplications(),
nodeLabels, physicalResource, nodeAttributes, nodeStatus);

if (containerReports != null && !containerReports.isEmpty()) {
LOG.info("Registering with RM using containers :" + containerReports);
LOG.info("Registering with RM using containers.size : {}." + containerReports.size());
Copy link
Contributor Author

@slfan1989 slfan1989 Aug 22, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is enough to print the number directly.

This function will call getNMContainerStatuses()
Line#394: getNMContainerStatuses()
The list of Containers has been printed, no need to reprint.

Line#402: I modified it to print size directly.

}
if (logAggregationEnabled) {
// pull log aggregation status for application running in this NM
Expand Down Expand Up @@ -641,6 +639,7 @@ private List<ApplicationId> getRunningApplications() {
runningApplications.add(appEntry.getKey());
}
}
LOG.info("Running Applications Size : {}.", runningApplications.size());
Copy link
Contributor Author

@slfan1989 slfan1989 Aug 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add log, print the number of running apps.

return runningApplications;
}

Expand All @@ -667,8 +666,8 @@ private List<NMContainerStatus> getNMContainerStatuses() throws IOException {
}
}
if (!containerStatuses.isEmpty()) {
LOG.info("Sending out " + containerStatuses.size()
+ " NM container statuses: " + containerStatuses);
LOG.info("Sending out {} container NM container statuses: {}.",
containerStatuses.size(), containerStatuses);
}
return containerStatuses;
}
Expand Down Expand Up @@ -724,8 +723,7 @@ public void removeOrTrackCompletedContainersFromContext(
}

if (!removedContainers.isEmpty()) {
LOG.info("Removed completed containers from NM context: "
+ removedContainers);
LOG.info("Removed completed containers from NM context: {}.", removedContainers);
}
}

Expand Down Expand Up @@ -792,7 +790,7 @@ public void removeVeryOldStoppedContainersFromCache() {
try {
context.getNMStateStore().removeContainer(cid);
} catch (IOException e) {
LOG.error("Unable to remove container " + cid + " in store", e);
LOG.error("Unable to remove container {} in store.", cid, e);
}
}
}
Expand Down Expand Up @@ -839,18 +837,15 @@ private boolean handleShutdownOrResyncCommand(
if (response.getNodeAction() == NodeAction.SHUTDOWN) {
LOG.warn("Received SHUTDOWN signal from Resourcemanager as part of"
+ " heartbeat, hence shutting down.");
LOG.warn("Message from ResourceManager: "
+ response.getDiagnosticsMessage());
LOG.warn("Message from ResourceManager: {}.", response.getDiagnosticsMessage());
context.setDecommissioned(true);
dispatcher.getEventHandler().handle(
new NodeManagerEvent(NodeManagerEventType.SHUTDOWN));
return true;
}
if (response.getNodeAction() == NodeAction.RESYNC) {
LOG.warn("Node is out of sync with ResourceManager,"
+ " hence resyncing.");
LOG.warn("Message from ResourceManager: "
+ response.getDiagnosticsMessage());
LOG.warn("Node is out of sync with ResourceManager, hence resyncing.");
Copy link
Contributor

@Samrat002 Samrat002 Aug 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both can be combined into 1 LOG.warn

Copy link
Contributor Author

@slfan1989 slfan1989 Aug 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your suggestion, I will modify the code.

LOG.warn("Message from ResourceManager: {}.", response.getDiagnosticsMessage());
// Invalidate the RMIdentifier while resync
NodeStatusUpdaterImpl.this.rmIdentifier =
ResourceManagerConstants.RM_INVALID_IDENTIFIER;
Expand Down Expand Up @@ -1095,8 +1090,7 @@ protected void validate(Set<NodeAttribute> nodeAttributes)
try {
NodeLabelUtil.validateNodeAttributes(nodeAttributes);
} catch (IOException e) {
LOG.error(
"Invalid node attribute(s) from Provider : " + e.getMessage());
LOG.error("Invalid node attribute(s) from Provider : {}.", e.getMessage());
throw e;
}
}
Expand Down Expand Up @@ -1136,9 +1130,8 @@ public void verifyRMHeartbeatResponseForNodeAttributes(
} else {
// case where updated node attributes from NodeAttributesProvider
// is sent to RM and RM rejected the attributes
LOG.error("NM node attributes {" + getPreviousValue()
+ "} were not accepted by RM and message from RM : " + response
.getDiagnosticsMessage());
LOG.error("NM node attributes [{}] were not accepted by RM and message from RM : {}.",
getPreviousValue(), response.getDiagnosticsMessage());
}
}
}
Expand Down Expand Up @@ -1262,7 +1255,7 @@ protected void validate(Set<NodeLabel> nodeLabels)
}
}
if (hasInvalidLabel) {
LOG.error("Invalid Node Label(s) from Provider : " + errorMsg);
LOG.error("Invalid Node Label(s) from Provider : {}.", errorMsg);
throw new IOException(errorMsg.toString());
}
}
Expand All @@ -1287,10 +1280,8 @@ public void verifyRMHeartbeatResponseForNodeLabels(
} else {
// case where updated labels from NodeLabelsProvider is sent to RM and
// RM rejected the labels
LOG.error(
"NM node labels {" + StringUtils.join(",", getPreviousValue())
+ "} were not accepted by RM and message from RM : "
+ response.getDiagnosticsMessage());
LOG.error("NM node labels [{}] were not accepted by RM and message from RM : {}.",
StringUtils.join(",", getPreviousValue()), response.getDiagnosticsMessage());
}
}
}
Expand Down
10 changes: 5 additions & 5 deletions ...anager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestNodeStatusUpdater.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ public RegisterNodeManagerResponse registerNodeManager(
IOException {
NodeId nodeId = request.getNodeId();
Resource resource = request.getResource();
LOG.info("Registering " + nodeId.toString());
LOG.info("Registering {}.", nodeId.toString());
// NOTE: this really should be checking against the config value
InetSocketAddress expected = NetUtils.getConnectAddress(
conf.getSocketAddr(YarnConfiguration.NM_ADDRESS, null, -1));
Expand Down Expand Up @@ -217,7 +217,7 @@ private Map<ApplicationId, List<ContainerStatus>> getAppToContainerStatusMap(
public NodeHeartbeatResponse nodeHeartbeat(NodeHeartbeatRequest request)
throws YarnException, IOException {
NodeStatus nodeStatus = request.getNodeStatus();
LOG.info("Got heartbeat number " + heartBeatID);
LOG.info("Got heartbeat number {}.", heartBeatID);
NodeManagerMetrics mockMetrics = mock(NodeManagerMetrics.class);
Dispatcher mockDispatcher = mock(Dispatcher.class);
@SuppressWarnings("unchecked")
Expand Down Expand Up @@ -625,7 +625,7 @@ public RegisterNodeManagerResponse registerNodeManager(
@Override
public NodeHeartbeatResponse nodeHeartbeat(NodeHeartbeatRequest request)
throws YarnException, IOException {
LOG.info("Got heartBeatId: [" + heartBeatID +"]");
LOG.info("Got heartBeatId: [{}]", heartBeatID);
NodeStatus nodeStatus = request.getNodeStatus();
nodeStatus.setResponseId(heartBeatID.getAndIncrement());
NodeHeartbeatResponse nhResponse = YarnServerBuilderUtils.
Expand All @@ -644,7 +644,7 @@ public NodeHeartbeatResponse nodeHeartbeat(NodeHeartbeatRequest request)
}
}
if (heartBeatID.get() == 2) {
LOG.info("Sending FINISH_APP for application: [" + appId + "]");
LOG.info("Sending FINISH_APP for application: [{}]", appId);
this.context.getApplications().put(appId, mock(Application.class));
nhResponse.addAllApplicationsToCleanup(Collections.singletonList(appId));
}
Expand Down Expand Up @@ -1528,7 +1528,7 @@ public void testApplicationKeepAlive() throws Exception {
rt.context.getApplications().remove(rt.appId);
Assert.assertEquals(1, rt.keepAliveRequests.size());
int numKeepAliveRequests = rt.keepAliveRequests.get(rt.appId).size();
LOG.info("Number of Keep Alive Requests: [" + numKeepAliveRequests + "]");
LOG.info("Number of Keep Alive Requests: [{}]", numKeepAliveRequests);
Assert.assertTrue(numKeepAliveRequests == 2 || numKeepAliveRequests == 3);
GenericTestUtils.waitFor(
() -> nm.getServiceState() != STATE.STARTED
Expand Down