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/06/05 06:36:07 UTC
[GitHub] [ignite] timoninmaxim opened a new pull request #7902: IGNITE-13051 check mvcc configuration after register caches futu…
timoninmaxim opened a new pull request #7902:
URL: https://github.com/apache/ignite/pull/7902
Validate MVCC configuration after the future completion (wait while all caches are finally registered).
----------------------------------------------------------------
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 #7902: IGNITE-13051 check mvcc configuration after register caches futu…
Posted by GitBox <gi...@apache.org>.
timoninmaxim commented on a change in pull request #7902:
URL: https://github.com/apache/ignite/pull/7902#discussion_r435740979
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/dht/preloader/GridDhtPartitionsExchangeFuture.java
##########
@@ -346,7 +346,7 @@
private final GridDhtPartitionsStateValidator validator;
/** Register caches future. Initialized on exchange init. Must be waited on exchange end. */
- private IgniteInternalFuture<?> registerCachesFuture;
+ private IgniteInternalFuture<?> registerCachesFut;
Review comment:
this is about abbr fix: Future -> Fut to decrease amount of warnings. I rely on this object, so picked the moment to fix this. Is there a process how to provide such fixes right way?
----------------------------------------------------------------
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 #7902: IGNITE-13051 check mvcc configuration after register caches futu…
Posted by GitBox <gi...@apache.org>.
anton-vinogradov commented on a change in pull request #7902:
URL: https://github.com/apache/ignite/pull/7902#discussion_r435766515
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/dht/preloader/GridDhtPartitionsExchangeFuture.java
##########
@@ -346,7 +346,7 @@
private final GridDhtPartitionsStateValidator validator;
/** Register caches future. Initialized on exchange init. Must be waited on exchange end. */
- private IgniteInternalFuture<?> registerCachesFuture;
+ private IgniteInternalFuture<?> registerCachesFut;
Review comment:
Such fixes snatch code ownership (git annotations), so they should be performed only in case you really changing something.
----------------------------------------------------------------
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 #7902: IGNITE-13051 check mvcc configuration after register caches futu…
Posted by GitBox <gi...@apache.org>.
anton-vinogradov commented on a change in pull request #7902:
URL: https://github.com/apache/ignite/pull/7902#discussion_r435732462
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/dht/preloader/GridDhtPartitionsExchangeFuture.java
##########
@@ -346,7 +346,7 @@
private final GridDhtPartitionsStateValidator validator;
/** Register caches future. Initialized on exchange init. Must be waited on exchange end. */
- private IgniteInternalFuture<?> registerCachesFuture;
+ private IgniteInternalFuture<?> registerCachesFut;
Review comment:
Such renaming just increases changes amount without real profit
----------------------------------------------------------------
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 #7902: IGNITE-13051 check mvcc configuration after register caches futu…
Posted by GitBox <gi...@apache.org>.
anton-vinogradov merged pull request #7902:
URL: https://github.com/apache/ignite/pull/7902
----------------------------------------------------------------
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 #7902: IGNITE-13051 check mvcc configuration after register caches futu…
Posted by GitBox <gi...@apache.org>.
timoninmaxim commented on a change in pull request #7902:
URL: https://github.com/apache/ignite/pull/7902#discussion_r435828569
##########
File path: modules/core/src/test/java/org/apache/ignite/internal/processors/cache/mvcc/CacheMvccConfigurationValidationTest.java
##########
@@ -352,6 +353,37 @@ public void testMvccEnabledForClientMode() throws Exception {
assertTrue(node.context().coordinators().mvccEnabled());
}
+ /**
+ * Check that node in client mode (filtered by AttributeNodeFilter) correctly works with MVCC.
+ */
+ @SuppressWarnings("unchecked")
+ @Test
+ public void testMvccEnabledForClientModeAfterTriggeringGlobalState() throws Exception {
Review comment:
Move to another class to move some code duplication, and add more tests
----------------------------------------------------------------
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 #7902: IGNITE-13051 check mvcc configuration after register caches futu…
Posted by GitBox <gi...@apache.org>.
anton-vinogradov commented on a change in pull request #7902:
URL: https://github.com/apache/ignite/pull/7902#discussion_r435731841
##########
File path: modules/core/src/test/java/org/apache/ignite/internal/processors/cache/mvcc/CacheMvccConfigurationValidationTest.java
##########
@@ -352,6 +353,37 @@ public void testMvccEnabledForClientMode() throws Exception {
assertTrue(node.context().coordinators().mvccEnabled());
}
+ /**
+ * Check that node in client mode (filtered by AttributeNodeFilter) correctly works with MVCC.
+ */
+ @SuppressWarnings("unchecked")
+ @Test
+ public void testMvccEnabledForClientModeAfterTriggeringGlobalState() throws Exception {
Review comment:
looks like a 90% duplication of testMvccEnabledForClientMode
----------------------------------------------------------------
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 #7902: IGNITE-13051 check mvcc configuration after register caches futu…
Posted by GitBox <gi...@apache.org>.
timoninmaxim commented on a change in pull request #7902:
URL: https://github.com/apache/ignite/pull/7902#discussion_r435828202
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/dht/preloader/GridDhtPartitionsExchangeFuture.java
##########
@@ -346,7 +346,7 @@
private final GridDhtPartitionsStateValidator validator;
/** Register caches future. Initialized on exchange init. Must be waited on exchange end. */
- private IgniteInternalFuture<?> registerCachesFuture;
+ private IgniteInternalFuture<?> registerCachesFut;
Review comment:
Got it. Fixed with re-push to keep the ownership
----------------------------------------------------------------
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