Skip to content

[Improve][Zeta] Set the default slot-num value to twice the number of CPU cores - #9601

Merged
liunaijie merged 2 commits into
apache:devfrom
Hisoka-X:default-slot-num
Jul 30, 2025
Merged

liunaijie merged 2 commits into
apache:devfrom
Hisoka-X:default-slot-num

Conversation

@Hisoka-X

Copy link
Copy Markdown
Member

Purpose of this pull request

Set the default slot-num value to twice the number of CPU cores, make default values more useful.

Does this PR introduce any user-facing change?

Yes, but it is more useful than the default of 2 and does not affect existing jobs.

How was this patch tested?

add new test

Check list

@nielifeng
nielifeng requested a review from Copilot July 23, 2025 07:25

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request changes the default value for the slot-num configuration parameter from a fixed value of 2 to twice the number of CPU cores, making the default configuration more appropriate for different hardware environments.

  • Updates the default slot-num value to be calculated dynamically based on CPU cores
  • Adds comprehensive test coverage for the new default behavior
  • Updates documentation to reflect the new default value calculation

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
ServerConfigOptions.java Changes default slot-num value from 2 to CPU cores * 2
YamlSeaTunnelConfigParserTest.java Adds test to verify new default slot calculation behavior
customize-seatunnel.yaml New test configuration file for slot service testing
customize-client.yaml Fixes typo in cluster name from "custmoize" to "customize"
YamlSeaTunnelConfigLocator.java Changes class visibility from final to non-final for testing
Documentation files Updates English and Chinese docs to explain new default behavior
Comments suppressed due to low confidence (1)

seatunnel-engine/seatunnel-engine-common/src/test/java/org/apache/seatunnel/engine/common/config/YamlSeaTunnelConfigParserTest.java:123

  • The error message should be more descriptive and professional. Consider using a more specific message like "Failed to locate customize-seatunnel.yaml configuration file in working directory or classpath".
            throw new RuntimeException("can't find yaml in resources");


import java.io.IOException;

import static com.hazelcast.internal.config.DeclarativeConfigUtil.YAML_ACCEPTED_SUFFIXES;

Copilot AI Jul 23, 2025

Copy link

Choose a reason for hiding this comment

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

[nitpick] The static import is added at the top but used only in the new test method. Consider moving this import closer to where it's used or ensure it follows the project's import organization standards.

Copilot uses AI. Check for mistakes.
@liunaijie
liunaijie merged commit 5aad17e into apache:dev Jul 30, 2025
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.

4 participants