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

[#12279 ] missed check exit code of stop before calling start #12368

Merged

Conversation

gaozhangmin
Copy link
Contributor

@gaozhangmin gaozhangmin commented Oct 14, 2021

#12279 missed check exit code of stop before calling start method, avoiding stop failed and continue to call start.

also #12279 has another problem, When restarting with -force, -force flag will also be passed to start by $@.
throws Caused by: org.apache.commons.cli.UnrecognizedOptionException: Unrecognized option: -force

@gaozhangmin gaozhangmin changed the title Pulsar daemon support restart [#12279 ] missed check exit code of stop before calling start Oct 14, 2021
@BewareMyPower BewareMyPower added this to the 2.10.0 milestone Oct 15, 2021
@gaozhangmin gaozhangmin force-pushed the pulsar-daemon-support-restart branch from 836124d to 02bc98e Compare October 15, 2021 02:48
@gaozhangmin
Copy link
Contributor Author

/pulsarbot run-failure-checks

@gaozhangmin
Copy link
Contributor Author

gaozhangmin commented Oct 15, 2021

@BewareMyPower Do you know why cpp test always failed:
I tested locally, it's successfully finished

2021-10-15T07:51:05,942+0000 [AsyncHttpClient-7-1] WARN  org.apache.pulsar.client.admin.internal.BaseResource - [https://localhost:8443/admin/v2/clusters/standalone] Failed to perform http put request: java.util.concurrent.CompletionException: org.apache.pulsar.client.admin.internal.http.AsyncHttpConnector$RetryException: Could not complete the operation. Number of retries has been exhausted. Failed reason: Connection refused: localhost/127.0.0.1:8443
null 

@BewareMyPower
Copy link
Contributor

It failed when CI ran pulsar-client-cpp/pulsar-test-service-start.sh. I'm not sure about the cause. Can you run the script locally?

@eolivelli
Copy link
Contributor

please do not merge this patch today,
I want to recreate the branch-2.9 from master

this patch looks risky to me

@gaozhangmin
Copy link
Contributor Author

pulsar-client-cpp/pulsar-test-service-start.sh

@BewareMyPower it's ok to run the script locally.

@BewareMyPower
Copy link
Contributor

I tried your script in my local env (macOS Big Sur 11.3, Java 8) and failed.

pulsar-standalone-xuyunzedeMacBook-Pro.local.log

Copy link
Contributor

@BewareMyPower BewareMyPower left a comment

Choose a reason for hiding this comment

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

pulsar-client-cpp/pulsar-test-service-start.sh failed

@Anonymitaet Anonymitaet added the doc-not-needed Your PR changes do not impact docs label Oct 18, 2021
@gaozhangmin
Copy link
Contributor Author

pulsar-client-cpp/pulsar-test-service-start.sh failed

#12279 has another problem, When restarting with -force, -force flag will also be passed to start by $@.
throws Caused by: org.apache.commons.cli.UnrecognizedOptionException: Unrecognized option: -force @BewareMyPower

@gaozhangmin gaozhangmin force-pushed the pulsar-daemon-support-restart branch from 02bc98e to 6cf78d8 Compare October 18, 2021 09:48
@gaozhangmin
Copy link
Contributor Author

/pulsarbot run-failure-checks

@codelipenghui codelipenghui merged commit aa45ecd into apache:master Nov 10, 2021
eolivelli pushed a commit to eolivelli/pulsar that referenced this pull request Nov 29, 2021
…pache#12368)

apache#12279  missed check exit code of stop before calling start method, avoiding stop failed and continue to call start.

also apache#12279 has another problem, When restarting with `-force`, `-force` flag will also be passed to `start` by `$@`.
throws` Caused by: org.apache.commons.cli.UnrecognizedOptionException: Unrecognized option: -force`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/cli doc-not-needed Your PR changes do not impact docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants