You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2020/05/27 03:44:45 UTC
[GitHub] [ignite] timoninmaxim opened a new pull request #7852: IGNITE-13051 still validate MVCC if cache will not start
timoninmaxim opened a new pull request #7852:
URL: https://github.com/apache/ignite/pull/7852
Validate and enable MVCC if cache will not start
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] timoninmaxim commented on a change in pull request #7852: IGNITE-13051 still validate MVCC if cache will not start
Posted by GitBox <gi...@apache.org>.
timoninmaxim commented on a change in pull request #7852:
URL: https://github.com/apache/ignite/pull/7852#discussion_r433093093
##########
File path: modules/core/src/test/java/org/apache/ignite/internal/processors/cache/mvcc/CacheMvccConfigurationValidationTest.java
##########
@@ -322,6 +325,33 @@ public void testTransactionalSnapshotLimitations() throws Exception {
);
}
+ /**
+ * Check that node in client mode (filtered by AttributeNodeFilter) correctly works with MVCC.
+ */
+ @SuppressWarnings("unchecked")
+ @Test
+ public void testMvccEnabledForClientMode() throws Exception {
+ String attrName = "has_cache";
+ Object attrVal = Boolean.TRUE;
+
+ final IgniteEx crd = (IgniteEx) startGrid(getTestIgniteInstanceName(0), getConfiguration());
+
+ // Do not start cache on non-affinity node.
+ CacheConfiguration ccfg = defaultCacheConfiguration()
Review comment:
Hi, this test checks MVCC mode so atomicity mode is required. Near set is required as it must be disabled with MVCC mode. Backups dont matter actually. Fixed it
##########
File path: modules/core/src/test/java/org/apache/ignite/internal/processors/cache/mvcc/CacheMvccConfigurationValidationTest.java
##########
@@ -322,6 +325,33 @@ public void testTransactionalSnapshotLimitations() throws Exception {
);
}
+ /**
+ * Check that node in client mode (filtered by AttributeNodeFilter) correctly works with MVCC.
+ */
+ @SuppressWarnings("unchecked")
+ @Test
+ public void testMvccEnabledForClientMode() throws Exception {
+ String attrName = "has_cache";
+ Object attrVal = Boolean.TRUE;
+
+ final IgniteEx crd = (IgniteEx) startGrid(getTestIgniteInstanceName(0), getConfiguration());
+
+ // Do not start cache on non-affinity node.
+ CacheConfiguration ccfg = defaultCacheConfiguration()
+ .setNearConfiguration(null)
+ .setNodeFilter(new AttributeNodeFilter(attrName, attrVal))
+ .setBackups(0)
+ .setAtomicityMode(TRANSACTIONAL_SNAPSHOT);
+
+ final Ignite node = startGrid(getTestIgniteInstanceName(1), getConfiguration()
+ .setCacheConfiguration(ccfg)
+ .setUserAttributes(F.asMap(attrName, attrVal)));
+
+ checkTopology(2);
+
+ assertTrue(crd.context().coordinators().mvccEnabled());
Review comment:
fixed
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/CacheAffinitySharedManager.java
##########
@@ -1350,6 +1350,9 @@ else if (!crd && grp != null && grp.localStartVersion().equals(fut.initialVersio
Collection<DynamicCacheDescriptor> descs
) throws IgniteCheckedException {
IgniteInternalFuture<?> res = cachesRegistry.addUnregistered(descs);
+ for (DynamicCacheDescriptor d: descs) {
Review comment:
fixed
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] timoninmaxim commented on a change in pull request #7852: IGNITE-13051 still validate MVCC if cache will not start
Posted by GitBox <gi...@apache.org>.
timoninmaxim commented on a change in pull request #7852:
URL: https://github.com/apache/ignite/pull/7852#discussion_r433080720
##########
File path: modules/core/src/test/java/org/apache/ignite/internal/processors/cache/mvcc/CacheMvccConfigurationValidationTest.java
##########
@@ -322,6 +325,33 @@ public void testTransactionalSnapshotLimitations() throws Exception {
);
}
+ /**
+ * Check that node in client mode (filtered by AttributeNodeFilter) correctly works with MVCC.
+ */
+ @SuppressWarnings("unchecked")
+ @Test
+ public void testMvccEnabledForClientMode() throws Exception {
+ String attrName = "has_cache";
+ Object attrVal = Boolean.TRUE;
+
+ final IgniteEx crd = (IgniteEx) startGrid(getTestIgniteInstanceName(0), getConfiguration());
+
+ // Do not start cache on non-affinity node.
+ CacheConfiguration ccfg = defaultCacheConfiguration()
Review comment:
Hi, this test checks that MVCC mode is enabled, so atomicity mode is required for testing. Near must be set to null with enabled MVCC. Backups doesn't matter actually, I'll fix it
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] timoninmaxim commented on a change in pull request #7852: IGNITE-13051 still validate MVCC if cache will not start
Posted by GitBox <gi...@apache.org>.
timoninmaxim commented on a change in pull request #7852:
URL: https://github.com/apache/ignite/pull/7852#discussion_r433187534
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/CacheAffinitySharedManager.java
##########
@@ -1351,6 +1351,10 @@ else if (!crd && grp != null && grp.localStartVersion().equals(fut.initialVersio
) throws IgniteCheckedException {
IgniteInternalFuture<?> res = cachesRegistry.addUnregistered(descs);
+ for (DynamicCacheDescriptor d: descs) {
Review comment:
fixed
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] anton-vinogradov merged pull request #7852: IGNITE-13051 still validate MVCC if cache will not start
Posted by GitBox <gi...@apache.org>.
anton-vinogradov merged pull request #7852:
URL: https://github.com/apache/ignite/pull/7852
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] anton-vinogradov commented on a change in pull request #7852: IGNITE-13051 still validate MVCC if cache will not start
Posted by GitBox <gi...@apache.org>.
anton-vinogradov commented on a change in pull request #7852:
URL: https://github.com/apache/ignite/pull/7852#discussion_r433077180
##########
File path: modules/core/src/test/java/org/apache/ignite/internal/processors/cache/mvcc/CacheMvccConfigurationValidationTest.java
##########
@@ -322,6 +325,33 @@ public void testTransactionalSnapshotLimitations() throws Exception {
);
}
+ /**
+ * Check that node in client mode (filtered by AttributeNodeFilter) correctly works with MVCC.
+ */
+ @SuppressWarnings("unchecked")
+ @Test
+ public void testMvccEnabledForClientMode() throws Exception {
+ String attrName = "has_cache";
+ Object attrVal = Boolean.TRUE;
+
+ final IgniteEx crd = (IgniteEx) startGrid(getTestIgniteInstanceName(0), getConfiguration());
+
+ // Do not start cache on non-affinity node.
+ CacheConfiguration ccfg = defaultCacheConfiguration()
Review comment:
Do we really need to specify near, backups, and atomicity mode here?
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/CacheAffinitySharedManager.java
##########
@@ -1350,6 +1350,9 @@ else if (!crd && grp != null && grp.localStartVersion().equals(fut.initialVersio
Collection<DynamicCacheDescriptor> descs
) throws IgniteCheckedException {
IgniteInternalFuture<?> res = cachesRegistry.addUnregistered(descs);
+ for (DynamicCacheDescriptor d: descs) {
Review comment:
newline required before "for"
see https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines#CodingGuidelines-SemanticUnits for details
##########
File path: modules/core/src/test/java/org/apache/ignite/internal/processors/cache/mvcc/CacheMvccConfigurationValidationTest.java
##########
@@ -322,6 +325,33 @@ public void testTransactionalSnapshotLimitations() throws Exception {
);
}
+ /**
+ * Check that node in client mode (filtered by AttributeNodeFilter) correctly works with MVCC.
+ */
+ @SuppressWarnings("unchecked")
+ @Test
+ public void testMvccEnabledForClientMode() throws Exception {
+ String attrName = "has_cache";
+ Object attrVal = Boolean.TRUE;
+
+ final IgniteEx crd = (IgniteEx) startGrid(getTestIgniteInstanceName(0), getConfiguration());
+
+ // Do not start cache on non-affinity node.
+ CacheConfiguration ccfg = defaultCacheConfiguration()
+ .setNearConfiguration(null)
+ .setNodeFilter(new AttributeNodeFilter(attrName, attrVal))
+ .setBackups(0)
+ .setAtomicityMode(TRANSACTIONAL_SNAPSHOT);
+
+ final Ignite node = startGrid(getTestIgniteInstanceName(1), getConfiguration()
+ .setCacheConfiguration(ccfg)
+ .setUserAttributes(F.asMap(attrName, attrVal)));
+
+ checkTopology(2);
+
+ assertTrue(crd.context().coordinators().mvccEnabled());
Review comment:
The better case is to check every node.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] timoninmaxim commented on a change in pull request #7852: IGNITE-13051 still validate MVCC if cache will not start
Posted by GitBox <gi...@apache.org>.
timoninmaxim commented on a change in pull request #7852:
URL: https://github.com/apache/ignite/pull/7852#discussion_r433080720
##########
File path: modules/core/src/test/java/org/apache/ignite/internal/processors/cache/mvcc/CacheMvccConfigurationValidationTest.java
##########
@@ -322,6 +325,33 @@ public void testTransactionalSnapshotLimitations() throws Exception {
);
}
+ /**
+ * Check that node in client mode (filtered by AttributeNodeFilter) correctly works with MVCC.
+ */
+ @SuppressWarnings("unchecked")
+ @Test
+ public void testMvccEnabledForClientMode() throws Exception {
+ String attrName = "has_cache";
+ Object attrVal = Boolean.TRUE;
+
+ final IgniteEx crd = (IgniteEx) startGrid(getTestIgniteInstanceName(0), getConfiguration());
+
+ // Do not start cache on non-affinity node.
+ CacheConfiguration ccfg = defaultCacheConfiguration()
Review comment:
Hi, this test checks that MVCC mode is enabled, so atomicity mode is required for testing. Near must be set to null with enabled MVCC. Backups doesn't matter actually, I'll fix it
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] anton-vinogradov commented on a change in pull request #7852: IGNITE-13051 still validate MVCC if cache will not start
Posted by GitBox <gi...@apache.org>.
anton-vinogradov commented on a change in pull request #7852:
URL: https://github.com/apache/ignite/pull/7852#discussion_r433158108
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/CacheAffinitySharedManager.java
##########
@@ -1351,6 +1351,10 @@ else if (!crd && grp != null && grp.localStartVersion().equals(fut.initialVersio
) throws IgniteCheckedException {
IgniteInternalFuture<?> res = cachesRegistry.addUnregistered(descs);
+ for (DynamicCacheDescriptor d: descs) {
Review comment:
oneliner for should not use braces
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org