Introducing the notion of shards as an encapsulation of primary and s… #59
Conversation
Added a FIXME wrt follower allocation
|
|
||
| import in.ashwanthkumar.suuchi.cluster.MemberAddress | ||
|
|
||
| case class Shard(primaryPartition: VNode, follower: List[VNode]) { |
There was a problem hiding this comment.
Can we please add a notion of id / name to Shard?
|
|
||
| import in.ashwanthkumar.suuchi.cluster.MemberAddress | ||
|
|
||
| case class Shard(primaryPartition: VNode, follower: List[VNode]) { |
There was a problem hiding this comment.
For cases where we know they're primary and followers - it would be best if we can abstract those away as MemberAddress isn't? VNode doesn't make sense anymore to me. What do you think?
| } | ||
|
|
||
| class CHRWithShards(hashFn: Hash, partitionsPerNode: Int, replicationFactor: Int = 2) { | ||
| val sortedMap = new util.TreeMap[Integer, Shard]() |
There was a problem hiding this comment.
Can we please mark this private? This is exposed as public.
| def getNodes = primaryPartition.node :: follower.map(_.node) | ||
| } | ||
|
|
||
| class CHRWithShards(hashFn: Hash, partitionsPerNode: Int, replicationFactor: Int = 2) { |
There was a problem hiding this comment.
It would be nice if we don't have defaults as part of the class and move them as part of the companion object or place them where it's actually being used.
| this | ||
| } | ||
|
|
||
| private def hash(shard: Shard): Int = hashFn.hash(shard.key.getBytes) |
There was a problem hiding this comment.
Please move all private methods to the bottom of the class.
| import in.ashwanthkumar.suuchi.cluster.MemberAddress | ||
|
|
||
| case class Shard(primaryPartition: VNode, follower: List[VNode]) { | ||
| def key = primaryPartition.node.host + "_" + primaryPartition.node.port + "_" + primaryPartition.nodeReplicaId |
There was a problem hiding this comment.
What's a nodeReplicaId ? Did it come in as part of #58 ?
|
|
||
| def add(node: MemberAddress, nodes: List[MemberAddress]) = { | ||
| // FIXME: followers based on nodes.filterNot will lead to a skew in first elements in the list | ||
| (1 to partitionsPerNode).map(i => Shard(VNode(node, 1, Primary), followers(nodes.filterNot(_.equals(node))))).foreach { shard => |
There was a problem hiding this comment.
Selva, Can we please have a test for this? It doesn't look right to me.
|
|
||
| import in.ashwanthkumar.suuchi.cluster.MemberAddress | ||
|
|
||
| case class Shard(primaryPartition: VNode, follower: List[VNode]) { |
There was a problem hiding this comment.
Also I thought we were using partitions and not partitionsPerNode ?
|
@gsriram7 Can we also please add tests to this PR? Also can you print the CHRing with some sample data and see if you can plot it on a ring like structure by taking their ranges into account. |
Added tests for NodePriority P.S More tests need to be added to CHRWithShards and method and class names of NodePriority has to be refactored
|
Closing this in favor of #60 |
…econdary partitions