airbnb / synapse

A transparent service discovery framework for connecting an SOA
MIT License
2.07k stars 251 forks source link

Block on watcher start to alleviate HAProxy config generator race condition #333

Closed panchr closed 4 years ago

panchr commented 4 years ago

Summary

Due to a race condition in the HAProxy config generator, each watcher start method should block until there is an initial discover performed. In particular, the watcher needs to have the config_for_generator set before it should return from startup.

The race condition occurs due to how the config generator reads the data at different times for different purposes, even though treats the data as being the same.

This reverts the thread use change (introduced with dual-read) specifically for the ZookeeperWatcher.

Note: the race condition in HAProxy config generator still exists. However, since the ZK watchers are the only ones that produce config_for_generators, it should not occur as long as config_for_generator is never the default value.

Testing

Initially, cannot talk to a Synapse-backed dependency through HAProxy when Synapse first starts:

$ sudo service synapse restart
ok: run: synapse: 1s
$ curl -i http://my-dependency:8080/health
curl: (7) Failed to connect to my-dependency port 8080: Connection refused

# but reloading HAProxy fixes the issue
$ sudo service haproxy reload
 * Reloading haproxy haproxy                                                                  [ OK ]
$ curl -i http://my-dependency:8080/health
HTTP/1.1 404 Not Found
date: Fri, 28 Aug 2020 15:25:07 GMT
...

After

With this patch applied, the curl works without needing to manually restart HAProxy:

$ sudo service synapse restart
ok: run: synapse: 0s
$ curl -i http://my-dependency:8080/health
HTTP/1.1 404 Not Found
date: Fri, 28 Aug 2020 15:29:21 GMT

Reviewers

@Jason-Jian @austin-zhu