Synapse fails to restart after zookeeper node disappears #94
Comments
|
I've done some further debugging, and so far I can say that: -Haproxy config is getting successfully re-applied when there are changes in the number of endpoints (begin begins & end ends below) Here's what happens if I restart Synapse (for reference): I'm curious if anyone is experiencing a similar issue, or if it is a corner case caused by Docker/Supervisord combination |
|
More debugging:
Restart Synapse Kill one of the nodes:
Restart Synapse Kill one of the nodes: Somehow, when the update happens, another thread is taking over, and @config_updated gets overwritten by each of them, causing the config update to fail (so I assume). Most likely, it's the ZK gem that's causing the issue (by creating threads and locks). I will try various Ruby versions, as well as to roll back ZK/Synapse, to see if that helps, as I am out of ideas. P.S. I am using a manually compiled Ruby 2.1.2 |
|
Update: I have taken Synapse out of Docker and Supervisord, as well as have tried multiple versions of Ruby and Synapse and still have the same issue. |
|
FINALLY! A solution: TL;DR Apparently, having one instance of Synapse would prevent it from being able to both query for changes and execute loops - something that I still can't wrap my head around. Running from Supevisord, as a command, or via plain synapse -c /etc/synapse.conf.yaml in the foreground was doing just that - making the process busy and not able to write modified Haproxy configs. To make it work, I had to adjust my Supervisord config: [program:synapse] thus forking Synapse. It took me a while to debug, so hopefully this will help someone else too. |
|
Today I have noticed another abnormality - although the changes made (as per my previous comment) did help with dead node removal, new nodes never made it to Haproxy config when registered. However, they started adding with numprocs=4, which is even stranger. Despite that working, I am still convinced that repeating every command multiple times is not the right way to go. What I have also noticed is that with numprocs=4 there are only 3 processes executing, which means that something is getting lost. Can anyone shed some light on how forking/threading is being done in Synapse? |
|
is this fixed? anyone? |
|
@hankya We haven't seen the behavior referenced in this PR but we don't use the supervisord. Have you been seeing the same issue and if so what's your setup? I did see something that I thought was like this bug when ZK was overloaded, but in that case the issue was just a large lag in event delivery. Our connection pooling fix alleviated those issues (plus a bug fix to the pooling code). While investigating the pooling bug though I setup a pretty thorough test setup where I was removing and adding nodes while messing with the network via netem and I still wasnt able to repro with Ruby 1.9 and running Synapse directly. I could try to modify that setup to get a repro of this issue if it's still impacting people. |
|
@jolynch This issue is related to the fact that (AFAIK) Synapse does reloads by forking the process and then executing the reload. The problem with Supervisord is that it keeps attached to STDOUT, running the process in the foreground, thus messing with the ability to fork&reload. That's a pity, because Supervisord is a really great tool for process monitoring. |
|
i don't think that makes sense. we run synapse via runit, which also keeps the process attached in the foreground, and we have no problems. also, i'm not sure what you're referring to as far as reloads via forking. our usual model is to exit the process and allow the process supervisor to restart it. |
|
I'm also confused by the forking issue. We use upstart and just have it kill the old process before starting the new one. For nerve we run two copies during a flip to achieve hitless nerve reloads. @Jaykah why do you need to fork Synapse to reload? I think that @hankya is having a separate issue, which I'm following up on in #137. |
|
@jolynch The first six comments of this ticket explain my reasoning. Basically what happens is if we have only one instance of Synapse running it does not write modified Haproxy configs for node removal, if we run two it does remove nodes, but does not add new ones. And if we run is as numprocs=4 (basically forking it into 4 processes) it seems to work fine, but I've still had some cases with reloads not happening (maybe once every 100 reloads or so as opposed to 100% of the time without forking). Unfortunately, this is something that I wasn't yet able to figure out... |
|
@Jaykah I think the relevant context here is that you are using ruby 2.1.2, same as @hankya over in #137. I will focus my efforts on that ruby version. It's also been reported that 2.2 doesn't work (although ruby-zk doesn't work with 2.2 so ...). For context, I am unable to reproduce this at all in our production environment or in my own testing using ruby 1.9, so if you can downgrade, that might be a good short term solution. |
|
@igor47 I thought you had mentioned airbnb moving to ruby 2.X soon, if you guys get around to debugging this before me that is also cool heh :-) |
|
fyi @rickypai |
|
I sent a PR to the zk gem to have its tests run and pass against Ruby 2.2. Unfortunately, we are yet to be running our infrastructure on Ruby 2.x, so we have not seen this error. |
|
FWIW, I've run the following test on Ubuntu 14.04 which works on ruby1.9.1 and does not work on ruby2.0 running a supervisord based nerve and synapse.
The difference is that on step 4, ruby1.9.1 synapse re-registers the backends. |
|
@jolynch No, I ran the test with |
|
@igor47 can you cut a bug fix release? I haven't been able to roll it out here but if you've done that then we probably want to cut a release. |
|
i'll do that now; i've only given it the barest testing on our infra, haven't rolled it out site-wide yet |
|
@Jaykah how does master work for you? Do you still see the lack of restart on update? |
|
@jolynch Have just tested it, and it still does not do reloads properly. Built from master. and then after restarted through supervisorctl: |
|
@Jaykah, OK I'll look into it. Can you post your docker file, it might make it easier for me to reproduce. Just to make sure we're talking about the same thing, Synapse is not supposed to reload itself if Synapse configuration changes (adding or removing services) however it is supposed to reload HAProxy if the servers backing those services change. I guess the part I'm confused about in this ticket is why Synapse needs to fork at all to reload HAProxy. Actually, how does this work at all in a Docker container? Does HAProxy run inside the container? |
|
@jolynch Sure, will post the Dockerfile shortly. That is correct, I am referring to the HAProxy reload when the list of nodes changes in Zookeeper. So the steps would be:
Both HAProxy and Synapse run in Docker with Supervisord. |
|
it needs to fork to reload haproxy because the haproxy reload is accomplished with a command. the unix way to execute another command is to |
|
I think that having the docker file (or something close to it) will be a big help in reproducing the bug. I have a suspicion supervisord and docker are not acting like we might expect. |
|
Yea there are going to be two forks (one by Synapse and one by the HAProxy that starts up), so if supervisord is like "what is this thing that just forked" that might be an issue. |
|
@jolynch I've created a Dockerfile bundle for you that can be used to reproduce the issue. All you have to do is add a "synapse.conf.yaml" and your public key (to ssh into container via ip:222), named "myserver.pub". You will then be able to connect to Supervisord console via "supervisorctl" with user "supervisord" and password "testpass". Port 3212 is also exposed for HAProxy stats. https://www.dropbox.com/sh/yjs6eds1t0xdpg7/AAA7BUQ-MkP8nfOyjPvAZ549a?dl=0 |
|
@jolynch Were you able to reproduce it? |
|
@Jaykah sorry I'll try to take a look this weekend, sorry about that. |
|
@jolynch Let me know if I can help in solving that issue in any way, would be glad to provide a staging environment or any other information that might be helpful. |
|
@Jaykah I'm still working on getting a repro working, but one of the first things I noticed is that you have the supervisord process managing haproxy which is probably not going to work with Synapse causing it to fork. This sounds similar to the problems that folks have with systemsd which assumes that the process it's managing does not fork. See the patch for more background. So I guess two questions:
|
Haproxy is launched in foreground as 1 process, but then exits with a status 0 (expected) when Synapse loads it. That way we are monitoring the initial launch of Haproxy without restraining forking (for the purpose you've mentioned). We are then stopping it's management by Supervisord with:
We have also tested dropping SYN and reloading via supervisorctl with limited success) 2.-Will try the haproxy-systemd-wrapper shortly. |
|
@jolynch Unfortunately nothing changes by using the wrapper. Synapse is not restarting Haproxy: If I restart Synapse manually I get: Moreover, with supervisord.conf and synapse.conf.yaml I can still see Haproxy's forks even after the process is terminated via Supervisor (we start with haproxy-systemd-wrapper & two children, two more are started when I issue a HUP, and then one of the first two and one of the second two remain after). Were you able to reproduce the issue with the Dockerfile I provided? |
|
Did some more tests with/without -Ds, with/without -db, with numprocs=1/2/3/4, without fork control, restarts/reloads via -USR2, -HUP - nothing worked. At this time I am almost certain that the issue has to do with Supervisor reaping Synapse processes because Synapse does not have a "foreground" (or non-daemon) mode, making it not systemd compliant. |
|
@jolynch Were you able to reproduce it? We're debating if we should move our infra off Synapse because of this issue, but if you have any pointers as to how we can make it run in foreground, that would be super helpful. Thanks. |
|
@Jaykah Yea I've been dropping the ball on this. I really think that the issue lies with how a Docker container really isn't a proper init system. We really like running synapse/haproxy outside the containers because of all kinds of reasons (namely our service data plane stays available during a docker roll and we don't like paying conntrack), is there any possibility you can run on the bare metal? To be honest I thought that Synapse does stay attached in the foreground (as @igor47 has implied they run it via runit). Why do you think those reaps are Synapse PIDs? Couldn't they just as easily be HAProxy instances? I'll keep working on repro but ... well I've been saying that for a while. |
|
@Jaykah Alright I got a few hours to debug this today and this is not a bug in Synapse, it's an issue with how supervisord and haproxy interact. There are a couple of problems with running Synapse like this but they all revolve around the problem that Supervisord assumes that it is the only thing creating processes ever and this is simply not true with Synapse (which typically forks haproxy) or HAProxy (which typically forks itself). Because we're running in supervisord the only way that Synapse can reload the service is via supervisorctl, e.g. However, if we do that and use the haproxy-systemd-wrapper, then we run into the problem that supervisord only supports stop then start restarts (I think), which means that it waits for the previous haproxy to completely finish before it will start the new one. This is bad because the way that HAProxy gracefully drains traffic is to have the old process drop its listeners but crucially not exit until all TCP connections are finished cleanly. This leads to the unknown pid problem as well, because there are a bunch of haproxy instances hanging around that will never go away due to the way the haproxy systemd wrapper works. I think this would all be solved if we could just supply the pid in the I'm going to keep poking at this to see if I can properly work around supervisord and haproxy's terribleness, but at this point I'm pretty convinced that this is not a bug with Synapse but rather an issue with supervisord being a really bad replacement for a proper init system. |
|
For what it's worth I was able to get a container that would restart haproxy, but it worked by basically having a bash script that would exec: #!/bin/bash
touch /var/haproxy/haproxy.pid
exec /usr/sbin/haproxy -db -f /etc/haproxy/haproxy.cfg -p /var/haproxy/haproxy.pid -sf $(cat /var/haproxy/haproxy.pid)And then a supervisord configuration that would just use this as the command and I'm pulling in HAProxy 1.6 to see if they've made any improvements to the haproxy-systemd-wrapper, but given my reading of the source code the haproxy-systemd-wrapper I'm not hopeful. @Jaykah I really think that this is going to work better running on bare metal... but it would be nice to support a docker deployment so I'll keep poking. |
|
Alright @Jaykah, I've got a working Docker setup with latest synapse, haproxy 1.6 and ruby 2.2 over at https://gist.github.com/jolynch/54db6eb64a8951450d6e. The TLDR is: use the systemd wrapper, have synapse send signals to the systemd wrapper (not to haproxy itself) and have supervisord always autostart the wrapper and try to start it before synapse (there is a bit of a chicken and an egg problem because synapse has to generate the config before supervisord can start it properly). But yea, I really wouldn't encourage someone doing this in production because:
|
I am using the Zookeeper watcher to get a list of available nodes - which it successfully does.
However, after a node fails, it does not get removed from haproxy.
Haproxy monitoring page shows those nodes as down, Zookeeper no longer has them as they are ephemeral, but the reconfiguration does not take place.
If I restart Synapse, however, everything goes back to normal.
Note: Synapse is deployed in Docker and managed by supervisord
The text was updated successfully, but these errors were encountered: