Skip to content

stop zk watches cleanly#129

Closed
talbright wants to merge 1 commit intodocker:masterfrom
talbright:clear-zk-watch
Closed

stop zk watches cleanly#129
talbright wants to merge 1 commit intodocker:masterfrom
talbright:clear-zk-watch

Conversation

@talbright
Copy link
Copy Markdown

@talbright talbright commented May 25, 2016

- changes zk to use talbright fork
- uses CancelWatch method in zk fork
@talbright
Copy link
Copy Markdown
Author

Seems odd etcd tests would be failing, is master stable? Will have to look into it.

@talbright talbright changed the title Stop zk watches cleanly stop zk watches cleanly May 25, 2016
@talbright talbright closed this May 25, 2016
@talbright talbright deleted the clear-zk-watch branch May 25, 2016 12:07
@abronan
Copy link
Copy Markdown
Contributor

abronan commented May 25, 2016

Hi @talbright, it's definitely not related to your PR.

Looking at the trace, this seems like a delete event (lock release) was triggered right when the check for Get happens with etcd but pair then is nil. From the travis history it looks like the tests have been running consistently slower and slower for the past few months, so seems like a odd timing issue. Will look into improving the tests to make it more reliable (like having a clear path and not triggering the nil pointer for the cleanup to happen properly between tests).

@talbright
Copy link
Copy Markdown
Author

Thanks for the confirmation! I closed this PR after reading the contributor guidelines that suggested it was favorable to separate library upgrades/changes from taking advantage of any new features they might bring in. See #130

Some food for thought:

Timing issues can really plague projects like this and go-zookeeper, its the nature of the beast to some extent. I've cleaned up a lot of the timing related bugs in my fork of go-zookeeper, but there's still some outstanding that can happen (I went from 9 of 10 builds failing to closer to 9 of 10 builds passing.) In our internal projects we use (ginkgo)[https://github.com/onsi/ginkgo], and have found the asynchronous matchers in gomega very helpful.

I know there is a general stance against using heavy testing frameworks in the contribution guidelines, not to mention the amount of work it would take to switch tests over. However, there might be something useful to glean from the implementation of http://onsi.github.io/gomega/#eventually and some of the other asynchronous matchers.

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