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

Switch to curator-go treecache #2914

Open
brian-brazil opened this Issue Jul 7, 2017 · 8 comments

Comments

Projects
None yet
5 participants
@brian-brazil
Copy link
Member

brian-brazil commented Jul 7, 2017

The treecache implementation used in our zookeeper service discovery was something I wrote as none existed in Go at the time. I attempted to get it upstreamed into a more appropriate library, but it appears that https://github.com/curator-go/curator/ is now the standard.

I think we should switch to that library rather than continuing to maintain our own version.

@brian-brazil brian-brazil changed the title Switch to go-curator treecache Switch to curator-go treecache Jul 7, 2017

@StephanErb

This comment has been minimized.

Copy link
Contributor

StephanErb commented Jul 14, 2017

This sounds like a good idea. The current implementation is collapsing regularly due to shortcomings of the used ZK go library (samuel/go-zookeeper#121, samuel/go-zookeeper#133)

@brian-brazil

This comment has been minimized.

Copy link
Member Author

brian-brazil commented Jul 14, 2017

curator-go relies on the same ZK library as what we have - there's only one of them in Go as far as I'm aware.

@StephanErb

This comment has been minimized.

Copy link
Contributor

StephanErb commented Jul 14, 2017

I have filed samuel/go-zookeeper#165 as a desperate attempt. Let's see what comes out of it.

@cstyan

This comment has been minimized.

Copy link
Contributor

cstyan commented Aug 28, 2017

@brian-brazil I'll give this a shot. Should I branch from master or 2.0?

@brian-brazil

This comment has been minimized.

Copy link
Member Author

brian-brazil commented Aug 28, 2017

Master.

@elifkus

This comment has been minimized.

Copy link
Contributor

elifkus commented Oct 8, 2017

@brian-brazil

I looked into the go-curator TreeCache implementation here. https://github.com/curator-go/curator/blob/master/recipes/cache/tree_cache.go

You’d like to remove this code https://github.com/prometheus/prometheus/blob/master/util/treecache/treecache.go and use the code above. Am I right?

The curator tree_cache implementation has a dependency on the curator client. Do you want to use curator itself as well?

(I couldn't find an implementation of tree_cache in go-curator that doesn’t have a dependency on the curator client.)

@brian-brazil

This comment has been minimized.

Copy link
Member Author

brian-brazil commented Oct 9, 2017

Am I right?

Yes.

The curator tree_cache implementation has a dependency on the curator client. Do you want to use curator itself as well?

If that's what's required.

@elifkus

This comment has been minimized.

Copy link
Contributor

elifkus commented Dec 1, 2017

curator-go looks unmaintained. This issue Fix various race conditions #4 especially this comment makes me think that using curator-go is not a good idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.