jjneely / buckytools

Go implementation of useful tools for dealing with Graphite's Whisper DBs and Carbon hashing
Other
87 stars 21 forks source link

Is there a plans to merge jump branch to the master? #8

Closed azhiltsov closed 7 years ago

jjneely commented 8 years ago

Are you looking to use that with carbon-c-relay's jump-fnv1a hashing algorithm? IIRC, that's what I was trying to imitate here. But being that I've never been able to push a hashring change in my current cluster this hasn't been merged.

azhiltsov commented 8 years ago

Yes. We are indeed trying to use it with carbon-c-relay's jump-fnv1a as it gives us more equal distribution of metrics between hosts. We currently switching our setup to a replication factor 1, so the #3 is not a case for us any more.

jjneely commented 8 years ago

The fact that Graphite just does multiple destinations rather than true replication has been a big reason why I've not done more work on replication factor > 1. There's no real redundancy / data safety that adds.

I'll take a look at getting that merged in.

deniszh commented 7 years ago

Hello! Does someone have working jumphash implementation for buckytools? @azhiltsov ? @jjneely ?

azhiltsov commented 7 years ago

Hi. Yes, we currently using it. It kinda hidden, we accidentally found it here https://github.com/jjneely/buckytools/tree/jump

deniszh commented 7 years ago

Ah, so you using a build from jump tree and it works, right? Cool, thanks!

deniszh commented 7 years ago

Nah, can't build it: "./cluster.go:67: master.Algo undefined (type buckytools.JSONRingType has no field or method Algo) ./cluster.go:69: undefined: hashing.NewCarbonHashRing ./cluster.go:71: undefined: hashing.NewJumpHashRing ./cluster.go:71: master.Replicas undefined (type buckytools.JSONRingType has no field or method Replicas) ./cluster.go:73: master.Algo undefined (type buckytools.JSONRingType has no field or method Algo) ./cluster.go:74: master.Algo undefined (type buckytools.JSONRingType has no field or method Algo) ./cluster.go:118: master.Algo undefined (type buckytools.JSONRingType has no field or method Algo) ./cluster.go:118: v.Algo undefined (type buckytools.JSONRingType has no field or method Algo) make: *** [bucky] Error 2"

deniszh commented 7 years ago

Ah, of course - all imports are pointed to master version. So, the only way to build it - fork into the own repo, rename jump branch to master and change imports. Meh. Will test it and will try to properly merge it to master.

jjneely commented 7 years ago

This is in progress on my end....

jjneely commented 7 years ago

This is merged. I'm sure there are a bug or three that I've forgotten about.

howdoicomputer commented 7 years ago

@jjneely I think this breaks the 'standard' carbon hash implementation because the isHealthy function is checking to see if the hash ring contains any replicas.

jjneely commented 7 years ago

I'm sure there are some bugs here... Do have an error case to reproduce?

I plan to be spending some more time on this in the coming weeks, I just unblocked myself from another issue this evening.

howdoicomputer commented 7 years ago

@jjneely So I could be totally misunderstanding how everything works but this is what I did:

https://github.com/jjneely/buckytools/compare/master...howdoicomputer:master

deniszh commented 7 years ago

I did an exactly same thing in my branch - https://github.com/deniszh/buckytools/commit/ad89e747220848b5d0eb05e9f7be99c6dd5dd7ad - remove '+1' in ring length comparison. Still not understand how it should work with it.

jjneely commented 7 years ago

Good to know....