Alexander, I was against any comparator and user defined logic exactly for reason that comparison may be inconsistent. After long discussion and consensus you implement approach with comparator. Can you please explain why you did not just add logic to compare the value of CLUSTER_REGION_ID node attribute?
--Yakov 2017-01-18 16:48 GMT+03:00 Александр Меньшиков <sharple...@gmail.com>: > I done that things: > > -- Add to TcpDiscoverySpi field Comparator<TcpDiscoveryNode> > nodeComparator for load custom comparators from config file like bean. > -- Add implementation with old behavior: BaseNodeComparator > -- Add region id implementation: RegionNodeComparator which get map from > IP address to region ID in constructor. > -- Modified TcpDiscoveryNodesRing#nextNode for using nodeComparator for > find next node. > > You can see that in PR: https://github.com/apache/ignite/pull/1436 > > Main question is: how to test it? > > For my local test i just changed BaseNodeComparator with this odd > comparator: > > new Comparator<TcpDiscoveryNode>() { > @Override > public int compare(TcpDiscoveryNode t1, TcpDiscoveryNode > t2) { > //shuffle nodes > final int ans = > Long.compare((t1.internalOrder()*3L+13L)%4L, > (t2.internalOrder()*3L+13L)%4L); > return (ans==0)?t1.compareTo(t2):ans; > } > }; > > It's looking scary, but in fact it just consistently shuffle nodes. If you > have 4 nodes with topology versions 1, 2, 3 and 4, it will be ring: 1-4-3-2. > > So I think if we just using in old test this shuffle comparator and > nothing gone wrong it's good enough. > > But any way I don't know how to add that to tests. > > And may be we need some test for custom comparators. But in fact comparators > just must be valid Java comparator and work the same on all nodes. > > Any comments are welcome. >
