You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2020/11/27 04:03:35 UTC
[GitHub] [apisix] willmafh opened a new pull request #2869: fix: start server failure
willmafh opened a new pull request #2869:
URL: https://github.com/apache/apisix/pull/2869
### What this PR does / why we need it:
When you shutdown the machine directly, the logs/nginx.pid won't be deleted.
After rebooting and run `make run`, it reports “APISIX is running...” but actually it's not.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on pull request #2869: fix: start server failure
Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #2869:
URL: https://github.com/apache/apisix/pull/2869#issuecomment-738780919
We mainly use the shell scripts for this case
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] willmafh commented on a change in pull request #2869: fix: start server failure
Posted by GitBox <gi...@apache.org>.
willmafh commented on a change in pull request #2869:
URL: https://github.com/apache/apisix/pull/2869#discussion_r531433205
##########
File path: Makefile
##########
@@ -86,6 +86,8 @@ run: default
ifeq ("$(wildcard logs/nginx.pid)", "")
mkdir -p logs
$(OR_EXEC) -p $$PWD/ -c $$PWD/conf/nginx.conf
+else ifeq ("$(shell lsof -p $(shell cat logs/nginx.pid))", "")
+ $(OR_EXEC) -p $$PWD/ -c $$PWD/conf/nginx.conf
Review comment:
Actually we can't...The only thing we know for sure is nginx.pid exists but there's no corresponding process...Also when a user execute `make run`, **the implication** here is that he believes the apisix server is not running, but if it does running, telling him that the server is running, just like the way we do now...But if it doesn't, then we should just start the new server process since the user doesn't care about it...
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander closed pull request #2869: fix: start server failure
Posted by GitBox <gi...@apache.org>.
spacewander closed pull request #2869:
URL: https://github.com/apache/apisix/pull/2869
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] willmafh commented on a change in pull request #2869: fix: start server failure
Posted by GitBox <gi...@apache.org>.
willmafh commented on a change in pull request #2869:
URL: https://github.com/apache/apisix/pull/2869#discussion_r531433205
##########
File path: Makefile
##########
@@ -86,6 +86,8 @@ run: default
ifeq ("$(wildcard logs/nginx.pid)", "")
mkdir -p logs
$(OR_EXEC) -p $$PWD/ -c $$PWD/conf/nginx.conf
+else ifeq ("$(shell lsof -p $(shell cat logs/nginx.pid))", "")
+ $(OR_EXEC) -p $$PWD/ -c $$PWD/conf/nginx.conf
Review comment:
Actually we can't...The only thing we know for sure is nginx.pid exists but there's no corresponding process...Also when a user executes `make run`, **the implication** here is that he believes the apisix server is not running, but if it does running, telling him that the server is running, just like the way we do now...But if it doesn't, then we should just start the new server process since the user doesn't care about it...
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander closed pull request #2869: fix: start server failure
Posted by GitBox <gi...@apache.org>.
spacewander closed pull request #2869:
URL: https://github.com/apache/apisix/pull/2869
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on pull request #2869: fix: start server failure
Posted by GitBox <gi...@apache.org>.
spacewander commented on pull request #2869:
URL: https://github.com/apache/apisix/pull/2869#issuecomment-908815079
The issue is already solved by others.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] willmafh edited a comment on pull request #2869: fix: start server failure
Posted by GitBox <gi...@apache.org>.
willmafh edited a comment on pull request #2869:
URL: https://github.com/apache/apisix/pull/2869#issuecomment-738548049
> we need some test cases about this change
I don't know how to add test cases in this condition. But I do test all situations on my local machine.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on pull request #2869: fix: start server failure
Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #2869:
URL: https://github.com/apache/apisix/pull/2869#issuecomment-738059850
we need some test cases about this change
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] willmafh commented on a change in pull request #2869: fix: start server failure
Posted by GitBox <gi...@apache.org>.
willmafh commented on a change in pull request #2869:
URL: https://github.com/apache/apisix/pull/2869#discussion_r534625705
##########
File path: Makefile
##########
@@ -86,6 +86,8 @@ run: default
ifeq ("$(wildcard logs/nginx.pid)", "")
mkdir -p logs
$(OR_EXEC) -p $$PWD/ -c $$PWD/conf/nginx.conf
+else ifeq ("$(shell lsof -p $(shell cat logs/nginx.pid))", "")
+ $(OR_EXEC) -p $$PWD/ -c $$PWD/conf/nginx.conf
Review comment:
@membphis @moonming
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #2869: fix: start server failure
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #2869:
URL: https://github.com/apache/apisix/pull/2869#discussion_r531417853
##########
File path: Makefile
##########
@@ -86,6 +86,8 @@ run: default
ifeq ("$(wildcard logs/nginx.pid)", "")
mkdir -p logs
$(OR_EXEC) -p $$PWD/ -c $$PWD/conf/nginx.conf
+else ifeq ("$(shell lsof -p $(shell cat logs/nginx.pid))", "")
+ $(OR_EXEC) -p $$PWD/ -c $$PWD/conf/nginx.conf
Review comment:
Would be better if we can output the reason to start APISIX even the `nginx.pid` exists.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Firstsawyou commented on a change in pull request #2869: fix: start server failure
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on a change in pull request #2869:
URL: https://github.com/apache/apisix/pull/2869#discussion_r531419318
##########
File path: Makefile
##########
@@ -86,6 +86,8 @@ run: default
ifeq ("$(wildcard logs/nginx.pid)", "")
mkdir -p logs
$(OR_EXEC) -p $$PWD/ -c $$PWD/conf/nginx.conf
+else ifeq ("$(shell lsof -p $(shell cat logs/nginx.pid))", "")
+ $(OR_EXEC) -p $$PWD/ -c $$PWD/conf/nginx.conf
Review comment:
agree +1.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] willmafh commented on a change in pull request #2869: fix: start server failure
Posted by GitBox <gi...@apache.org>.
willmafh commented on a change in pull request #2869:
URL: https://github.com/apache/apisix/pull/2869#discussion_r532158031
##########
File path: Makefile
##########
@@ -86,6 +86,8 @@ run: default
ifeq ("$(wildcard logs/nginx.pid)", "")
mkdir -p logs
$(OR_EXEC) -p $$PWD/ -c $$PWD/conf/nginx.conf
+else ifeq ("$(shell lsof -p $(shell cat logs/nginx.pid))", "")
+ $(OR_EXEC) -p $$PWD/ -c $$PWD/conf/nginx.conf
Review comment:
@spacewander I still don't think it's necessary to output some message here...We can see [bin/apisix](https://github.com/apache/apisix/blob/ce1d40b42f056f46b20386e1937fc81ba69e84c6/bin/apisix#L348) start function, their logic are actually the same...If we really want to output some message here, then the start function of bin/apisix should also be changed, but it's really unnecessary.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #2869: fix: start server failure
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #2869:
URL: https://github.com/apache/apisix/pull/2869#discussion_r531441675
##########
File path: Makefile
##########
@@ -86,6 +86,8 @@ run: default
ifeq ("$(wildcard logs/nginx.pid)", "")
mkdir -p logs
$(OR_EXEC) -p $$PWD/ -c $$PWD/conf/nginx.conf
+else ifeq ("$(shell lsof -p $(shell cat logs/nginx.pid))", "")
+ $(OR_EXEC) -p $$PWD/ -c $$PWD/conf/nginx.conf
Review comment:
@willmafh
Yes, our suggestion is to output some message that `nginx.pid exists but there's no corresponding process`, and then start the new one.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on pull request #2869: fix: start server failure
Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #2869:
URL: https://github.com/apache/apisix/pull/2869#issuecomment-907554341
@willmafh need to resolve Conflicting files
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on pull request #2869: fix: start server failure
Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #2869:
URL: https://github.com/apache/apisix/pull/2869#issuecomment-738781185
You can take a look at the existing test cases
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] willmafh commented on pull request #2869: fix: start server failure
Posted by GitBox <gi...@apache.org>.
willmafh commented on pull request #2869:
URL: https://github.com/apache/apisix/pull/2869#issuecomment-738548049
> we need some test cases about this change
I don't know how to add test cases in this condition. But I do test all situation on my local machine.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on pull request #2869: fix: start server failure
Posted by GitBox <gi...@apache.org>.
spacewander commented on pull request #2869:
URL: https://github.com/apache/apisix/pull/2869#issuecomment-908815079
The issue is already solved by others.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on a change in pull request #2869: fix: start server failure
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #2869:
URL: https://github.com/apache/apisix/pull/2869#discussion_r534888151
##########
File path: Makefile
##########
@@ -86,6 +86,8 @@ run: default
ifeq ("$(wildcard logs/nginx.pid)", "")
mkdir -p logs
$(OR_EXEC) -p $$PWD/ -c $$PWD/conf/nginx.conf
+else ifeq ("$(shell lsof -p $(shell cat logs/nginx.pid))", "")
+ $(OR_EXEC) -p $$PWD/ -c $$PWD/conf/nginx.conf
Review comment:
`nginx.pid exists but there's no corresponding process`
We should add this tip in both `Makefile` and `bin/apisix`, it is more user-friendly.
I created a new issue https://github.com/apache/apisix/issues/2948 , welcome to fix this issue.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] github-actions[bot] commented on pull request #2869: fix: start server failure
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #2869:
URL: https://github.com/apache/apisix/pull/2869#issuecomment-907089691
This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 4 weeks if no further activity occurs. If you think that's incorrect or this pull request should instead be reviewed, please simply write any comment. Even if closed, you can still revive the PR at any time or discuss it on the dev@apisix.apache.org list. Thank you for your contributions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org