just-containers / socklog-overlay

Small syslog add-on for s6-overlay
Other
20 stars 3 forks source link

Terminating a Docker container with 1 process takes > 10 sec #8

Open bmccorm2 opened 3 years ago

bmccorm2 commented 3 years ago

I'm frustrated at the time it takes my container to shut down when using the S6 socklog overlay service. When i use the socklog service it takes ~10sec but when i don't it takes ~700ms.

** I realize in the example below nginx does not need syslog. The real service i want to use, postfix, does use syslog so that is why i am using socklog. I used the example below because that is what s6 uses for their example.

Dockerfile

FROM ubuntu
ADD https://github.com/just-containers/s6-overlay/releases/download/v2.2.0.1/s6-overlay-amd64-installer /tmp/
RUN chmod +x /tmp/s6-overlay-amd64-installer && /tmp/s6-overlay-amd64-installer /

# Add S6 Socklog
ADD https://github.com/just-containers/socklog-overlay/releases/download/v3.1.1-1/socklog-overlay-amd64.tar.gz /tmp/
RUN tar xzf /tmp/socklog-overlay-amd64.tar.gz -C /

RUN apt-get update && \
    apt-get install -y nginx && \
    echo "daemon off;" >> /etc/nginx/nginx.conf
ENTRYPOINT ["/init"]
CMD ["nginx"]

Build the image: docker build -t test .

Run the Image: docker run --name test --rm -d test

Stop the Image: time docker stop test

bryan@cave ~/s/b/shutdown (master)> time docker stop test
test

________________________________________________________
Executed in   10.31 secs   fish           external 
   usr time   15.02 millis  317.00 micros   14.70 millis 
   sys time    9.88 millis   68.00 micros    9.81 millis 
rauanmayemir commented 3 years ago

Experiencing similar issue, except sometimes terminating gets stuck for over 4 minutes. I had to shell into the pod and manually 'kill -9' the socklog process.

skarnet commented 3 years ago

I took a quick look at the code and I think I know what is happening. The read_socket() function in the socklog code is a simple read/process/write loop, checking for an exit signal at every iteration, after the reading phase. If a signal arrives during the processing or writing phases, and all the other processes are dead, the code can get stuck on recvfrom() forever because no event will make recvfrom() return.

The correct fix is to rewrite socklog as an asynchronous event loop, with "a message arrived on the socket" and "a signal arrived" (or "the selfpipe is readable" in skalibs idiom) as the two processable events. The quick and dirty fix is to add another if (flag_exitasap) break; line before the recvfrom() line.

Well I guess it's a good incentive for me to finally add a syslog reader to s6, but in the meantime, @jprjr can you please fix this?

jprjr commented 3 years ago

Done - just pushed up socklog v2.2.3 with the quick and dirty fix, and socklog-overlay v3.1.2 with the updated socklog binary.

alexyao2015 commented 2 years ago

I recently started seeing this issue crop up. I'm not entirely sure why though because I see this hasn't been updated in months, while my issue started occuring 3 days ago. My CI/CD pipeline builds a container and tests the shutdown mechanism daily which has stopped working suddenly with no changes to any of the service files. Entering into the container, I see all the processes have been reaped except for socklog. Forcibly killing it, immediately allows the container to terminate.

skarnet commented 2 years ago

The latest version of s6, which was released yesterday, includes a s6-socklog binary that can be used in place of socklog. So as soon as the new skarnet.org release is integrated, I think we'll be able to close this. @jprjr, hint, hint 😁

jprjr commented 2 years ago

Got the latest binaries available at https://github.com/just-containers/skaware/releases/tag/v2.0.7

It'll be a bit before I can get down to testing/verifying everything, I'm heading out of town tomorrow.

My plan is to keep this repo and just remove binaries from the released tarballs, to keep the install process the same. I considered merging it into s6-overlay, but that would add a script to cont-init.d, which users may not want/expect.

I also need to sit down and finally address https://github.com/just-containers/s6-overlay/issues/329 - when I get back I'll get that done, and bump the major version for both projects.