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

[Bug] JVM parameters distinguish between client and server #4297

Merged
merged 4 commits into from
Mar 16, 2023

Conversation

MonsterChenzhuo
Copy link
Contributor

Purpose of this pull request

Check list

@MonsterChenzhuo MonsterChenzhuo changed the title [bug] JVM parameters distinguish between client and server [Bug] JVM parameters distinguish between client and server Mar 7, 2023
@MonsterChenzhuo
Copy link
Contributor Author

MonsterChenzhuo commented Mar 9, 2023

@EricJoy2048 I made a simple distinction so that there would be two configuration files under config, and starting the client or server would read the respective jvm configuration files. I'm not sure if it's appropriate.

EricJoy2048
EricJoy2048 previously approved these changes Mar 9, 2023
@TyrantLucifer
Copy link
Member

Offer the test snapshot images to verify you change has worked. You can compile the whole project and change the different jvm parameter between client and server.

@MonsterChenzhuo
Copy link
Contributor Author

MonsterChenzhuo commented Mar 9, 2023

Offer the test snapshot images to verify you change has worked. You can compile the whole project and change the different jvm parameter between client and server.

The historical e2e is enough to cover the current changes.
org.apache.seatunnel.core.starter.seatunnel.jvm.JvmOptionsParserTests

TaoZex
TaoZex previously approved these changes Mar 10, 2023
Copy link
Contributor

@TaoZex TaoZex left a comment

Choose a reason for hiding this comment

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

+1

@MonsterChenzhuo
Copy link
Contributor Author

图片

图片

cc @TyrantLucifer

@Hisoka-X
Copy link
Member

Hi, please add default jvm heap size with 128M for client.

@MonsterChenzhuo MonsterChenzhuo dismissed stale reviews from TaoZex and EricJoy2048 via bc6bfc6 March 14, 2023 05:33
@Hisoka-X Hisoka-X added this to the 2.3.1 milestone Mar 14, 2023
@MonsterChenzhuo
Copy link
Contributor Author

MonsterChenzhuo commented Mar 14, 2023

@TyrantLucifer @Hisoka-X @EricJoy2048 Thanks for your review,I have made the changes.

config/jvm_client_options Outdated Show resolved Hide resolved
@EricJoy2048 EricJoy2048 merged commit de98567 into apache:dev Mar 16, 2023
ic4y pushed a commit to ic4y/incubator-seatunnel that referenced this pull request May 22, 2023
* [bug] JVM parameters distinguish between client and server
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.

5 participants