Skip to content

TEZ-4605: Enable hadoop CallerContext in TezChild and DAGAppMaster #394

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 6 commits into from
Feb 21, 2025
+5 −1
Merged

TEZ-4605: Enable hadoop CallerContext in TezChild and DAGAppMaster #394

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1349c36
enbale hadoop CallerContext in TezChild and DAGAppMaster
liangyu-1 Feb 20, 2025
8fe1b40
TEZ-4605:enbale hadoop CallerContext in TezChild and DAGAppMaster
liangyu-1 Feb 20, 2025
65d2ce4
use callercontext append rather than overwrite
liangyu-1 Feb 21, 2025
8b10406
use callercontext append rather than overwrite
liangyu-1 Feb 21, 2025
39a8e80
Revert "use callercontext append rather than overwrite"
liangyu-1 Feb 21, 2025
908517a
Revert "use callercontext append rather than overwrite"
liangyu-1 Feb 21, 2025
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
4 changes: 3 additions & 1 deletion tez-dag/src/main/java/org/apache/tez/dag/app/DAGAppMaster.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2390,7 +2390,9 @@ public static void main(String[] args) {
ContainerId containerId = ConverterUtils.toContainerId(containerIdStr);
ApplicationAttemptId applicationAttemptId =
containerId.getApplicationAttemptId();

org.apache.hadoop.ipc.CallerContext.setCurrent(new org.apache.hadoop.ipc.CallerContext
.Builder("tez_appmaster_" + containerId.getApplicationAttemptId()
).build());
Comment on lines +2393 to +2395
Copy link
Member

@ayushtkn ayushtkn Feb 20, 2025

Choose a reason for hiding this comment

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

This will overwrite any existing CallerContext, You need to append the values not overwrite

Copy link
Contributor Author

@liangyu-1 liangyu-1 Feb 21, 2025

Choose a reason for hiding this comment

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

Thanks for you review,@ayushtkn
I just fixed this line, is it OK now?

Copy link
Contributor Author

@liangyu-1 liangyu-1 Feb 21, 2025

Choose a reason for hiding this comment

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

This will overwrite any existing CallerContext, You need to append the values not overwrite

But in my opinion, this org.apache.hadoop.ipc.CallerContextis different from org.apache.tez.client.CallerContext, in this main function, it is the first time I set a new value to org.apache.hadoop.ipc.CallerContext, so there will be no existing CallerContext.

is it correct?

Copy link
Member

@ayushtkn ayushtkn Feb 21, 2025

Choose a reason for hiding this comment

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

if any code or application which invokes the DagAppMaster sets it then?

Copy link
Contributor Author

@liangyu-1 liangyu-1 Feb 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

org.apache.hadoop.ipc.CallerContext is a threadLocal variable, if other application invokes the DagAppMaster, it should start a new thread, and create a new callercontext variable.

Copy link
Member

@ayushtkn ayushtkn Feb 21, 2025

Choose a reason for hiding this comment

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

Ahh, then we can live without it.

is there a possibility to extend some tests?

Copy link
Contributor Author

@liangyu-1 liangyu-1 Feb 21, 2025

Choose a reason for hiding this comment

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

I tried to implement some unit tests, but the ThreadLocal variable could not be accessed. In Hadoop, this ThreadLocal variable is tested by checking the audit log, but I am unable to start a Hadoop Mini Cluster and correctly invoke DAGAppMaster, so I cannot implement this unit test.

However, the code implementation is simple, and there is a similar implementation in this Hadoop pr which is implemented by my coworker, so it should definitely work.

long appSubmitTime = Long.parseLong(appSubmitTimeStr);

String jobUserName = System
Expand Down
2 changes: 2 additions & 0 deletions tez-runtime-internals/src/main/java/org/apache/tez/runtime/task/TezChild.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.ipc.RPC;
import org.apache.hadoop.ipc.CallerContext;
import org.apache.hadoop.net.NetUtils;
import org.apache.hadoop.security.Credentials;
import org.apache.hadoop.security.SecurityUtil;
Expand Down Expand Up @@ -533,6 +534,7 @@ public static void main(String[] args) throws IOException, InterruptedException,
final int attemptNumber = Integer.parseInt(args[4]);
final String[] localDirs = TezCommonUtils.getTrimmedStrings(System.getenv(Environment.LOCAL_DIRS
.name()));
CallerContext.setCurrent(new CallerContext.Builder("tez_"+tokenIdentifier).build());
LOG.info("TezChild starting with PID=" + pid + ", containerIdentifier=" + containerIdentifier);
if (LOG.isDebugEnabled()) {
LOG.debug("Info from cmd line: AM-host: " + host + " AM-port: " + port
Expand Down