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/02/20 10:49:27 UTC
[GitHub] [ignite] sk0x50 opened a new pull request #7457: IGNITE-12705 SYNC
caches are rebalanced in the first place.
sk0x50 opened a new pull request #7457: IGNITE-12705 SYNC caches are rebalanced in the first place.
URL: https://github.com/apache/ignite/pull/7457
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] sk0x50 commented on a change in pull request #7457:
IGNITE-12705 SYNC caches are rebalanced in the first place.
Posted by GitBox <gi...@apache.org>.
sk0x50 commented on a change in pull request #7457: IGNITE-12705 SYNC caches are rebalanced in the first place.
URL: https://github.com/apache/ignite/pull/7457#discussion_r385756261
##########
File path: modules/core/src/main/java/org/apache/ignite/configuration/CacheConfiguration.java
##########
@@ -1194,14 +1194,15 @@ public CacheRebalanceMode getRebalanceMode() {
* Gets cache rebalance order. Rebalance order can be set to non-zero value for caches with
* {@link CacheRebalanceMode#SYNC SYNC} or {@link CacheRebalanceMode#ASYNC ASYNC} rebalance modes only.
Review comment:
I updated the Javadoc as follows:
> * Gets cache rebalance order. Rebalance order can be set to non-zero value for caches with
> * {@link CacheRebalanceMode#SYNC SYNC} or {@link CacheRebalanceMode#ASYNC ASYNC} rebalance modes only.
> * Note that caches with {@link CacheRebalanceMode#SYNC SYNC} rebalancing mode are always rebalanced prior to caches
> * with {@link CacheRebalanceMode#ASYNC ASYNC} rebalancing mode when rebalancing order is the same.
> * <p/>
> * The rebalance order guarantees that rebalancing for this cache will start only when rebalancing for
> * all caches with smaller rebalance order will be completed.
> * <p/>
> * If not set, cache order is 0.
Does it look good enough?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] Mmuzaf commented on a change in pull request #7457:
IGNITE-12705 SYNC caches are rebalanced in the first place.
Posted by GitBox <gi...@apache.org>.
Mmuzaf commented on a change in pull request #7457: IGNITE-12705 SYNC caches are rebalanced in the first place.
URL: https://github.com/apache/ignite/pull/7457#discussion_r385625134
##########
File path: modules/core/src/main/java/org/apache/ignite/configuration/CacheConfiguration.java
##########
@@ -1194,14 +1194,15 @@ public CacheRebalanceMode getRebalanceMode() {
* Gets cache rebalance order. Rebalance order can be set to non-zero value for caches with
* {@link CacheRebalanceMode#SYNC SYNC} or {@link CacheRebalanceMode#ASYNC ASYNC} rebalance modes only.
Review comment:
I think so since the `order` has complex meaning now.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] Mmuzaf commented on a change in pull request #7457:
IGNITE-12705 SYNC caches are rebalanced in the first place.
Posted by GitBox <gi...@apache.org>.
Mmuzaf commented on a change in pull request #7457: IGNITE-12705 SYNC caches are rebalanced in the first place.
URL: https://github.com/apache/ignite/pull/7457#discussion_r385770831
##########
File path: modules/core/src/test/java/org/apache/ignite/internal/processors/cache/GridCacheRebalanceOrderTest.java
##########
@@ -0,0 +1,177 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.processors.cache;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.IgniteCache;
+import org.apache.ignite.cache.CacheRebalanceMode;
+import org.apache.ignite.configuration.CacheConfiguration;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.events.CacheRebalancingEvent;
+import org.apache.ignite.events.Event;
+import org.apache.ignite.events.EventType;
+import org.apache.ignite.internal.util.typedef.T2;
+import org.apache.ignite.lang.IgniteBiTuple;
+import org.apache.ignite.lang.IgnitePredicate;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.After;
+import org.junit.Test;
+
+import static org.apache.ignite.cache.CacheRebalanceMode.ASYNC;
+import static org.apache.ignite.cache.CacheRebalanceMode.SYNC;
+import static org.apache.ignite.events.EventType.EVT_CACHE_REBALANCE_STOPPED;
+
+/**
+ * Tests that caches with rebalance mode equals to SYNC are rebalanced in the first place.
+ */
+public class GridCacheRebalanceOrderTest extends GridCommonAbstractTest {
+ /** Rebalance timeout. */
+ private static final long REBALANCE_TIMEOUT = 5_000;
+
+ /** Flag indicates that local listener should be used to track rebalance events. */
+ private boolean trackRebalanceEvts;
+
+ /** Caches rebalance events. */
+ private final List<CacheRebalancingEvent> evtsList = Collections.synchronizedList(new ArrayList<>());
+
+ /** {@inheritDoc} */
+ @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception {
+ IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName);
+
+ if (trackRebalanceEvts) {
+ Map<IgnitePredicate<? extends Event>, int[]> listeners = new HashMap<>();
+
+ listeners.put(new IgnitePredicate<CacheRebalancingEvent>() {
+ @Override public boolean apply(CacheRebalancingEvent evt) {
+ evtsList.add(evt);
+
+ return true;
+ }
+ }, new int[] {EVT_CACHE_REBALANCE_STOPPED});
+
+ cfg.setLocalEventListeners(listeners);
+
+ cfg.setIncludeEventTypes(EventType.EVTS_ALL);
+ }
+
+ return cfg;
+ }
+
+ /**
+ * Stops all nodes after test.
+ */
+ @After
+ public void testCleanup() {
+ stopAllGrids();
+ }
+
+ /**
+ * Tests that caches with rebalance mode equals to SYNC are rebalanced in the first place.
+ *
+ * @throws Exception If failed.
+ */
+ @Test
+ public void testRebalanceOrderBasedOnCacheRebalanceMode() throws Exception {
+ Ignite g = startGrid(0);
+
+ // Fix the expected order of rebalance.
+ List<IgniteBiTuple<Integer, CacheRebalanceMode>> order = new ArrayList<>();
+ order.add(new T2<>(0, SYNC));
+ order.add(new T2<>(0, SYNC));
+ order.add(new T2<>(0, ASYNC));
+ order.add(new T2<>(0, ASYNC));
+ order.add(new T2<>(1, SYNC));
+ order.add(new T2<>(1, SYNC));
+ order.add(new T2<>(1, ASYNC));
+ order.add(new T2<>(1, ASYNC));
+
+ // Prepare caches with different rebalance mode and order.
+ List<IgniteCache<Integer, Integer>> caches = new ArrayList<>();
+ for (int i = order.size() - 1; i >= 0; i--) {
+ int rebalanceOrder = order.get(i).get1();
+
+ CacheRebalanceMode rebalanceMode = order.get(i).get2();
+
+ caches.add(g.getOrCreateCache(getCacheConfiguration(
+ "cache-" + i + "-order-" + rebalanceOrder + "-mode-" + rebalanceMode,
+ rebalanceOrder,
+ rebalanceMode)));
+ }
+
+ // Fill values.
+ for (IgniteCache<Integer, Integer> c : caches)
+ c.put(12, 21);
+
+ trackRebalanceEvts = true;
+
+ Ignite g1 = startGrid(1);
+
+ // Wait for all rebalance futures.
+ for (IgniteCache<Integer, Integer> c : caches)
+ grid(1).context().cache().internalCache(c.getName()).preloader().syncFuture().get(REBALANCE_TIMEOUT);
Review comment:
Yes, you're right. Agree
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] sk0x50 commented on a change in pull request #7457:
IGNITE-12705 SYNC caches are rebalanced in the first place.
Posted by GitBox <gi...@apache.org>.
sk0x50 commented on a change in pull request #7457: IGNITE-12705 SYNC caches are rebalanced in the first place.
URL: https://github.com/apache/ignite/pull/7457#discussion_r385760486
##########
File path: modules/core/src/test/java/org/apache/ignite/internal/processors/cache/GridCacheRebalanceOrderTest.java
##########
@@ -0,0 +1,177 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.processors.cache;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.IgniteCache;
+import org.apache.ignite.cache.CacheRebalanceMode;
+import org.apache.ignite.configuration.CacheConfiguration;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.events.CacheRebalancingEvent;
+import org.apache.ignite.events.Event;
+import org.apache.ignite.events.EventType;
+import org.apache.ignite.internal.util.typedef.T2;
+import org.apache.ignite.lang.IgniteBiTuple;
+import org.apache.ignite.lang.IgnitePredicate;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.After;
+import org.junit.Test;
+
+import static org.apache.ignite.cache.CacheRebalanceMode.ASYNC;
+import static org.apache.ignite.cache.CacheRebalanceMode.SYNC;
+import static org.apache.ignite.events.EventType.EVT_CACHE_REBALANCE_STOPPED;
+
+/**
+ * Tests that caches with rebalance mode equals to SYNC are rebalanced in the first place.
+ */
+public class GridCacheRebalanceOrderTest extends GridCommonAbstractTest {
+ /** Rebalance timeout. */
+ private static final long REBALANCE_TIMEOUT = 5_000;
+
+ /** Flag indicates that local listener should be used to track rebalance events. */
+ private boolean trackRebalanceEvts;
+
+ /** Caches rebalance events. */
+ private final List<CacheRebalancingEvent> evtsList = Collections.synchronizedList(new ArrayList<>());
+
+ /** {@inheritDoc} */
+ @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception {
+ IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName);
+
+ if (trackRebalanceEvts) {
+ Map<IgnitePredicate<? extends Event>, int[]> listeners = new HashMap<>();
+
+ listeners.put(new IgnitePredicate<CacheRebalancingEvent>() {
+ @Override public boolean apply(CacheRebalancingEvent evt) {
+ evtsList.add(evt);
+
+ return true;
+ }
+ }, new int[] {EVT_CACHE_REBALANCE_STOPPED});
+
+ cfg.setLocalEventListeners(listeners);
+
+ cfg.setIncludeEventTypes(EventType.EVTS_ALL);
+ }
+
+ return cfg;
+ }
+
+ /**
+ * Stops all nodes after test.
+ */
+ @After
+ public void testCleanup() {
+ stopAllGrids();
+ }
+
+ /**
+ * Tests that caches with rebalance mode equals to SYNC are rebalanced in the first place.
+ *
+ * @throws Exception If failed.
+ */
+ @Test
+ public void testRebalanceOrderBasedOnCacheRebalanceMode() throws Exception {
+ Ignite g = startGrid(0);
+
+ // Fix the expected order of rebalance.
+ List<IgniteBiTuple<Integer, CacheRebalanceMode>> order = new ArrayList<>();
+ order.add(new T2<>(0, SYNC));
+ order.add(new T2<>(0, SYNC));
+ order.add(new T2<>(0, ASYNC));
+ order.add(new T2<>(0, ASYNC));
+ order.add(new T2<>(1, SYNC));
+ order.add(new T2<>(1, SYNC));
+ order.add(new T2<>(1, ASYNC));
+ order.add(new T2<>(1, ASYNC));
+
+ // Prepare caches with different rebalance mode and order.
+ List<IgniteCache<Integer, Integer>> caches = new ArrayList<>();
+ for (int i = order.size() - 1; i >= 0; i--) {
+ int rebalanceOrder = order.get(i).get1();
+
+ CacheRebalanceMode rebalanceMode = order.get(i).get2();
+
+ caches.add(g.getOrCreateCache(getCacheConfiguration(
+ "cache-" + i + "-order-" + rebalanceOrder + "-mode-" + rebalanceMode,
+ rebalanceOrder,
+ rebalanceMode)));
+ }
+
+ // Fill values.
+ for (IgniteCache<Integer, Integer> c : caches)
+ c.put(12, 21);
+
+ trackRebalanceEvts = true;
+
+ Ignite g1 = startGrid(1);
+
+ // Wait for all rebalance futures.
+ for (IgniteCache<Integer, Integer> c : caches)
+ grid(1).context().cache().internalCache(c.getName()).preloader().syncFuture().get(REBALANCE_TIMEOUT);
Review comment:
`evtsList` should contain all events after the following loop:
```
// Wait for all rebalance futures.
for (IgniteCache<Integer, Integer> c : caches)
grid(1).context().cache().internalCache(c.getName()).preloader().syncFuture().get(REBALANCE_TIMEOUT);
```
In other words, `syncFuture().get()` guarantees that initial rebalancing for the cache is already completed. It does not matter which cache rebalancing mode is used - `SYNC` or `ASYNC`.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] sk0x50 commented on a change in pull request #7457:
IGNITE-12705 SYNC caches are rebalanced in the first place.
Posted by GitBox <gi...@apache.org>.
sk0x50 commented on a change in pull request #7457: IGNITE-12705 SYNC caches are rebalanced in the first place.
URL: https://github.com/apache/ignite/pull/7457#discussion_r384516335
##########
File path: modules/core/src/main/java/org/apache/ignite/configuration/CacheConfiguration.java
##########
@@ -1194,14 +1194,15 @@ public CacheRebalanceMode getRebalanceMode() {
* Gets cache rebalance order. Rebalance order can be set to non-zero value for caches with
* {@link CacheRebalanceMode#SYNC SYNC} or {@link CacheRebalanceMode#ASYNC ASYNC} rebalance modes only.
Review comment:
Well, IMHO it is already mentioned below:
> * Note that a cache with {@link CacheRebalanceMode#SYNC SYNC} rebalancing mode always takes precedence
> * over caches with {@link CacheRebalanceMode#ASYNC ASYNC} rebalancing mode
> * in the group of caches with the same rebalance order. This means caches with {@link CacheRebalanceMode#SYNC SYNC}
> * rebalancing mode will be rebalanced in the first place.
Do you prefer that this fact should be highlighted in the very beginning?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] Mmuzaf commented on a change in pull request #7457:
IGNITE-12705 SYNC caches are rebalanced in the first place.
Posted by GitBox <gi...@apache.org>.
Mmuzaf commented on a change in pull request #7457: IGNITE-12705 SYNC caches are rebalanced in the first place.
URL: https://github.com/apache/ignite/pull/7457#discussion_r385630429
##########
File path: modules/core/src/test/java/org/apache/ignite/internal/processors/cache/GridCacheRebalanceOrderTest.java
##########
@@ -0,0 +1,177 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.processors.cache;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.IgniteCache;
+import org.apache.ignite.cache.CacheRebalanceMode;
+import org.apache.ignite.configuration.CacheConfiguration;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.events.CacheRebalancingEvent;
+import org.apache.ignite.events.Event;
+import org.apache.ignite.events.EventType;
+import org.apache.ignite.internal.util.typedef.T2;
+import org.apache.ignite.lang.IgniteBiTuple;
+import org.apache.ignite.lang.IgnitePredicate;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.After;
+import org.junit.Test;
+
+import static org.apache.ignite.cache.CacheRebalanceMode.ASYNC;
+import static org.apache.ignite.cache.CacheRebalanceMode.SYNC;
+import static org.apache.ignite.events.EventType.EVT_CACHE_REBALANCE_STOPPED;
+
+/**
+ * Tests that caches with rebalance mode equals to SYNC are rebalanced in the first place.
+ */
+public class GridCacheRebalanceOrderTest extends GridCommonAbstractTest {
+ /** Rebalance timeout. */
+ private static final long REBALANCE_TIMEOUT = 5_000;
+
+ /** Flag indicates that local listener should be used to track rebalance events. */
+ private boolean trackRebalanceEvts;
+
+ /** Caches rebalance events. */
+ private final List<CacheRebalancingEvent> evtsList = Collections.synchronizedList(new ArrayList<>());
+
+ /** {@inheritDoc} */
+ @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception {
+ IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName);
+
+ if (trackRebalanceEvts) {
+ Map<IgnitePredicate<? extends Event>, int[]> listeners = new HashMap<>();
+
+ listeners.put(new IgnitePredicate<CacheRebalancingEvent>() {
+ @Override public boolean apply(CacheRebalancingEvent evt) {
+ evtsList.add(evt);
+
+ return true;
+ }
+ }, new int[] {EVT_CACHE_REBALANCE_STOPPED});
+
+ cfg.setLocalEventListeners(listeners);
+
+ cfg.setIncludeEventTypes(EventType.EVTS_ALL);
+ }
+
+ return cfg;
+ }
+
+ /**
+ * Stops all nodes after test.
+ */
+ @After
+ public void testCleanup() {
+ stopAllGrids();
+ }
+
+ /**
+ * Tests that caches with rebalance mode equals to SYNC are rebalanced in the first place.
+ *
+ * @throws Exception If failed.
+ */
+ @Test
+ public void testRebalanceOrderBasedOnCacheRebalanceMode() throws Exception {
+ Ignite g = startGrid(0);
+
+ // Fix the expected order of rebalance.
+ List<IgniteBiTuple<Integer, CacheRebalanceMode>> order = new ArrayList<>();
+ order.add(new T2<>(0, SYNC));
+ order.add(new T2<>(0, SYNC));
+ order.add(new T2<>(0, ASYNC));
+ order.add(new T2<>(0, ASYNC));
+ order.add(new T2<>(1, SYNC));
+ order.add(new T2<>(1, SYNC));
+ order.add(new T2<>(1, ASYNC));
+ order.add(new T2<>(1, ASYNC));
+
+ // Prepare caches with different rebalance mode and order.
+ List<IgniteCache<Integer, Integer>> caches = new ArrayList<>();
+ for (int i = order.size() - 1; i >= 0; i--) {
+ int rebalanceOrder = order.get(i).get1();
+
+ CacheRebalanceMode rebalanceMode = order.get(i).get2();
+
+ caches.add(g.getOrCreateCache(getCacheConfiguration(
+ "cache-" + i + "-order-" + rebalanceOrder + "-mode-" + rebalanceMode,
+ rebalanceOrder,
+ rebalanceMode)));
+ }
+
+ // Fill values.
+ for (IgniteCache<Integer, Integer> c : caches)
+ c.put(12, 21);
+
+ trackRebalanceEvts = true;
+
+ Ignite g1 = startGrid(1);
+
+ // Wait for all rebalance futures.
+ for (IgniteCache<Integer, Integer> c : caches)
+ grid(1).context().cache().internalCache(c.getName()).preloader().syncFuture().get(REBALANCE_TIMEOUT);
Review comment:
As far I can see, the `evtsList` may not contains all of the rebalanced caches since at the moment of the current thread will be released and we start checking for cache order (in the loop above) the rebalance still may not be finished for ASYNC caches. Right?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] Mmuzaf commented on a change in pull request #7457:
IGNITE-12705 SYNC caches are rebalanced in the first place.
Posted by GitBox <gi...@apache.org>.
Mmuzaf commented on a change in pull request #7457: IGNITE-12705 SYNC caches are rebalanced in the first place.
URL: https://github.com/apache/ignite/pull/7457#discussion_r384395229
##########
File path: modules/core/src/test/java/org/apache/ignite/internal/processors/cache/GridCacheRebalanceOrderTest.java
##########
@@ -0,0 +1,177 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.processors.cache;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.IgniteCache;
+import org.apache.ignite.cache.CacheRebalanceMode;
+import org.apache.ignite.configuration.CacheConfiguration;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.events.CacheRebalancingEvent;
+import org.apache.ignite.events.Event;
+import org.apache.ignite.events.EventType;
+import org.apache.ignite.internal.util.typedef.T2;
+import org.apache.ignite.lang.IgniteBiTuple;
+import org.apache.ignite.lang.IgnitePredicate;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.After;
+import org.junit.Test;
+
+import static org.apache.ignite.cache.CacheRebalanceMode.ASYNC;
+import static org.apache.ignite.cache.CacheRebalanceMode.SYNC;
+import static org.apache.ignite.events.EventType.EVT_CACHE_REBALANCE_STOPPED;
+
+/**
+ * Tests that caches with rebalance mode equals to SYNC are rebalanced in the first place.
+ */
+public class GridCacheRebalanceOrderTest extends GridCommonAbstractTest {
+ /** Rebalance timeout. */
+ private static final long REBALANCE_TIMEOUT = 5_000;
+
+ /** Flag indicates that local listener should be used to track rebalance events. */
+ private boolean trackRebalanceEvts;
+
+ /** Caches rebalance events. */
+ private final List<CacheRebalancingEvent> evtsList = Collections.synchronizedList(new ArrayList<>());
+
+ /** {@inheritDoc} */
+ @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception {
+ IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName);
+
+ if (trackRebalanceEvts) {
+ Map<IgnitePredicate<? extends Event>, int[]> listeners = new HashMap<>();
+
+ listeners.put(new IgnitePredicate<CacheRebalancingEvent>() {
+ @Override public boolean apply(CacheRebalancingEvent evt) {
+ evtsList.add(evt);
+
+ return true;
+ }
+ }, new int[] {EVT_CACHE_REBALANCE_STOPPED});
Review comment:
Do we have a strong guarantee that such events will be received by listeners with the same order they fired?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] sk0x50 commented on a change in pull request #7457:
IGNITE-12705 SYNC caches are rebalanced in the first place.
Posted by GitBox <gi...@apache.org>.
sk0x50 commented on a change in pull request #7457: IGNITE-12705 SYNC caches are rebalanced in the first place.
URL: https://github.com/apache/ignite/pull/7457#discussion_r385760486
##########
File path: modules/core/src/test/java/org/apache/ignite/internal/processors/cache/GridCacheRebalanceOrderTest.java
##########
@@ -0,0 +1,177 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.processors.cache;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.IgniteCache;
+import org.apache.ignite.cache.CacheRebalanceMode;
+import org.apache.ignite.configuration.CacheConfiguration;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.events.CacheRebalancingEvent;
+import org.apache.ignite.events.Event;
+import org.apache.ignite.events.EventType;
+import org.apache.ignite.internal.util.typedef.T2;
+import org.apache.ignite.lang.IgniteBiTuple;
+import org.apache.ignite.lang.IgnitePredicate;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.After;
+import org.junit.Test;
+
+import static org.apache.ignite.cache.CacheRebalanceMode.ASYNC;
+import static org.apache.ignite.cache.CacheRebalanceMode.SYNC;
+import static org.apache.ignite.events.EventType.EVT_CACHE_REBALANCE_STOPPED;
+
+/**
+ * Tests that caches with rebalance mode equals to SYNC are rebalanced in the first place.
+ */
+public class GridCacheRebalanceOrderTest extends GridCommonAbstractTest {
+ /** Rebalance timeout. */
+ private static final long REBALANCE_TIMEOUT = 5_000;
+
+ /** Flag indicates that local listener should be used to track rebalance events. */
+ private boolean trackRebalanceEvts;
+
+ /** Caches rebalance events. */
+ private final List<CacheRebalancingEvent> evtsList = Collections.synchronizedList(new ArrayList<>());
+
+ /** {@inheritDoc} */
+ @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception {
+ IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName);
+
+ if (trackRebalanceEvts) {
+ Map<IgnitePredicate<? extends Event>, int[]> listeners = new HashMap<>();
+
+ listeners.put(new IgnitePredicate<CacheRebalancingEvent>() {
+ @Override public boolean apply(CacheRebalancingEvent evt) {
+ evtsList.add(evt);
+
+ return true;
+ }
+ }, new int[] {EVT_CACHE_REBALANCE_STOPPED});
+
+ cfg.setLocalEventListeners(listeners);
+
+ cfg.setIncludeEventTypes(EventType.EVTS_ALL);
+ }
+
+ return cfg;
+ }
+
+ /**
+ * Stops all nodes after test.
+ */
+ @After
+ public void testCleanup() {
+ stopAllGrids();
+ }
+
+ /**
+ * Tests that caches with rebalance mode equals to SYNC are rebalanced in the first place.
+ *
+ * @throws Exception If failed.
+ */
+ @Test
+ public void testRebalanceOrderBasedOnCacheRebalanceMode() throws Exception {
+ Ignite g = startGrid(0);
+
+ // Fix the expected order of rebalance.
+ List<IgniteBiTuple<Integer, CacheRebalanceMode>> order = new ArrayList<>();
+ order.add(new T2<>(0, SYNC));
+ order.add(new T2<>(0, SYNC));
+ order.add(new T2<>(0, ASYNC));
+ order.add(new T2<>(0, ASYNC));
+ order.add(new T2<>(1, SYNC));
+ order.add(new T2<>(1, SYNC));
+ order.add(new T2<>(1, ASYNC));
+ order.add(new T2<>(1, ASYNC));
+
+ // Prepare caches with different rebalance mode and order.
+ List<IgniteCache<Integer, Integer>> caches = new ArrayList<>();
+ for (int i = order.size() - 1; i >= 0; i--) {
+ int rebalanceOrder = order.get(i).get1();
+
+ CacheRebalanceMode rebalanceMode = order.get(i).get2();
+
+ caches.add(g.getOrCreateCache(getCacheConfiguration(
+ "cache-" + i + "-order-" + rebalanceOrder + "-mode-" + rebalanceMode,
+ rebalanceOrder,
+ rebalanceMode)));
+ }
+
+ // Fill values.
+ for (IgniteCache<Integer, Integer> c : caches)
+ c.put(12, 21);
+
+ trackRebalanceEvts = true;
+
+ Ignite g1 = startGrid(1);
+
+ // Wait for all rebalance futures.
+ for (IgniteCache<Integer, Integer> c : caches)
+ grid(1).context().cache().internalCache(c.getName()).preloader().syncFuture().get(REBALANCE_TIMEOUT);
Review comment:
`evtsList` should contain all events after the following loop:
```
// Wait for all rebalance futures.
for (IgniteCache<Integer, Integer> c : caches)
grid(1).context().cache().internalCache(c.getName()).preloader().syncFuture().get(REBALANCE_TIMEOUT);
```
In other words, `syncFuture().get()` guarantees that initial rebalancing for the cache is already completed. It does not matter which cache rebalancing mode is used - `SYNC` or `ASYNC`.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] sk0x50 commented on a change in pull request #7457:
IGNITE-12705 SYNC caches are rebalanced in the first place.
Posted by GitBox <gi...@apache.org>.
sk0x50 commented on a change in pull request #7457: IGNITE-12705 SYNC caches are rebalanced in the first place.
URL: https://github.com/apache/ignite/pull/7457#discussion_r385760486
##########
File path: modules/core/src/test/java/org/apache/ignite/internal/processors/cache/GridCacheRebalanceOrderTest.java
##########
@@ -0,0 +1,177 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.processors.cache;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.IgniteCache;
+import org.apache.ignite.cache.CacheRebalanceMode;
+import org.apache.ignite.configuration.CacheConfiguration;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.events.CacheRebalancingEvent;
+import org.apache.ignite.events.Event;
+import org.apache.ignite.events.EventType;
+import org.apache.ignite.internal.util.typedef.T2;
+import org.apache.ignite.lang.IgniteBiTuple;
+import org.apache.ignite.lang.IgnitePredicate;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.After;
+import org.junit.Test;
+
+import static org.apache.ignite.cache.CacheRebalanceMode.ASYNC;
+import static org.apache.ignite.cache.CacheRebalanceMode.SYNC;
+import static org.apache.ignite.events.EventType.EVT_CACHE_REBALANCE_STOPPED;
+
+/**
+ * Tests that caches with rebalance mode equals to SYNC are rebalanced in the first place.
+ */
+public class GridCacheRebalanceOrderTest extends GridCommonAbstractTest {
+ /** Rebalance timeout. */
+ private static final long REBALANCE_TIMEOUT = 5_000;
+
+ /** Flag indicates that local listener should be used to track rebalance events. */
+ private boolean trackRebalanceEvts;
+
+ /** Caches rebalance events. */
+ private final List<CacheRebalancingEvent> evtsList = Collections.synchronizedList(new ArrayList<>());
+
+ /** {@inheritDoc} */
+ @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception {
+ IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName);
+
+ if (trackRebalanceEvts) {
+ Map<IgnitePredicate<? extends Event>, int[]> listeners = new HashMap<>();
+
+ listeners.put(new IgnitePredicate<CacheRebalancingEvent>() {
+ @Override public boolean apply(CacheRebalancingEvent evt) {
+ evtsList.add(evt);
+
+ return true;
+ }
+ }, new int[] {EVT_CACHE_REBALANCE_STOPPED});
+
+ cfg.setLocalEventListeners(listeners);
+
+ cfg.setIncludeEventTypes(EventType.EVTS_ALL);
+ }
+
+ return cfg;
+ }
+
+ /**
+ * Stops all nodes after test.
+ */
+ @After
+ public void testCleanup() {
+ stopAllGrids();
+ }
+
+ /**
+ * Tests that caches with rebalance mode equals to SYNC are rebalanced in the first place.
+ *
+ * @throws Exception If failed.
+ */
+ @Test
+ public void testRebalanceOrderBasedOnCacheRebalanceMode() throws Exception {
+ Ignite g = startGrid(0);
+
+ // Fix the expected order of rebalance.
+ List<IgniteBiTuple<Integer, CacheRebalanceMode>> order = new ArrayList<>();
+ order.add(new T2<>(0, SYNC));
+ order.add(new T2<>(0, SYNC));
+ order.add(new T2<>(0, ASYNC));
+ order.add(new T2<>(0, ASYNC));
+ order.add(new T2<>(1, SYNC));
+ order.add(new T2<>(1, SYNC));
+ order.add(new T2<>(1, ASYNC));
+ order.add(new T2<>(1, ASYNC));
+
+ // Prepare caches with different rebalance mode and order.
+ List<IgniteCache<Integer, Integer>> caches = new ArrayList<>();
+ for (int i = order.size() - 1; i >= 0; i--) {
+ int rebalanceOrder = order.get(i).get1();
+
+ CacheRebalanceMode rebalanceMode = order.get(i).get2();
+
+ caches.add(g.getOrCreateCache(getCacheConfiguration(
+ "cache-" + i + "-order-" + rebalanceOrder + "-mode-" + rebalanceMode,
+ rebalanceOrder,
+ rebalanceMode)));
+ }
+
+ // Fill values.
+ for (IgniteCache<Integer, Integer> c : caches)
+ c.put(12, 21);
+
+ trackRebalanceEvts = true;
+
+ Ignite g1 = startGrid(1);
+
+ // Wait for all rebalance futures.
+ for (IgniteCache<Integer, Integer> c : caches)
+ grid(1).context().cache().internalCache(c.getName()).preloader().syncFuture().get(REBALANCE_TIMEOUT);
Review comment:
`evtsList` should contain all events after the following loop:
```
// Wait for all rebalance futures.
for (IgniteCache<Integer, Integer> c : caches)
grid(1).context().cache().internalCache(c.getName()).preloader().syncFuture().get(REBALANCE_TIMEOUT);
```
In other words, `syncFuture().get()` guarantees that initial rebalancing for the cache is already completed. It does not matter which cache rebalancing mode is used - `SYNC` or `ASYNC`.
Moreover, the event `EVT_CACHE_REBALANCE_STOPPED` is fired before the `syncFuture` will be done.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] Mmuzaf commented on a change in pull request #7457:
IGNITE-12705 SYNC caches are rebalanced in the first place.
Posted by GitBox <gi...@apache.org>.
Mmuzaf commented on a change in pull request #7457: IGNITE-12705 SYNC caches are rebalanced in the first place.
URL: https://github.com/apache/ignite/pull/7457#discussion_r385628563
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCachePartitionExchangeManager.java
##########
@@ -3846,4 +3849,66 @@ void trim() {
activeObjects.clear();
}
}
+
+ /**
+ * Represents a cache rebalance order that takes into account both values: rebalance order itself and rebalance mode.
+ * It is assumed SYNC caches should be rebalanced in the first place.
+ */
+ private static class CacheRebalanceOrder implements Comparable<CacheRebalanceOrder> {
Review comment:
Agree.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] Mmuzaf commented on a change in pull request #7457:
IGNITE-12705 SYNC caches are rebalanced in the first place.
Posted by GitBox <gi...@apache.org>.
Mmuzaf commented on a change in pull request #7457: IGNITE-12705 SYNC caches are rebalanced in the first place.
URL: https://github.com/apache/ignite/pull/7457#discussion_r385630429
##########
File path: modules/core/src/test/java/org/apache/ignite/internal/processors/cache/GridCacheRebalanceOrderTest.java
##########
@@ -0,0 +1,177 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.processors.cache;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.IgniteCache;
+import org.apache.ignite.cache.CacheRebalanceMode;
+import org.apache.ignite.configuration.CacheConfiguration;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.events.CacheRebalancingEvent;
+import org.apache.ignite.events.Event;
+import org.apache.ignite.events.EventType;
+import org.apache.ignite.internal.util.typedef.T2;
+import org.apache.ignite.lang.IgniteBiTuple;
+import org.apache.ignite.lang.IgnitePredicate;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.After;
+import org.junit.Test;
+
+import static org.apache.ignite.cache.CacheRebalanceMode.ASYNC;
+import static org.apache.ignite.cache.CacheRebalanceMode.SYNC;
+import static org.apache.ignite.events.EventType.EVT_CACHE_REBALANCE_STOPPED;
+
+/**
+ * Tests that caches with rebalance mode equals to SYNC are rebalanced in the first place.
+ */
+public class GridCacheRebalanceOrderTest extends GridCommonAbstractTest {
+ /** Rebalance timeout. */
+ private static final long REBALANCE_TIMEOUT = 5_000;
+
+ /** Flag indicates that local listener should be used to track rebalance events. */
+ private boolean trackRebalanceEvts;
+
+ /** Caches rebalance events. */
+ private final List<CacheRebalancingEvent> evtsList = Collections.synchronizedList(new ArrayList<>());
+
+ /** {@inheritDoc} */
+ @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception {
+ IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName);
+
+ if (trackRebalanceEvts) {
+ Map<IgnitePredicate<? extends Event>, int[]> listeners = new HashMap<>();
+
+ listeners.put(new IgnitePredicate<CacheRebalancingEvent>() {
+ @Override public boolean apply(CacheRebalancingEvent evt) {
+ evtsList.add(evt);
+
+ return true;
+ }
+ }, new int[] {EVT_CACHE_REBALANCE_STOPPED});
+
+ cfg.setLocalEventListeners(listeners);
+
+ cfg.setIncludeEventTypes(EventType.EVTS_ALL);
+ }
+
+ return cfg;
+ }
+
+ /**
+ * Stops all nodes after test.
+ */
+ @After
+ public void testCleanup() {
+ stopAllGrids();
+ }
+
+ /**
+ * Tests that caches with rebalance mode equals to SYNC are rebalanced in the first place.
+ *
+ * @throws Exception If failed.
+ */
+ @Test
+ public void testRebalanceOrderBasedOnCacheRebalanceMode() throws Exception {
+ Ignite g = startGrid(0);
+
+ // Fix the expected order of rebalance.
+ List<IgniteBiTuple<Integer, CacheRebalanceMode>> order = new ArrayList<>();
+ order.add(new T2<>(0, SYNC));
+ order.add(new T2<>(0, SYNC));
+ order.add(new T2<>(0, ASYNC));
+ order.add(new T2<>(0, ASYNC));
+ order.add(new T2<>(1, SYNC));
+ order.add(new T2<>(1, SYNC));
+ order.add(new T2<>(1, ASYNC));
+ order.add(new T2<>(1, ASYNC));
+
+ // Prepare caches with different rebalance mode and order.
+ List<IgniteCache<Integer, Integer>> caches = new ArrayList<>();
+ for (int i = order.size() - 1; i >= 0; i--) {
+ int rebalanceOrder = order.get(i).get1();
+
+ CacheRebalanceMode rebalanceMode = order.get(i).get2();
+
+ caches.add(g.getOrCreateCache(getCacheConfiguration(
+ "cache-" + i + "-order-" + rebalanceOrder + "-mode-" + rebalanceMode,
+ rebalanceOrder,
+ rebalanceMode)));
+ }
+
+ // Fill values.
+ for (IgniteCache<Integer, Integer> c : caches)
+ c.put(12, 21);
+
+ trackRebalanceEvts = true;
+
+ Ignite g1 = startGrid(1);
+
+ // Wait for all rebalance futures.
+ for (IgniteCache<Integer, Integer> c : caches)
+ grid(1).context().cache().internalCache(c.getName()).preloader().syncFuture().get(REBALANCE_TIMEOUT);
Review comment:
As far I can see, the `evtsList` may not contains all of the rebalanced caches since at the moment of the current thread will be released and we start checking for cache orders (in the loop below) the rebalance still may not be finished for ASYNC caches. Right?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] sk0x50 closed pull request #7457: IGNITE-12705 SYNC caches
are rebalanced in the first place.
Posted by GitBox <gi...@apache.org>.
sk0x50 closed pull request #7457: IGNITE-12705 SYNC caches are rebalanced in the first place.
URL: https://github.com/apache/ignite/pull/7457
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] sk0x50 commented on a change in pull request #7457:
IGNITE-12705 SYNC caches are rebalanced in the first place.
Posted by GitBox <gi...@apache.org>.
sk0x50 commented on a change in pull request #7457: IGNITE-12705 SYNC caches are rebalanced in the first place.
URL: https://github.com/apache/ignite/pull/7457#discussion_r384524612
##########
File path: modules/core/src/test/java/org/apache/ignite/internal/processors/cache/GridCacheRebalanceOrderTest.java
##########
@@ -0,0 +1,177 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.processors.cache;
Review comment:
Thanks for pointing this out! Will do!
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] Mmuzaf commented on a change in pull request #7457:
IGNITE-12705 SYNC caches are rebalanced in the first place.
Posted by GitBox <gi...@apache.org>.
Mmuzaf commented on a change in pull request #7457: IGNITE-12705 SYNC caches are rebalanced in the first place.
URL: https://github.com/apache/ignite/pull/7457#discussion_r384397007
##########
File path: modules/core/src/test/java/org/apache/ignite/internal/processors/cache/GridCacheRebalanceOrderTest.java
##########
@@ -0,0 +1,177 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.processors.cache;
Review comment:
Let's move the test here: `package org.apache.ignite.internal.processors.cache.distributed.rebalancing;`
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] sk0x50 commented on a change in pull request #7457:
IGNITE-12705 SYNC caches are rebalanced in the first place.
Posted by GitBox <gi...@apache.org>.
sk0x50 commented on a change in pull request #7457: IGNITE-12705 SYNC caches are rebalanced in the first place.
URL: https://github.com/apache/ignite/pull/7457#discussion_r384526619
##########
File path: modules/core/src/test/java/org/apache/ignite/internal/processors/cache/GridCacheRebalanceOrderTest.java
##########
@@ -0,0 +1,177 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.processors.cache;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.IgniteCache;
+import org.apache.ignite.cache.CacheRebalanceMode;
+import org.apache.ignite.configuration.CacheConfiguration;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.events.CacheRebalancingEvent;
+import org.apache.ignite.events.Event;
+import org.apache.ignite.events.EventType;
+import org.apache.ignite.internal.util.typedef.T2;
+import org.apache.ignite.lang.IgniteBiTuple;
+import org.apache.ignite.lang.IgnitePredicate;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.After;
+import org.junit.Test;
+
+import static org.apache.ignite.cache.CacheRebalanceMode.ASYNC;
+import static org.apache.ignite.cache.CacheRebalanceMode.SYNC;
+import static org.apache.ignite.events.EventType.EVT_CACHE_REBALANCE_STOPPED;
+
+/**
+ * Tests that caches with rebalance mode equals to SYNC are rebalanced in the first place.
+ */
+public class GridCacheRebalanceOrderTest extends GridCommonAbstractTest {
+ /** Rebalance timeout. */
+ private static final long REBALANCE_TIMEOUT = 5_000;
+
+ /** Flag indicates that local listener should be used to track rebalance events. */
+ private boolean trackRebalanceEvts;
+
+ /** Caches rebalance events. */
+ private final List<CacheRebalancingEvent> evtsList = Collections.synchronizedList(new ArrayList<>());
+
+ /** {@inheritDoc} */
+ @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception {
+ IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName);
+
+ if (trackRebalanceEvts) {
+ Map<IgnitePredicate<? extends Event>, int[]> listeners = new HashMap<>();
+
+ listeners.put(new IgnitePredicate<CacheRebalancingEvent>() {
+ @Override public boolean apply(CacheRebalancingEvent evt) {
+ evtsList.add(evt);
+
+ return true;
+ }
+ }, new int[] {EVT_CACHE_REBALANCE_STOPPED});
Review comment:
If I am not mistaken, the answer is yes, we have such a guarantee. Unfortunately, it is not clearly stated in the Javadoc.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] Mmuzaf commented on a change in pull request #7457:
IGNITE-12705 SYNC caches are rebalanced in the first place.
Posted by GitBox <gi...@apache.org>.
Mmuzaf commented on a change in pull request #7457: IGNITE-12705 SYNC caches are rebalanced in the first place.
URL: https://github.com/apache/ignite/pull/7457#discussion_r384392166
##########
File path: modules/core/src/main/java/org/apache/ignite/configuration/CacheConfiguration.java
##########
@@ -1194,14 +1194,15 @@ public CacheRebalanceMode getRebalanceMode() {
* Gets cache rebalance order. Rebalance order can be set to non-zero value for caches with
* {@link CacheRebalanceMode#SYNC SYNC} or {@link CacheRebalanceMode#ASYNC ASYNC} rebalance modes only.
Review comment:
I think we should also mention for SYNC, ASYNC cache mode in javadoc something like 'SYNC caches with the same order will always be rebalanced first'
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] sk0x50 commented on a change in pull request #7457:
IGNITE-12705 SYNC caches are rebalanced in the first place.
Posted by GitBox <gi...@apache.org>.
sk0x50 commented on a change in pull request #7457: IGNITE-12705 SYNC caches are rebalanced in the first place.
URL: https://github.com/apache/ignite/pull/7457#discussion_r384531773
##########
File path: modules/core/src/test/java/org/apache/ignite/internal/processors/cache/GridCacheRebalanceOrderTest.java
##########
@@ -0,0 +1,177 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.processors.cache;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.IgniteCache;
+import org.apache.ignite.cache.CacheRebalanceMode;
+import org.apache.ignite.configuration.CacheConfiguration;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.events.CacheRebalancingEvent;
+import org.apache.ignite.events.Event;
+import org.apache.ignite.events.EventType;
+import org.apache.ignite.internal.util.typedef.T2;
+import org.apache.ignite.lang.IgniteBiTuple;
+import org.apache.ignite.lang.IgnitePredicate;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.After;
+import org.junit.Test;
+
+import static org.apache.ignite.cache.CacheRebalanceMode.ASYNC;
+import static org.apache.ignite.cache.CacheRebalanceMode.SYNC;
+import static org.apache.ignite.events.EventType.EVT_CACHE_REBALANCE_STOPPED;
+
+/**
+ * Tests that caches with rebalance mode equals to SYNC are rebalanced in the first place.
+ */
+public class GridCacheRebalanceOrderTest extends GridCommonAbstractTest {
+ /** Rebalance timeout. */
+ private static final long REBALANCE_TIMEOUT = 5_000;
+
+ /** Flag indicates that local listener should be used to track rebalance events. */
+ private boolean trackRebalanceEvts;
+
+ /** Caches rebalance events. */
+ private final List<CacheRebalancingEvent> evtsList = Collections.synchronizedList(new ArrayList<>());
+
+ /** {@inheritDoc} */
+ @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception {
+ IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName);
+
+ if (trackRebalanceEvts) {
+ Map<IgnitePredicate<? extends Event>, int[]> listeners = new HashMap<>();
+
+ listeners.put(new IgnitePredicate<CacheRebalancingEvent>() {
+ @Override public boolean apply(CacheRebalancingEvent evt) {
+ evtsList.add(evt);
+
+ return true;
+ }
+ }, new int[] {EVT_CACHE_REBALANCE_STOPPED});
+
+ cfg.setLocalEventListeners(listeners);
+
+ cfg.setIncludeEventTypes(EventType.EVTS_ALL);
+ }
+
+ return cfg;
+ }
+
+ /**
+ * Stops all nodes after test.
+ */
+ @After
+ public void testCleanup() {
+ stopAllGrids();
+ }
+
+ /**
+ * Tests that caches with rebalance mode equals to SYNC are rebalanced in the first place.
+ *
+ * @throws Exception If failed.
+ */
+ @Test
+ public void testRebalanceOrderBasedOnCacheRebalanceMode() throws Exception {
+ Ignite g = startGrid(0);
+
+ // Fix the expected order of rebalance.
+ List<IgniteBiTuple<Integer, CacheRebalanceMode>> order = new ArrayList<>();
+ order.add(new T2<>(0, SYNC));
+ order.add(new T2<>(0, SYNC));
+ order.add(new T2<>(0, ASYNC));
+ order.add(new T2<>(0, ASYNC));
+ order.add(new T2<>(1, SYNC));
+ order.add(new T2<>(1, SYNC));
+ order.add(new T2<>(1, ASYNC));
+ order.add(new T2<>(1, ASYNC));
+
+ // Prepare caches with different rebalance mode and order.
+ List<IgniteCache<Integer, Integer>> caches = new ArrayList<>();
+ for (int i = order.size() - 1; i >= 0; i--) {
+ int rebalanceOrder = order.get(i).get1();
+
+ CacheRebalanceMode rebalanceMode = order.get(i).get2();
+
+ caches.add(g.getOrCreateCache(getCacheConfiguration(
+ "cache-" + i + "-order-" + rebalanceOrder + "-mode-" + rebalanceMode,
+ rebalanceOrder,
+ rebalanceMode)));
+ }
+
+ // Fill values.
+ for (IgniteCache<Integer, Integer> c : caches)
+ c.put(12, 21);
+
+ trackRebalanceEvts = true;
+
+ Ignite g1 = startGrid(1);
+
+ // Wait for all rebalance futures.
+ for (IgniteCache<Integer, Integer> c : caches)
+ grid(1).context().cache().internalCache(c.getName()).preloader().syncFuture().get(REBALANCE_TIMEOUT);
Review comment:
Yep, and that is the case! :)
The most important impact of the proposed change is "unblocking" a user thread that is starting Ignite instance.
As you know, `Ignition.start()` blocks the thread until the node joined the cluster and all `SYNC` caches are rebalanced.
So, the fix allows us to "speed up" the start of the node, from the user point of view.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] sk0x50 commented on a change in pull request #7457:
IGNITE-12705 SYNC caches are rebalanced in the first place.
Posted by GitBox <gi...@apache.org>.
sk0x50 commented on a change in pull request #7457: IGNITE-12705 SYNC caches are rebalanced in the first place.
URL: https://github.com/apache/ignite/pull/7457#discussion_r384523982
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCachePartitionExchangeManager.java
##########
@@ -3846,4 +3849,66 @@ void trim() {
activeObjects.clear();
}
}
+
+ /**
+ * Represents a cache rebalance order that takes into account both values: rebalance order itself and rebalance mode.
+ * It is assumed SYNC caches should be rebalanced in the first place.
+ */
+ private static class CacheRebalanceOrder implements Comparable<CacheRebalanceOrder> {
Review comment:
Yes, it can be replaced. On the other hand, in this case, we need to implement two classes (`CacheRebalanceOrder` and `Comparator` itself) instead of one. The method equals can be removed in both cases I think, but I would prefer to support the general contract of `Map` (it is easy to implement and maintain).
> Note that the ordering maintained by a tree map, like any sorted map, and whether or not an explicit comparator is provided, must be consistent with equals if this sorted map is to correctly implement the Map interface. (See Comparable or Comparator for a precise definition of consistent with equals.) This is so because the Map interface is defined in terms of the equals operation, but a sorted map performs all key comparisons using its compareTo (or compare) method, so two keys that are deemed equal by this method are, from the standpoint of the sorted map, equal. The behavior of a sorted map is well-defined even if its ordering is inconsistent with equals; it just fails to obey the general contract of the Map interface.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] Mmuzaf commented on a change in pull request #7457:
IGNITE-12705 SYNC caches are rebalanced in the first place.
Posted by GitBox <gi...@apache.org>.
Mmuzaf commented on a change in pull request #7457: IGNITE-12705 SYNC caches are rebalanced in the first place.
URL: https://github.com/apache/ignite/pull/7457#discussion_r384400380
##########
File path: modules/core/src/test/java/org/apache/ignite/internal/processors/cache/GridCacheRebalanceOrderTest.java
##########
@@ -0,0 +1,177 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.processors.cache;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.IgniteCache;
+import org.apache.ignite.cache.CacheRebalanceMode;
+import org.apache.ignite.configuration.CacheConfiguration;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.events.CacheRebalancingEvent;
+import org.apache.ignite.events.Event;
+import org.apache.ignite.events.EventType;
+import org.apache.ignite.internal.util.typedef.T2;
+import org.apache.ignite.lang.IgniteBiTuple;
+import org.apache.ignite.lang.IgnitePredicate;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.After;
+import org.junit.Test;
+
+import static org.apache.ignite.cache.CacheRebalanceMode.ASYNC;
+import static org.apache.ignite.cache.CacheRebalanceMode.SYNC;
+import static org.apache.ignite.events.EventType.EVT_CACHE_REBALANCE_STOPPED;
+
+/**
+ * Tests that caches with rebalance mode equals to SYNC are rebalanced in the first place.
+ */
+public class GridCacheRebalanceOrderTest extends GridCommonAbstractTest {
+ /** Rebalance timeout. */
+ private static final long REBALANCE_TIMEOUT = 5_000;
+
+ /** Flag indicates that local listener should be used to track rebalance events. */
+ private boolean trackRebalanceEvts;
+
+ /** Caches rebalance events. */
+ private final List<CacheRebalancingEvent> evtsList = Collections.synchronizedList(new ArrayList<>());
+
+ /** {@inheritDoc} */
+ @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception {
+ IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName);
+
+ if (trackRebalanceEvts) {
+ Map<IgnitePredicate<? extends Event>, int[]> listeners = new HashMap<>();
+
+ listeners.put(new IgnitePredicate<CacheRebalancingEvent>() {
+ @Override public boolean apply(CacheRebalancingEvent evt) {
+ evtsList.add(evt);
+
+ return true;
+ }
+ }, new int[] {EVT_CACHE_REBALANCE_STOPPED});
+
+ cfg.setLocalEventListeners(listeners);
+
+ cfg.setIncludeEventTypes(EventType.EVTS_ALL);
+ }
+
+ return cfg;
+ }
+
+ /**
+ * Stops all nodes after test.
+ */
+ @After
+ public void testCleanup() {
+ stopAllGrids();
+ }
+
+ /**
+ * Tests that caches with rebalance mode equals to SYNC are rebalanced in the first place.
+ *
+ * @throws Exception If failed.
+ */
+ @Test
+ public void testRebalanceOrderBasedOnCacheRebalanceMode() throws Exception {
+ Ignite g = startGrid(0);
+
+ // Fix the expected order of rebalance.
+ List<IgniteBiTuple<Integer, CacheRebalanceMode>> order = new ArrayList<>();
+ order.add(new T2<>(0, SYNC));
+ order.add(new T2<>(0, SYNC));
+ order.add(new T2<>(0, ASYNC));
+ order.add(new T2<>(0, ASYNC));
+ order.add(new T2<>(1, SYNC));
+ order.add(new T2<>(1, SYNC));
+ order.add(new T2<>(1, ASYNC));
+ order.add(new T2<>(1, ASYNC));
+
+ // Prepare caches with different rebalance mode and order.
+ List<IgniteCache<Integer, Integer>> caches = new ArrayList<>();
+ for (int i = order.size() - 1; i >= 0; i--) {
+ int rebalanceOrder = order.get(i).get1();
+
+ CacheRebalanceMode rebalanceMode = order.get(i).get2();
+
+ caches.add(g.getOrCreateCache(getCacheConfiguration(
+ "cache-" + i + "-order-" + rebalanceOrder + "-mode-" + rebalanceMode,
+ rebalanceOrder,
+ rebalanceMode)));
+ }
+
+ // Fill values.
+ for (IgniteCache<Integer, Integer> c : caches)
+ c.put(12, 21);
+
+ trackRebalanceEvts = true;
+
+ Ignite g1 = startGrid(1);
+
+ // Wait for all rebalance futures.
+ for (IgniteCache<Integer, Integer> c : caches)
+ grid(1).context().cache().internalCache(c.getName()).preloader().syncFuture().get(REBALANCE_TIMEOUT);
Review comment:
`syncFuture` is for preloading only on a node startup, let's use `awaitPartitionMapExchange`
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] Mmuzaf commented on a change in pull request #7457:
IGNITE-12705 SYNC caches are rebalanced in the first place.
Posted by GitBox <gi...@apache.org>.
Mmuzaf commented on a change in pull request #7457: IGNITE-12705 SYNC caches are rebalanced in the first place.
URL: https://github.com/apache/ignite/pull/7457#discussion_r384390560
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCachePartitionExchangeManager.java
##########
@@ -3846,4 +3849,66 @@ void trim() {
activeObjects.clear();
}
}
+
+ /**
+ * Represents a cache rebalance order that takes into account both values: rebalance order itself and rebalance mode.
+ * It is assumed SYNC caches should be rebalanced in the first place.
+ */
+ private static class CacheRebalanceOrder implements Comparable<CacheRebalanceOrder> {
Review comment:
I think it can be replaced with a comparator which is passing to the constructor for TreeMap. This will require fewer changes and reduce the boilerplate equals\hashcode.
WDYT?
----------------------------------------------------------------
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
With regards,
Apache Git Services