Skip to content

YARN-11398 DelegationTokenRenewer timeout feature may cause high utilization of CPU and memory leak #5233

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

Closed
wants to merge 1 commit into from
Closed

YARN-11398 DelegationTokenRenewer timeout feature may cause high utilization of CPU and memory leak #5233

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
14 changes: 14 additions & 0 deletions ...n/java/org/apache/hadoop/yarn/server/resourcemanager/security/DelegationTokenRenewer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1017,12 +1017,26 @@ public void run() {
"Exhausted max retry attempts {} in token renewer "
+ "thread for {}",
tokenRenewerThreadRetryMaxAttempts, evt.getApplicationId());
continue;
}
}
} catch (Exception e) {
LOG.info("Problem in submitting renew tasks in token renewer "
+ "thread.", e);
continue;
}

// For normally finished DT renew event, it should be removed from futures.
futures.remove(evt);
}

// If the cluster is idle for some time, futures map is empty or no event handler found which may still cause high CPU utilization
// Therefore a short nap should be added here.
try {
Thread.sleep(1000);
Copy link
Contributor

@slfan1989 slfan1989 Dec 17, 2022

Choose a reason for hiding this comment

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

Set sleep 1000ms, what is the effect of this?

Copy link
Contributor Author

@Daniel-009497 Daniel-009497 Dec 19, 2022

Choose a reason for hiding this comment

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

Set sleep 1000ms, what is the effect of this?

Give up CPU

Copy link
Contributor

@slfan1989 slfan1989 Dec 19, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Can we give data? The CPU ratio before and after the modification. Why did we choose 1000ms? there seems to be no data support for this.

Copy link
Contributor Author

@Daniel-009497 Daniel-009497 Dec 22, 2022

Choose a reason for hiding this comment

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

@slfan1989
Thanks for review.
The CPU utilization statistics is as follows:
Before optimize:
image

After optimize:
image

Copy link
Contributor

@slfan1989 slfan1989 Dec 22, 2022

Choose a reason for hiding this comment

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

This does not seem to be a good way to deal with it through thread sleep.

Copy link
Member

@ayushtkn ayushtkn Dec 31, 2022

Choose a reason for hiding this comment

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

Not convinced with the sleep either as @slfan1989 mentioned and it looks very arbitrary to me, worked for you but might cause issues for others

} catch (InterruptedException ex) {
// No big deal, just ignore this exception here and try the next round
LOG.warn("DelegationTokenRenewerePoolTracker idle sleep is interrupted.");
}
}
}
Expand Down