[
https://issues.apache.org/jira/browse/IGNITE-7883?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16802903#comment-16802903
]
Andrey Gura commented on IGNITE-7883:
-------------------------------------
[~a-polyakov] I've took a look ti the change and have some comments:
1. {{GridCacheUtils#validateAffinityKeyConfiguration(CacheKeyConfiguration[])}}
I think better name is {{validateKeyConfigiration}}.
Please fix message of {{IgniteCheckedException}}. At the moment it is too
verbose and unclear. I think it should be something like "Cache key
configuration contains conflicting definitions: [cacheGroup=<>, cache=<>,
typeName=<>, affKeyFieldName1=<>, affKeyFieldName2=<>]".
Javadoc should have more accurate formulation. E..g it's unclear words "items"
and "full". Proper comment will be "all fields are initialized and not empty".
Please rewrite javadoc.
2. {{GridCacheUtils#validateAffinityKeyConfiguration(String, UUID,
CacheKeyConfiguration[], CacheKeyConfiguration[])}}
I think better name is {{validateKeyConfigiration}}.
{{oldCacheKeyCfgs}} and {{newCacheKeyCfgs}} parameters should be renamed to
{{rmtCacheKeyCfgs}} and {{locCacheKeyCfgs}} respectively.
Please fix message of {{IgniteCheckedException}}. At the moment it is too
verbose and unclear. For example see message that generates
{{GridCacheUtils#checkAttributeMismatch}} method. Also note that it could be
exception or just log message at warning level (it depends on
{{IGNITE_SKIP_CONFIGURATION_CONSISTENCY_CHECK}} system property). Also see p. 1
above.
Format method's code properly.
Fix javadoc in order to properly describe method parameters after renaming.
3. {{CacheKeyConfiguration#equals}}
Use {{Objects.equals}} instead of too long expressions at the method end.
4. {{CacheAffinityKeyConfigurationMismatchTest}}
Please, rewrite javadoc o the class.
Reformat code. Comma should always be on the same line where previous method
parameter placed.
Rewrite test code. Framework has set of methods for getting ignite and cache
configuration. So methods like {getIgnite}} are redundant. See
{{GridAbstractTest#getConfiguration()}} and other ignite tests for example.
Add additional tests for teh following cases:
- testKeyConfigurationLengthMismatch - you test only case when we have key
configuration on one node and don't have on another. What about different key
configurations on nodes?
- testKeyConfigurationDuplicateTypeName - you test only case for one node. What
about conflicting defintions from different nodes?
It makes sense to add test cases for configurations that were constructed due
to using {{@AffinityKeyMapped}} annotation.
> Cluster can have inconsistent affinity configuration
> -----------------------------------------------------
>
> Key: IGNITE-7883
> URL: https://issues.apache.org/jira/browse/IGNITE-7883
> Project: Ignite
> Issue Type: Bug
> Components: cache
> Affects Versions: 2.3
> Reporter: Mikhail Cherkasov
> Assignee: Alexand Polyakov
> Priority: Major
> Fix For: 2.8
>
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> A cluster can have inconsistent affinity configuration if you created two
> nodes, one with affinity key configuration and other without it(in IgniteCfg
> or CacheCfg), both nodes will work fine with no exceptions, but in the same
> time they will apply different affinity rules to keys:
>
> {code:java}
> package affinity;
> import org.apache.ignite.Ignite;
> import org.apache.ignite.Ignition;
> import org.apache.ignite.cache.CacheAtomicityMode;
> import org.apache.ignite.cache.CacheKeyConfiguration;
> import org.apache.ignite.cache.CacheMode;
> import org.apache.ignite.cache.affinity.Affinity;
> import org.apache.ignite.configuration.CacheConfiguration;
> import org.apache.ignite.configuration.IgniteConfiguration;
> import org.apache.ignite.spi.discovery.tcp.TcpDiscoverySpi;
> import org.apache.ignite.spi.discovery.tcp.ipfinder.vm.TcpDiscoveryVmIpFinder;
> import java.util.Arrays;
> public class Test {
> private static int id = 0;
> public static void main(String[] args) {
> Ignite ignite = Ignition.start(getConfiguration(true, false));
> Ignite ignite2 = Ignition.start(getConfiguration(false, false));
> Affinity<Object> affinity = ignite.affinity("TEST");
> Affinity<Object> affinity2 = ignite2.affinity("TEST");
> for (int i = 0; i < 1_000_000; i++) {
> AKey key = new AKey(i);
> if(affinity.partition(key) != affinity2.partition(key))
> System.out.println("FAILED for: " + key);
> }
> System.out.println("DONE");
> }
> private static IgniteConfiguration getConfiguration(boolean
> withAffinityCfg, boolean client) {
> IgniteConfiguration cfg = new IgniteConfiguration();
> TcpDiscoveryVmIpFinder finder = new TcpDiscoveryVmIpFinder(true);
> finder.setAddresses(Arrays.asList("localhost:47500..47600"));
> cfg.setClientMode(client);
> cfg.setIgniteInstanceName("test" + id++);
> CacheConfiguration cacheCfg = new CacheConfiguration("TEST");
> cacheCfg.setAtomicityMode(CacheAtomicityMode.TRANSACTIONAL);
> cacheCfg.setCacheMode(CacheMode.PARTITIONED);
> if(withAffinityCfg) {
> cacheCfg.setKeyConfiguration(new
> CacheKeyConfiguration("affinity.AKey", "a"));
> }
> cfg.setCacheConfiguration(cacheCfg);
> cfg.setDiscoverySpi(new TcpDiscoverySpi().setIpFinder(finder));
> return cfg;
> }
> }
> class AKey {
> int a;
> public AKey(int a) {
> this.a = a;
> }
> @Override public String toString() {
> return "AKey{" +
> "a=" + a +
> '}';
> }
> }
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
