Skip to content

[Improvement-17169][service] unify preTasks and depList to predecessors in TaskNode, to avoid type conversion #17170

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

Open
unigof wants to merge 3 commits into
base: dev
Choose a base branch
from
+16 −38
Open

[Improvement-17169][service] unify preTasks and depList to predecessors in TaskNode, to avoid type conversion #17170

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f053a08
[service] use depList instead of preTasks in TaskNode
unigof May 9, 2025
561879b
unify preTasks and depList to depList in TaskNode
unigof May 13, 2025
acc0a03
update depList to predecessors
unigof May 15, 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
13 changes: 5 additions & 8 deletions ...main/java/org/apache/dolphinscheduler/api/service/impl/WorkflowDefinitionServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1970,19 +1970,16 @@ public Map<String, Object> viewTree(User loginUser, long projectCode, long code,
* @return if graph has cycle flag
*/
private boolean graphHasCycle(List<TaskNode> taskNodeResponseList) {
DAG<String, TaskNode, String> graph = new DAG<>();
DAG<Long, TaskNode, String> graph = new DAG<>();
// Fill the vertices
for (TaskNode taskNodeResponse : taskNodeResponseList) {
graph.addNode(Long.toString(taskNodeResponse.getCode()), taskNodeResponse);
graph.addNode(taskNodeResponse.getCode(), taskNodeResponse);
}
// Fill edge relations
for (TaskNode taskNodeResponse : taskNodeResponseList) {
List<String> preTasks = JSONUtils.toList(taskNodeResponse.getPreTasks(), String.class);
if (CollectionUtils.isNotEmpty(preTasks)) {
for (String preTask : preTasks) {
if (!graph.addEdge(preTask, Long.toString(taskNodeResponse.getCode()))) {
return true;
}
for (Long preTask : taskNodeResponse.getPredecessors()) {
if (!graph.addEdge(preTask, taskNodeResponse.getCode())) {
return true;
}
}
}
Expand Down
37 changes: 9 additions & 28 deletions ...inscheduler-service/src/main/java/org/apache/dolphinscheduler/service/model/TaskNode.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,13 +99,6 @@ public class TaskNode {
@JsonSerialize(using = JSONUtils.JsonDataSerializer.class)
private String params;

/**
* inner dependency information
*/
@JsonDeserialize(using = JSONUtils.JsonDataDeserializer.class)
@JsonSerialize(using = JSONUtils.JsonDataSerializer.class)
private String preTasks;

Comment on lines -102 to -108
Copy link
Member

@ruanwenjun ruanwenjun May 15, 2025

Choose a reason for hiding this comment

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

Seems this field used in front-end?

Copy link
Contributor Author

@unigof unigof May 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I think TaskNode just used in back-end when workflow running, in front-end it use taskDefinitionJson to represent task node in dag. And I searched ui module and not found TaskNode to use.

It maybe same name preTasks in front-end, because TaskNode is a inner data structure in Master module when workflow running.
If I loss some info, please tell me.

Copy link
Member

@SbloodyS SbloodyS May 19, 2025

Choose a reason for hiding this comment

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

This field is used by frontend in workflow definition and workflow instance and task definition and task instance. You should change the frontend too.

Copy link
Contributor Author

@unigof unigof May 20, 2025

Choose a reason for hiding this comment

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

This field is used by frontend in workflow definition and workflow instance and task definition and task instance. You should change the frontend too.

Maybe I miss some info and not found usage by frontend, could you give me an example? @SbloodyS thank you~

e.g. I found the field preTasks used in task-node.ts and taskRelationJson in definition/create/create/index.tsx, but maybe it is not same with preTasks in class TaskNode. It represent task relation, but it is not TaskNode.

Copy link
Contributor Author

@unigof unigof May 22, 2025

Choose a reason for hiding this comment

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

Are you mean just change field name preTasks to predecessors in front-end? @SbloodyS @ruanwenjun

/**
* users store additional information
*/
Expand All @@ -116,7 +109,7 @@ public class TaskNode {
/**
* node dependency list
*/
private List<Long> depList;
private List<Long> predecessors;

/**
* task instance priority
Expand Down Expand Up @@ -200,15 +193,6 @@ public void setParams(String params) {
this.params = params;
}

public String getPreTasks() {
return preTasks;
}

public void setPreTasks(String preTasks) {
this.preTasks = preTasks;
this.depList = JSONUtils.toList(preTasks, Long.class);
}

public String getExtras() {
return extras;
}
Expand All @@ -217,14 +201,13 @@ public void setExtras(String extras) {
this.extras = extras;
}

public List<Long> getDepList() {
return depList;
public List<Long> getPredecessors() {
return predecessors;
}

public void setDepList(List<Long> depList) {
if (depList != null) {
this.depList = depList;
this.preTasks = JSONUtils.toJsonString(depList);
public void setPredecessors(List<Long> predecessors) {
if (predecessors != null) {
this.predecessors = predecessors;
}
}

Expand Down Expand Up @@ -265,18 +248,17 @@ public boolean equals(Object o) {
&& Objects.equals(desc, taskNode.desc)
&& Objects.equals(type, taskNode.type)
&& Objects.equals(params, taskNode.params)
&& Objects.equals(preTasks, taskNode.preTasks)
&& Objects.equals(extras, taskNode.extras)
&& Objects.equals(runFlag, taskNode.runFlag)
&& Objects.equals(workerGroup, taskNode.workerGroup)
&& Objects.equals(environmentCode, taskNode.environmentCode)
&& CollectionUtils.isEqualCollection(depList, taskNode.depList)
&& CollectionUtils.isEqualCollection(predecessors, taskNode.predecessors)
&& Objects.equals(taskExecuteType, taskNode.taskExecuteType);
}

@Override
public int hashCode() {
return Objects.hash(name, desc, type, params, preTasks, extras, depList, runFlag);
return Objects.hash(name, desc, type, params, extras, predecessors, runFlag);
}

public int getMaxRetryTimes() {
Expand Down Expand Up @@ -357,9 +339,8 @@ public String toString() {
+ ", maxRetryTimes=" + maxRetryTimes
+ ", retryInterval=" + retryInterval
+ ", params='" + params + '\''
+ ", preTasks='" + preTasks + '\''
+ ", extras='" + extras + '\''
+ ", depList=" + depList
+ ", depList=" + predecessors
+ ", taskInstancePriority=" + taskInstancePriority
+ ", workerGroup='" + workerGroup + '\''
+ ", environmentCode=" + environmentCode
Expand Down
4 changes: 2 additions & 2 deletions ...service/src/main/java/org/apache/dolphinscheduler/service/process/ProcessServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -718,8 +718,8 @@ public List<TaskNode> transformTask(List<WorkflowTaskRelation> taskRelationList,
taskDefinitionLog.getTimeoutNotifyStrategy(),
taskDefinitionLog.getTimeout())));
taskNode.setDelayTime(taskDefinitionLog.getDelayTime());
taskNode.setPreTasks(JSONUtils.toJsonString(code.getValue().stream().map(taskDefinitionLogMap::get)
.map(TaskDefinition::getCode).collect(Collectors.toList())));
taskNode.setPredecessors(code.getValue().stream().map(taskDefinitionLogMap::get)
.map(TaskDefinition::getCode).collect(Collectors.toList()));
taskNode.setTaskGroupId(taskDefinitionLog.getTaskGroupId());
taskNode.setTaskGroupPriority(taskDefinitionLog.getTaskGroupPriority());
taskNode.setCpuQuota(taskDefinitionLog.getCpuQuota());
Expand Down