You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pulsar.apache.org by GitBox <gi...@apache.org> on 2020/08/27 11:43:48 UTC

[GitHub] [pulsar-helm-chart] lhotari opened a new pull request #59: Get OS signals passed to container process to enable graceful shutdown

lhotari opened a new pull request #59:
URL: https://github.com/apache/pulsar-helm-chart/pull/59


   ### Changes 
   
   - using "exec" to run a command replaces the shell process with the executed process
   - this is required so that the process running in the container is able to receive OS signals
     - explained in https://docs.docker.com/develop/develop-images/dockerfile_best-practices/
       and https://docs.docker.com/engine/reference/builder/#entrypoint
   - receiving SIGINT signal is required for graceful shutdown
   
   This change might fix issues such as https://github.com/apache/pulsar/issues/6603 . One expectation of this fix is that graceful shutdown would allow Pulsar components such as a bookies to deregistered from Zookeeper properly before shutdown. 
   
   ### Motivation
   
   Dockerfile best practices mention that "exec" should be used so that the process running in a container can receive OS signals. This is explained in https://docs.docker.com/develop/develop-images/dockerfile_best-practices/
       and https://docs.docker.com/engine/reference/builder/#entrypoint .
   Currently some issues while running Pulsar are caused by the lack of graceful shutdown.


----------------------------------------------------------------
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] [pulsar-helm-chart] sijie merged pull request #59: Pass OS signals to Pulsar container process to enable graceful shutdown

Posted by GitBox <gi...@apache.org>.
sijie merged pull request #59:
URL: https://github.com/apache/pulsar-helm-chart/pull/59


   


----------------------------------------------------------------
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] [pulsar-helm-chart] lhotari edited a comment on pull request #59: Pass OS signals to Pulsar container process to enable graceful shutdown

Posted by GitBox <gi...@apache.org>.
lhotari edited a comment on pull request #59:
URL: https://github.com/apache/pulsar-helm-chart/pull/59#issuecomment-682162804


   >  [bin/pulsar](https://github.com/apache/pulsar/blob/master/bin/pulsar#L314) script already uses `exec` to running the process. Are you sure that we need this change?
   
   @sijie  Yes, it's needed. `exec` is a shell built-in and it will only be able to replace it's current shell, not any parent shells. Therefore the exec in `bin/pulsar` won't affect the top-level shell (`command: ["sh", "-c"]`).
   
   You can quickly check the behavior without exec in docker:
   ```
   docker run --name pulsar-standalone --rm -it --entrypoint '/bin/sh' apachepulsar/pulsar:2.6.1 -c 'bin/apply-config-from-env.py conf/standalone.conf && bin/pulsar standalone'
   ```
   
   ```
   $ docker exec -it pulsar-standalone ps -ef
   UID        PID  PPID  C STIME TTY          TIME CMD
   root         1     0  0 19:56 pts/0    00:00:00 /bin/sh -c bin/apply-config-from
   root         7     1 99 19:56 pts/0    00:01:01 /usr/local/openjdk-8/bin/java -c
   root       685     0 10 19:56 pts/1    00:00:00 ps -ef
   ```
   
   As it can be seen, /bin/sh is PID 1 in the docker container when exec isn't used.
   
   
   after adding `exec` this changes:
   ```
   docker run --name pulsar-standalone --rm -it --entrypoint '/bin/sh' apachepulsar/pulsar:2.6.1 -c 'bin/apply-config-from-env.py conf/standalone.conf && exec bin/pulsar standalone'
   ```
   
   ```
   $ docker exec -it pulsar-standalone ps -ef
   UID        PID  PPID  C STIME TTY          TIME CMD
   root         1     0 99 19:58 pts/0    00:00:14 /usr/local/openjdk-8/bin/java -c
   root       253     0  0 19:58 pts/1    00:00:00 ps -ef
   ```
   
   the `java` process is now PID 1 and it will receive OS signals as recommended in Docker and k8s documentation (links provided in the issue description).
   


----------------------------------------------------------------
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] [pulsar-helm-chart] sijie commented on pull request #59: Pass OS signals to Pulsar container process to enable graceful shutdown

Posted by GitBox <gi...@apache.org>.
sijie commented on pull request #59:
URL: https://github.com/apache/pulsar-helm-chart/pull/59#issuecomment-682047299


   @lhotari [bin/pulsar](https://github.com/apache/pulsar/blob/master/bin/pulsar#L314) script already uses `exec` to running the process. Are you sure that we need 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] [pulsar-helm-chart] lhotari edited a comment on pull request #59: Pass OS signals to Pulsar container process to enable graceful shutdown

Posted by GitBox <gi...@apache.org>.
lhotari edited a comment on pull request #59:
URL: https://github.com/apache/pulsar-helm-chart/pull/59#issuecomment-682162804


   >  [bin/pulsar](https://github.com/apache/pulsar/blob/master/bin/pulsar#L314) script already uses `exec` to running the process. Are you sure that we need this change?
   
   @sijie  Yes, it's needed. `exec` is a shell built-in and it will only be able to replace it's current shell, not any parent shells. Therefore the exec in `bin/pulsar` won't affect the top-level shell (`command: ["sh", "-c"]`).
   
   You can quickly check the behavior without exec in docker:
   ```
   docker run --name pulsar-standalone --rm -it --entrypoint '/bin/sh' apachepulsar/pulsar:2.6.1 -c 'bin/apply-config-from-env.py conf/standalone.conf && bin/pulsar standalone'
   ```
   
   ```
   $ docker exec -it pulsar-standalone ps -ef
   UID        PID  PPID  C STIME TTY          TIME CMD
   root         1     0  0 19:56 pts/0    00:00:00 /bin/sh -c bin/apply-config-from
   root         7     1 99 19:56 pts/0    00:01:01 /usr/local/openjdk-8/bin/java -c
   root       685     0 10 19:56 pts/1    00:00:00 ps -ef
   ```
   
   As it can be seen, /bin/sh is PID 1 in the docker container when exec isn't used.
   
   When issuing `docker stop pulsar-standalone`, one can see that the Pulsar process doesn't receive any signals. No console output is produced. The stop command waits 10 seconds and kills the container processes.
   
   after adding `exec` this changes:
   ```
   docker run --name pulsar-standalone --rm -it --entrypoint '/bin/sh' apachepulsar/pulsar:2.6.1 -c 'bin/apply-config-from-env.py conf/standalone.conf && exec bin/pulsar standalone'
   ```
   
   ```
   $ docker exec -it pulsar-standalone ps -ef
   UID        PID  PPID  C STIME TTY          TIME CMD
   root         1     0 99 19:58 pts/0    00:00:14 /usr/local/openjdk-8/bin/java -c
   root       253     0  0 19:58 pts/1    00:00:00 ps -ef
   ```
   
   the `java` process is now PID 1 and it will receive OS signals as recommended in Docker and k8s documentation (links provided in the issue description).
   
   The graceful shutdown can now be verified by issuing `docker stop pulsar-standalone`. The shutdown process in Pulsar starts and the console is filled with log entries which confirm the graceful shutdown. For example:
   ```
   04:33:26.011 [Thread-1] INFO  org.apache.bookkeeper.bookie.Journal - Shutting down Journal
   04:33:26.011 [ForceWriteThread] INFO  org.apache.bookkeeper.bookie.Journal - ForceWrite thread interrupted
   04:33:26.011 [BookieJournal-3181] INFO  org.apache.bookkeeper.bookie.Journal - Journal exits when shutting down
   04:33:26.011 [BookieJournal-3181] INFO  org.apache.bookkeeper.bookie.Journal - Journal exited loop!
   04:33:26.012 [Thread-1] INFO  org.apache.bookkeeper.bookie.Journal - Finished Shutting down Journal thread
   04:33:26.012 [Bookie-3181] INFO  org.apache.bookkeeper.bookie.Bookie - Journal thread(s) quit.
   ```
   


----------------------------------------------------------------
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] [pulsar-helm-chart] lhotari edited a comment on pull request #59: Pass OS signals to Pulsar container process to enable graceful shutdown

Posted by GitBox <gi...@apache.org>.
lhotari edited a comment on pull request #59:
URL: https://github.com/apache/pulsar-helm-chart/pull/59#issuecomment-682162804


   >  [bin/pulsar](https://github.com/apache/pulsar/blob/master/bin/pulsar#L314) script already uses `exec` to running the process. Are you sure that we need this change?
   
   @sijie  Yes, it's needed. `exec` is a shell built-in and it will only be able to replace it's current shell not any parent shells. Therefore the exec in `bin/pulsar` won't affect the top-level shell (`command: ["sh", "-c"]`).
   
   You can quickly check the behavior without exec in docker:
   ```
   docker run --name pulsar-standalone --rm -it --entrypoint '/bin/sh' apachepulsar/pulsar:2.6.1 -c 'bin/apply-config-from-env.py conf/standalone.conf && bin/pulsar standalone'
   ```
   
   ```
   $ docker exec -it pulsar-standalone ps -ef
   UID        PID  PPID  C STIME TTY          TIME CMD
   root         1     0  0 19:56 pts/0    00:00:00 /bin/sh -c bin/apply-config-from
   root         7     1 99 19:56 pts/0    00:01:01 /usr/local/openjdk-8/bin/java -c
   root       685     0 10 19:56 pts/1    00:00:00 ps -ef
   ```
   
   As it can be seen, /bin/sh is PID 1 in the docker container when exec isn't used.
   
   
   after adding `exec` this changes:
   ```
   docker run --name pulsar-standalone --rm -it --entrypoint '/bin/sh' apachepulsar/pulsar:2.6.1 -c 'bin/apply-config-from-env.py conf/standalone.conf && exec bin/pulsar standalone'
   ```
   
   ```
   $ docker exec -it pulsar-standalone ps -ef
   UID        PID  PPID  C STIME TTY          TIME CMD
   root         1     0 99 19:58 pts/0    00:00:14 /usr/local/openjdk-8/bin/java -c
   root       253     0  0 19:58 pts/1    00:00:00 ps -ef
   ```
   
   the `java` process is now PID 1 and it will receive OS signals as recommended in Docker and k8s documentation (links provided in the issue description).
   


----------------------------------------------------------------
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] [pulsar-helm-chart] sijie commented on pull request #59: Pass OS signals to Pulsar container process to enable graceful shutdown

Posted by GitBox <gi...@apache.org>.
sijie commented on pull request #59:
URL: https://github.com/apache/pulsar-helm-chart/pull/59#issuecomment-683550914


   @lhotari Thank you for your detailed explanation! It makes sense to me.


----------------------------------------------------------------
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] [pulsar-helm-chart] lhotari commented on pull request #59: Pass OS signals to Pulsar container process to enable graceful shutdown

Posted by GitBox <gi...@apache.org>.
lhotari commented on pull request #59:
URL: https://github.com/apache/pulsar-helm-chart/pull/59#issuecomment-682162804


   > @lhotari [bin/pulsar](https://github.com/apache/pulsar/blob/master/bin/pulsar#L314) script already uses `exec` to running the process. Are you sure that we need this change?
   
   Yes, it's needed. `exec` is a shell built-in and it will only be able to replace it's current shell not any parent shells. Therefore the exec in `bin/pulsar` won't affect the top-level shell (`command: ["sh", "-c"]`).
   
   You can quickly check the behavior without exec in docker:
   ```
   docker run --name pulsar-standalone --rm -it --entrypoint '/bin/sh' apachepulsar/pulsar:2.6.1 -c 'bin/apply-config-from-env.py conf/standalone.conf && bin/pulsar standalone'
   ```
   
   ```
   $ docker exec -it pulsar-standalone ps -ef
   UID        PID  PPID  C STIME TTY          TIME CMD
   root         1     0  0 19:56 pts/0    00:00:00 /bin/sh -c bin/apply-config-from
   root         7     1 99 19:56 pts/0    00:01:01 /usr/local/openjdk-8/bin/java -c
   root       685     0 10 19:56 pts/1    00:00:00 ps -ef
   ```
   
   As it can be seen, /bin/sh is PID 1 in the docker container when exec isn't used.
   
   
   after adding `exec` this changes:
   ```
   docker run --name pulsar-standalone --rm -it --entrypoint '/bin/sh' apachepulsar/pulsar:2.6.1 -c 'bin/apply-config-from-env.py conf/standalone.conf && exec bin/pulsar standalone'
   ```
   
   ```
   $ docker exec -it pulsar-standalone ps -ef
   UID        PID  PPID  C STIME TTY          TIME CMD
   root         1     0 99 19:58 pts/0    00:00:14 /usr/local/openjdk-8/bin/java -c
   root       253     0  0 19:58 pts/1    00:00:00 ps -ef
   ```
   
   the `java` process is now PID 1 and it will receive OS signals as recommended in Docker and k8s documentation (links provided in the issue description).
   


----------------------------------------------------------------
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