You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by GitBox <gi...@apache.org> on 2020/09/01 23:53:54 UTC
[GitHub] [trafficserver] randall opened a new pull request #7157: rc: fixes systemd unit file stopping
randall opened a new pull request #7157:
URL: https://github.com/apache/trafficserver/pull/7157
In the ExecStopPost stanza of the unit file, an errant "}" made the lock
file never be deleted. Also the "-f" flag to rm always makes the
returncode 0 whether or not the file exists making the return code check
always return success.
----------------------------------------------------------------
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] [trafficserver] bneradt commented on a change in pull request #7157: rc: fixes systemd unit file stopping
Posted by GitBox <gi...@apache.org>.
bneradt commented on a change in pull request #7157:
URL: https://github.com/apache/trafficserver/pull/7157#discussion_r482319148
##########
File path: rc/trafficserver.service.in
##########
@@ -27,8 +27,8 @@ Restart=on-failure
RestartSec=5s
LimitNOFILE=1000000
ExecStopPost=/bin/sh -c ' \
- export TM_PIDFILE=$(@exp_bindir@/traffic_layout} 2>/dev/null | grep RUNTIMEDIR | cut -d: -f2)/manager.lock ; \
- /bin/rm -f $TM_PIDFILE ; \
Review comment:
Just checking: are we sure we want to remove the -f? -f will return a non-zero return code in the case of a permissions error. If all we care about is that the pid file does not exist after this, regardless of whether it existed before or not, then -f is correct.
Or do we want a non-zero return code if the pid file did not exist? In that case, removing the -f is correct.
----------------------------------------------------------------
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] [trafficserver] randall removed a comment on pull request #7157: rc: fixes systemd unit file stopping
Posted by GitBox <gi...@apache.org>.
randall removed a comment on pull request #7157:
URL: https://github.com/apache/trafficserver/pull/7157#issuecomment-685197790
[approve ci docs]
----------------------------------------------------------------
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] [trafficserver] randall commented on a change in pull request #7157: rc: fixes systemd unit file stopping
Posted by GitBox <gi...@apache.org>.
randall commented on a change in pull request #7157:
URL: https://github.com/apache/trafficserver/pull/7157#discussion_r482332479
##########
File path: rc/trafficserver.service.in
##########
@@ -27,8 +27,8 @@ Restart=on-failure
RestartSec=5s
LimitNOFILE=1000000
ExecStopPost=/bin/sh -c ' \
- export TM_PIDFILE=$(@exp_bindir@/traffic_layout} 2>/dev/null | grep RUNTIMEDIR | cut -d: -f2)/manager.lock ; \
- /bin/rm -f $TM_PIDFILE ; \
Review comment:
We want the non-zero status code. I think we assume that ATS started up correctly in the first place.
----------------------------------------------------------------
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] [trafficserver] randall commented on pull request #7157: rc: fixes systemd unit file stopping
Posted by GitBox <gi...@apache.org>.
randall commented on pull request #7157:
URL: https://github.com/apache/trafficserver/pull/7157#issuecomment-685197790
[approve ci docs]
----------------------------------------------------------------
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] [trafficserver] randall commented on pull request #7157: rc: fixes systemd unit file stopping
Posted by GitBox <gi...@apache.org>.
randall commented on pull request #7157:
URL: https://github.com/apache/trafficserver/pull/7157#issuecomment-685834428
[approve ci docs]
----------------------------------------------------------------
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] [trafficserver] zwoop commented on pull request #7157: rc: fixes systemd unit file stopping
Posted by GitBox <gi...@apache.org>.
zwoop commented on pull request #7157:
URL: https://github.com/apache/trafficserver/pull/7157#issuecomment-686564566
Cherry-picked to v9.0.x branch.
----------------------------------------------------------------
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] [trafficserver] randall merged pull request #7157: rc: fixes systemd unit file stopping
Posted by GitBox <gi...@apache.org>.
randall merged pull request #7157:
URL: https://github.com/apache/trafficserver/pull/7157
----------------------------------------------------------------
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] [trafficserver] randall commented on pull request #7157: rc: fixes systemd unit file stopping
Posted by GitBox <gi...@apache.org>.
randall commented on pull request #7157:
URL: https://github.com/apache/trafficserver/pull/7157#issuecomment-685906061
[approve ci]
----------------------------------------------------------------
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] [trafficserver] randall removed a comment on pull request #7157: rc: fixes systemd unit file stopping
Posted by GitBox <gi...@apache.org>.
randall removed a comment on pull request #7157:
URL: https://github.com/apache/trafficserver/pull/7157#issuecomment-685906061
----------------------------------------------------------------
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] [trafficserver] randall commented on pull request #7157: rc: fixes systemd unit file stopping
Posted by GitBox <gi...@apache.org>.
randall commented on pull request #7157:
URL: https://github.com/apache/trafficserver/pull/7157#issuecomment-685936314
@ywkaras Can I get a review again? (No change to code, just to commit msg)
----------------------------------------------------------------
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] [trafficserver] bneradt commented on a change in pull request #7157: rc: fixes systemd unit file stopping
Posted by GitBox <gi...@apache.org>.
bneradt commented on a change in pull request #7157:
URL: https://github.com/apache/trafficserver/pull/7157#discussion_r482340062
##########
File path: rc/trafficserver.service.in
##########
@@ -27,8 +27,8 @@ Restart=on-failure
RestartSec=5s
LimitNOFILE=1000000
ExecStopPost=/bin/sh -c ' \
- export TM_PIDFILE=$(@exp_bindir@/traffic_layout} 2>/dev/null | grep RUNTIMEDIR | cut -d: -f2)/manager.lock ; \
- /bin/rm -f $TM_PIDFILE ; \
Review comment:
OK, that makes sense.
----------------------------------------------------------------
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