Skip to content
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

[Feature][Zeta] Add tag to node used to filter worker when submit job #7045

Merged
merged 12 commits into from
Jun 26, 2024

Conversation

liunaijie
Copy link
Contributor

Purpose of this pull request

close #7036

Does this PR introduce any user-facing change?

yes, use can add tag in seatunnel config use to choose node by tag

How was this patch tested?

Check list

@Hisoka-X Hisoka-X added the feature New feature label Jun 24, 2024
@Hisoka-X Hisoka-X changed the title [Feature] add tag to node, job config. when submit use tag to filter node [Feature][Zeta] Add tag to node used to filter worker when submit job Jun 24, 2024
Copy link
Member

@Hisoka-X Hisoka-X left a comment

Choose a reason for hiding this comment

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

Thanks @liunaijie ! This feature is very important for SeaTunnel.

Comment on lines 42 to 48
member-attributes:
group:
type: string
value: platform
team:
type: string
value: team1
Copy link
Member

Choose a reason for hiding this comment

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

Hi, please do not change this. I believe this feature should not open by default.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

reverted

Comment on lines 147 to 149
if (workerAttr == null || workerAttr.isEmpty()) {
return true;
}
Copy link
Member

Choose a reason for hiding this comment

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

Why workerAttr is empty it will be matched?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@@ -150,7 +151,8 @@ public PhysicalPlanGenerator(
}

public Tuple2<PhysicalPlan, Map<Integer, CheckpointPlan>> generate() {

Map<String, Object> envOptions = jobImmutableInformation.getJobConfig().getEnvOptions();
Map<String, String> tags = JsonUtils.toMap(envOptions.getOrDefault("tag", "").toString());
Copy link
Member

Choose a reason for hiding this comment

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

Please add tag into

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -0,0 +1,81 @@
---

sidebar_position: 9
Copy link
Member

Choose a reason for hiding this comment

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

Add this page into

"items": [

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@Hisoka-X Hisoka-X added the Zeta label Jun 24, 2024
Copy link
Contributor Author

Choose a reason for hiding this comment

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

move image folder from docs/en/images to docs/images

@liunaijie
Copy link
Contributor Author

The tag filter can also apply to task level, but for now not implement this.

Copy link
Member

@Hisoka-X Hisoka-X left a comment

Choose a reason for hiding this comment

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

Please add a test case with end to end use tag filter feature too.

docs/sidebars.js Outdated
@@ -178,7 +178,8 @@ const sidebars = {
"seatunnel-engine/checkpoint-storage",
"seatunnel-engine/rest-api",
"seatunnel-engine/tcp",
"seatunnel-engine/engine-jar-storage-mode"
"seatunnel-engine/engine-jar-storage-mode",
"seatunnel-engine/resource-manager",
Copy link
Member

Choose a reason for hiding this comment

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

how about named this part as Resource Isolation?

@@ -96,6 +96,10 @@ public void toMap(Map<String, String> result) {
}
}

public Map<String, Object> getMapConfig() {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
public Map<String, Object> getMapConfig() {
public Map<String, Object> getSourceMap() {

Comment on lines 24 to 27
tag_filter {
group = "platform"
team = "team1"
}
Copy link
Member

Choose a reason for hiding this comment

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

Please revert.

Copy link
Member

@Hisoka-X Hisoka-X left a comment

Choose a reason for hiding this comment

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

LGTM except some minior problem.

Options.key("tag_filter")
.mapType()
.noDefaultValue()
.withDescription("custom parameters for run engine");
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
.withDescription("custom parameters for run engine");
.withDescription("Define the worker where the job runs by tag");

Map.Entry::getKey, Map.Entry::getValue));
}
if (matchedWorker.isEmpty()) {
log.error("No matched worker.");
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
log.error("No matched worker.");
log.error("No matched worker with tag filter {}.", tagFilter);

Comment on lines +53 to +56
tag_filter {
group = "platform"
team = "team1"
}
Copy link
Member

Choose a reason for hiding this comment

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

How to adapt to flink/spark?

env{} is visible to all engines

Copy link
Member

Choose a reason for hiding this comment

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

At now, it is fine to only take effect under the zeta engine. We should not mention it on other engine's doc.

Copy link
Member

Choose a reason for hiding this comment

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

ok

@hailin0 hailin0 merged commit 3af6b2f into apache:dev Jun 26, 2024
10 checks passed
hawk9821 pushed a commit to hawk9821/seatunnel that referenced this pull request Jul 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature][Resource Manager][Zeta] add tag to worker, when submit job can use tag to choose worker
3 participants