Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[synapse] fix bug in generate_backend_stanze #242

Merged
merged 1 commit into from
Sep 15, 2017
Merged

[synapse] fix bug in generate_backend_stanze #242

merged 1 commit into from
Sep 15, 2017

Conversation

lap1817
Copy link
Contributor

@lap1817 lap1817 commented Sep 14, 2017

to: @jolynch

what

A bug was introduced into the generate_Backend_stanza. @state_cache.backends is expecting watcher.name as the input argument, but it got the watcher object.
With the bug, when Synapse generates the haproxy config, it only sees the "enabled" services as reported by the watchers, but does not see all known backends in the state cache (because search by 'watch', instead of watcher.name, in the state cache failed silently). This increases number of haproxy restarts when Synapse is restarted.

fix

Pass the correct argument to @state_cache.backends

test

Added rspec test to cover this bug.

Copy link
Collaborator

@jolynch jolynch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lg2m

.travis.yml Outdated
@@ -2,7 +2,6 @@ language: ruby
cache: bundler
sudo: false
rvm:
- 1.9.3-p551
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean to remove this? I think some companies still run this version (I know ...)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just replied below in the comments. sounds good to you?

@lap1817
Copy link
Contributor Author

lap1817 commented Sep 15, 2017

@jolynch , I am also removing Travis build on rvm 1.9.3-p551. Travis CI is having odd failures. I made a testing PR which only changed the README file, but still got the same CI build failure on rvm 1.9.30p551. #243

The error is likely due to the old version of bundler (1.7.6) being used (other builds use 1.15.4). I found a discussion thread related to this: rubygems/bundler#3558. Seems that the issue is fixed in bundler version after 1.7.15.

I tried to enforce upgrading bundle in travis CI before bundle install. But it didn't work either.

So I am removing the build on rvm 1.9.3.

PTAL

@jolynch
Copy link
Collaborator

jolynch commented Sep 15, 2017

@lap1817 I just merged a workaround (upgrade bundler before_install), can you pull master and keep 1.9?

@lap1817
Copy link
Contributor Author

lap1817 commented Sep 15, 2017

sure. doing it. odd that it didn't work for me

@jolynch jolynch merged commit 26c5ab1 into airbnb:master Sep 15, 2017
@jolynch
Copy link
Collaborator

jolynch commented Sep 15, 2017

Thanks @lap1817, I'll cut a bug fix release in a sec.

@lap1817
Copy link
Contributor Author

lap1817 commented Sep 15, 2017

thank you @jolynch

@jolynch
Copy link
Collaborator

jolynch commented Sep 15, 2017

0.15.1 cut with the fix, cheers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants