Skip to content

Commit

Permalink
fix: null app-info-path cause NPE (apache#14752)
Browse files Browse the repository at this point in the history
  • Loading branch information
Radeity authored and biaoma-ty committed Aug 21, 2023
1 parent 8a0c7c6 commit bfdc68e
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@
@NoArgsConstructor
public class GetAppIdRequest implements RequestMessageBuilder {

private String logPath;
private int taskInstanceId;

private String appInfoPath;
private String logPath;

@Override
public MessageType getCommandType() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@

import org.apache.dolphinscheduler.common.utils.JSONUtils;
import org.apache.dolphinscheduler.common.utils.PropertyUtils;
import org.apache.dolphinscheduler.plugin.task.api.TaskExecutionContext;
import org.apache.dolphinscheduler.plugin.task.api.TaskExecutionContextCacheManager;
import org.apache.dolphinscheduler.plugin.task.api.utils.LogUtils;
import org.apache.dolphinscheduler.remote.command.Message;
import org.apache.dolphinscheduler.remote.command.MessageType;
Expand All @@ -44,7 +46,9 @@ public class GetAppIdProcessor extends BaseLogProcessor implements NettyRequestP
public void process(Channel channel, Message message) {
GetAppIdRequest getAppIdRequest =
JSONUtils.parseObject(message.getBody(), GetAppIdRequest.class);
String appInfoPath = getAppIdRequest.getAppInfoPath();
TaskExecutionContext taskExecutionContext =
TaskExecutionContextCacheManager.getByTaskInstanceId(getAppIdRequest.getTaskInstanceId());
String appInfoPath = taskExecutionContext.getAppInfoPath();
String logPath = getAppIdRequest.getLogPath();
List<String> appIds = LogUtils.getAppIds(logPath, appInfoPath,
PropertyUtils.getString(APPID_COLLECT, DEFAULT_COLLECT_WAY));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -210,8 +210,9 @@ public void removeTaskLog(@NonNull Host host, String path) {
}
}

public @Nullable List<String> getAppIds(@NonNull String host, int port, @NonNull String taskLogFilePath,
@NonNull String taskAppInfoPath) throws RemotingException, InterruptedException {
public @Nullable List<String> getAppIds(@NonNull String host, int port, String taskLogFilePath,
String taskAppInfoPath,
int taskInstanceId) throws RemotingException, InterruptedException {
log.info("Begin to get appIds from worker: {}:{} taskLogPath: {}, taskAppInfoPath: {}", host, port,
taskLogFilePath, taskAppInfoPath);
final Host workerAddress = new Host(host, port);
Expand All @@ -220,7 +221,7 @@ public void removeTaskLog(@NonNull Host host, String path) {
appIds = LogUtils.getAppIds(taskLogFilePath, taskAppInfoPath,
PropertyUtils.getString(APPID_COLLECT, DEFAULT_COLLECT_WAY));
} else {
final Message message = new GetAppIdRequest(taskLogFilePath, taskAppInfoPath).convert2Command();
final Message message = new GetAppIdRequest(taskInstanceId, taskLogFilePath).convert2Command();
Message response = this.client.sendSync(workerAddress, message, LOG_REQUEST_TIMEOUT);
if (response != null) {
GetAppIdResponse responseCommand =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ public static String getPidsStr(int processId) throws Exception {
Thread.sleep(Constants.SLEEP_TIME_MILLIS);
Host host = Host.of(taskExecutionContext.getHost());
List<String> appIds = logClient.getAppIds(host.getIp(), host.getPort(), taskExecutionContext.getLogPath(),
taskExecutionContext.getAppInfoPath());
taskExecutionContext.getAppInfoPath(), taskExecutionContext.getTaskInstanceId());
if (CollectionUtils.isNotEmpty(appIds)) {
taskExecutionContext.setAppIds(String.join(TaskConstants.COMMA, appIds));
if (StringUtils.isEmpty(taskExecutionContext.getExecutePath())) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public class LogUtils {
* @param fetchWay fetch way
* @return application id list.
*/
public List<String> getAppIds(@NonNull String logPath, @NonNull String appInfoPath, String fetchWay) {
public List<String> getAppIds(String logPath, String appInfoPath, String fetchWay) {
if (!StringUtils.isEmpty(fetchWay) && fetchWay.equals("aop")) {
log.info("Start finding appId in {}, fetch way: {} ", appInfoPath, fetchWay);
return getAppIdsFromAppInfoFile(appInfoPath);
Expand Down Expand Up @@ -142,7 +142,11 @@ public static Path getTaskInstanceLogBasePath() {
.orElse(null);
}

public List<String> getAppIdsFromAppInfoFile(@NonNull String appInfoPath) {
public List<String> getAppIdsFromAppInfoFile(String appInfoPath) {
if (StringUtils.isEmpty(appInfoPath)) {
log.warn("appInfoPath is empty");
return Collections.emptyList();
}
File appInfoFile = new File(appInfoPath);
if (!appInfoFile.exists() || !appInfoFile.isFile()) {
return Collections.emptyList();
Expand Down

0 comments on commit bfdc68e

Please sign in to comment.