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

[Improve][Seatunnel-Cluster] Run the server through daemon #4161

Merged
merged 11 commits into from
Mar 20, 2023

Conversation

liugddx
Copy link
Member

@liugddx liugddx commented Feb 18, 2023

Purpose of this pull request

Check list

@liugddx
Copy link
Member Author

liugddx commented Feb 18, 2023

image
image

lightzhao
lightzhao previously approved these changes Feb 22, 2023
@TyrantLucifer
Copy link
Member

Please fix CI error

Copy link
Member

@TyrantLucifer TyrantLucifer left a comment

Choose a reason for hiding this comment

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

seatunnel-cluster.sh dose not only has the one function to start cluster node, it also has the function of showing help message like the following shown:

image

So if using this solution user will not see the help message realtime.

@TyrantLucifer
Copy link
Member

seatunnel-cluster.sh dose not only has the one function to start cluster node, it also has the function of showing help message like the following shown:

image

So if using this solution user will not see the help message realtime.

cc @Hisoka-X @EricJoy2048

@Hisoka-X
Copy link
Member

yes, you should put -d to start cluster with daemon mode.

seatunnel-cluster.sh dose not only has the one function to start cluster node, it also has the function of showing help message like the following shown:
image
So if using this solution user will not see the help message realtime.

cc @Hisoka-X @EricJoy2048

@liugddx
Copy link
Member Author

liugddx commented Mar 5, 2023

yes, you should put -d to start cluster with daemon mode.

seatunnel-cluster.sh dose not only has the one function to start cluster node, it also has the function of showing help message like the following shown:
image
So if using this solution user will not see the help message realtime.

cc @Hisoka-X @EricJoy2048

Agree with you

@liugddx
Copy link
Member Author

liugddx commented Mar 5, 2023

image

@TyrantLucifer
Copy link
Member

image

If user input seatunnel.sh -h -d or seatunnel.sh -xxx -d, the output log of option -xxx will be rewrited in files too.

@Hisoka-X
Copy link
Member

Hisoka-X commented Mar 5, 2023

image

image

I use your changed in local, report this error when SeaTunnel don't have seatunnel-server.out.

@liugddx
Copy link
Member Author

liugddx commented Mar 12, 2023

image

If user input seatunnel-cluster.sh -h -d or seatunnel-cluster.sh -xxx -d, the output log of option -xxx will be rewrited in files too.

We can not use -d if it is an interactive argument. So seatunnel-cluster.sh -h -d is not allowed, but the internal parameters of the program can be parsed, like seatunnel-cluster.sh -cn test_seatunnel -d, Have any good Suggestions?

image image

I use your changed in local, report this error when SeaTunnel don't have seatunnel-server.out.

I'm in deployment.md illustrates this .

mkdir -p $SEATUNNEL_HOME/logs
touch $SEATUNNEL_HOME/logs/seatunnel-server.out
./bin/seatunnel-cluster.sh -d

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

…unnel/core/starter/seatunnel/args/ServerCommandArgs.java

Co-authored-by: Hisoka <[email protected]>
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

@EricJoy2048 EricJoy2048 merged commit 40699b6 into apache:dev Mar 20, 2023
@liugddx liugddx deleted the improve-18 branch March 20, 2023 10:43
ic4y pushed a commit to ic4y/incubator-seatunnel that referenced this pull request May 22, 2023
* Add Support SeaTunnel Zeta Cluster Node Run Through A Daemon
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