-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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.
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
[SparkLoad]Use the yarn command to get status and kill the application #4383
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if user change the properties in resource, the config file may need to be rewrote again.
we can simply read add check it content every time.
fe/fe-core/src/main/java/org/apache/doris/load/loadv2/YarnApplicationReport.java
Outdated
Show resolved
Hide resolved
// prepare yarn config | ||
String configDir = resource.prepareYarnConfig(); | ||
// yarn client path | ||
String yarnClient = Config.yarn_client_path; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better to create a function called getYarnClienthPath()
and check if the binary file exist in that function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
@xy720 Hi, do we have any user doc for this PR? |
fe/fe-core/src/test/java/org/apache/doris/load/loadv2/SparkEtlJobHandlerTest.java
Outdated
Show resolved
Hide resolved
fe/fe-core/src/main/java/org/apache/doris/load/loadv2/SparkEtlJobHandler.java
Outdated
Show resolved
Hide resolved
String line = null; | ||
long startTime = System.currentTimeMillis(); | ||
try { | ||
Preconditions.checkState(process.isAlive()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How to make sure the process is still alive here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to make sure the process is alive. We can get output even if the process is not alive.
while (!isStop && (line = outReader.readLine()) != null) { | ||
LOG.info("Monitor Log: " + line); | ||
// parse state and appId | ||
if (line.contains(STATE)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can add an example output line here, so that the reviewer can know what the line looks like.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
fe/fe-core/src/main/java/org/apache/doris/load/loadv2/SparkLauncherMonitors.java
Outdated
Show resolved
Hide resolved
} | ||
} | ||
// parse other values | ||
else if (line.contains(QUEUE) || line.contains(START_TIME) || line.contains(FINAL_STATUS) || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if the line contains "STATE", the while loop may be broken. So how to guarantee that this else if
block can be ran?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The state's changing follows the rule of
submited > running > finished / failed
submitted > killed
submitted > running > killed
Normally, the laucher will periodically print thequeue
、start time
、final status
、tracking url
、user
logs in state submitted/runnning. So in case 2, the else if block may still not be ran when the while loop be broken.
But it's not very terrible that this else if block not be ran, beacuse the necessary value we need only containsappId
andstate
. In this case,queue
、start time
、final status
、tracking url
、user
is just missing.
I will add some relevant user doc later |
fe/fe-core/src/main/java/org/apache/doris/catalog/SparkResource.java
Outdated
Show resolved
Hide resolved
fe/fe-core/src/main/java/org/apache/doris/load/loadv2/SparkEtlJobHandler.java
Outdated
Show resolved
Hide resolved
import java.util.regex.Matcher; | ||
import java.util.regex.Pattern; | ||
|
||
public class SparkLauncherMonitors { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not like the xxxxs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about SparkLauncherMonitor?
/** | ||
* Default yarn client path | ||
*/ | ||
public static String yarn_client_path = PaloFe.DORIS_HOME_DIR + "/lib/yarn-client/hadoop/bin/yarn"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ConfField
private static final String YARN_STATUS_CMD = "%s --config %s application -status %s"; | ||
private static final String YARN_KILL_CMD = "%s --config %s application -kill %s"; | ||
|
||
class SparkAppListener implements SparkLoadAppHandle.Listener { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not used?
@@ -243,6 +264,16 @@ protected void setProperties(Map<String, String> properties) throws DdlException | |||
return sparkConfigs; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return sparkConfigs; | |
return sparkConfig; |
The method name also need changed?
} | ||
|
||
try { | ||
report.setApplicationId(ConverterUtils.toApplicationId(reportMap.get(APPLICATION_ID))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
format seems to wrong?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Please add doc for config and usage in next PR.
Proposed changes
#4346 #4203
This cl will use yarn command as follows to kill or get status of application running on YARN.
To do
1、 Make yarn command executable in spark load.
2、Write spark resource into config files and update it before running command.
3、Parse the result of executing the command line.
Types of changes
What types of changes does your code introduce to Doris?
Put an
x
in the boxes that applyChecklist
Put an
x
in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.Further comments
If this is a relatively large or complex change, kick off the discussion at dev@doris.apache.org by explaining why you chose the solution you did and what alternatives you considered, etc...