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/11/08 12:47:21 UTC

[GitHub] [ignite] konstantins304 opened a new pull request #8439: IGNITE-13572 Don't skip filtering for caches with zero backups.

konstantins304 opened a new pull request #8439:
URL: https://github.com/apache/ignite/pull/8439


   Thank you for submitting the pull request to the Apache Ignite.
   
   In order to streamline the review of the contribution 
   we ask you to ensure the following steps have been taken:
   
   ### The Contribution Checklist
   - [ ] There is a single JIRA ticket related to the pull request. 
   - [ ] The web-link to the pull request is attached to the JIRA ticket.
   - [ ] The JIRA ticket has the _Patch Available_ state.
   - [ ] The pull request body describes changes that have been made. 
   The description explains _WHAT_ and _WHY_ was made instead of _HOW_.
   - [ ] The pull request title is treated as the final commit message. 
   The following pattern must be used: `IGNITE-XXXX Change summary` where `XXXX` - number of JIRA issue.
   - [ ] A reviewer has been mentioned through the JIRA comments 
   (see [the Maintainers list](https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute#HowtoContribute-ReviewProcessandMaintainers)) 
   - [ ] The pull request has been checked by the Teamcity Bot and 
   the `green visa` attached to the JIRA ticket (see [TC.Bot: Check PR](https://mtcga.gridgain.com/prs.html))
   
   ### Notes
   - [How to Contribute](https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute)
   - [Coding abbreviation rules](https://cwiki.apache.org/confluence/display/IGNITE/Abbreviation+Rules)
   - [Coding Guidelines](https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines)
   - [Apache Ignite Teamcity Bot](https://cwiki.apache.org/confluence/display/IGNITE/Apache+Ignite+Teamcity+Bot)
   
   If you need any help, please email dev@ignite.apache.org or ask anу advice on http://asf.slack.com _#ignite_ channel.
   


----------------------------------------------------------------
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] ivandasch commented on a change in pull request #8439: IGNITE-13572 Don't skip filtering for caches with zero backups.

Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #8439:
URL: https://github.com/apache/ignite/pull/8439#discussion_r529419669



##########
File path: modules/indexing/src/test/java/org/apache/ignite/internal/processors/cache/index/SqlPartitionEvictionTest.java
##########
@@ -0,0 +1,210 @@
+/*
+ * 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.index;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Random;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ThreadLocalRandom;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.IgniteCache;
+import org.apache.ignite.IgniteCheckedException;
+import org.apache.ignite.IgniteDataStreamer;
+import org.apache.ignite.binary.BinaryObject;
+import org.apache.ignite.cache.CacheAtomicityMode;
+import org.apache.ignite.cache.CacheMode;
+import org.apache.ignite.cache.CacheWriteSynchronizationMode;
+import org.apache.ignite.cache.QueryEntity;
+import org.apache.ignite.cache.query.SqlFieldsQuery;
+import org.apache.ignite.configuration.CacheConfiguration;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.processors.cache.GridCacheContext;
+import org.apache.ignite.internal.processors.cache.persistence.CacheDataRow;
+import org.apache.ignite.internal.processors.query.GridQueryProcessor;
+import org.apache.ignite.internal.processors.query.GridQueryTypeDescriptor;
+import org.apache.ignite.internal.processors.query.h2.IgniteH2Indexing;
+import org.apache.ignite.internal.util.typedef.G;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+/** */
+@RunWith(Parameterized.class)
+public class SqlPartitionEvictionTest extends GridCommonAbstractTest {
+    /** */
+    private static final String POI_CACHE_NAME = "POI_CACHE";
+
+    /** */
+    private static final String POI_SCHEMA_NAME = "DOMAIN";
+
+    /** */
+    private static final String POI_TABLE_NAME = "POI";
+
+    /** */
+    private static final String POI_CLASS_NAME = "PointOfInterest";
+
+    /** */
+    private static final String ID_FIELD_NAME = "id";
+
+    /** */
+    private static final String NAME_FIELD_NAME = "name";
+
+    /** */
+    private static final String LATITUDE_FIELD_NAME = "latitude";
+
+    /** */
+    private static final String LONGITUDE_FIELD_NAME = "longitude";
+
+    /** */
+    private static final int NUM_ENTITIES = 1_000;
+
+    /** Test parameters. */
+    @Parameterized.Parameters(name = "backups_count={0}")
+    public static Iterable<Object[]> params() {
+        return Arrays.asList(
+                new Object[] { 0 },
+                new Object[] { 1 },
+                new Object[] { 2 }
+            );
+    }
+
+    /** */
+    @Parameterized.Parameter
+    public int backupsCount;
+
+    private static CountDownLatch latch;
+
+    /** {@inheritDoc} */
+    @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception {
+        IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName);
+
+        cfg.setCacheConfiguration(new CacheConfiguration<>(POI_CACHE_NAME)
+            .setAtomicityMode(CacheAtomicityMode.ATOMIC)
+            .setSqlSchema("DOMAIN")
+            .setQueryEntities(Collections.singletonList(queryEntity()))
+            .setWriteSynchronizationMode(CacheWriteSynchronizationMode.FULL_SYNC)
+            .setCacheMode(CacheMode.PARTITIONED)
+            .setBackups(backupsCount)
+        );
+
+        cfg.setActiveOnStart(true);
+
+        return cfg;
+    }
+
+    @Override protected void beforeTest() throws Exception {
+        latch = new CountDownLatch(1);
+
+        super.beforeTest();
+    }
+
+    /** {@inheritDoc} */
+    @Override protected void afterTest() throws Exception {
+        stopAllGrids(true);
+
+        super.afterTest();
+    }
+
+    @Test

Review comment:
       Missing 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



[GitHub] [ignite] ivandasch commented on a change in pull request #8439: IGNITE-13572 Don't skip filtering for caches with zero backups.

Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #8439:
URL: https://github.com/apache/ignite/pull/8439#discussion_r525842284



##########
File path: modules/indexing/src/test/java/org/apache/ignite/internal/processors/cache/index/SqlPartitionEvictionTest.java
##########
@@ -0,0 +1,196 @@
+/*
+ * 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.index;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Random;
+import java.util.concurrent.ThreadLocalRandom;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.IgniteCache;
+import org.apache.ignite.IgniteCheckedException;
+import org.apache.ignite.IgniteDataStreamer;
+import org.apache.ignite.binary.BinaryObject;
+import org.apache.ignite.cache.CacheAtomicityMode;
+import org.apache.ignite.cache.CacheMode;
+import org.apache.ignite.cache.CacheWriteSynchronizationMode;
+import org.apache.ignite.cache.QueryEntity;
+import org.apache.ignite.cache.query.SqlFieldsQuery;
+import org.apache.ignite.configuration.CacheConfiguration;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.processors.cache.GridCacheContext;
+import org.apache.ignite.internal.processors.cache.persistence.CacheDataRow;
+import org.apache.ignite.internal.processors.query.GridQueryProcessor;
+import org.apache.ignite.internal.processors.query.GridQueryTypeDescriptor;
+import org.apache.ignite.internal.processors.query.h2.IgniteH2Indexing;
+import org.apache.ignite.internal.util.typedef.G;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+/** */
+@RunWith(Parameterized.class)
+public class SqlPartitionEvictionTest extends GridCommonAbstractTest {
+    /** */
+    private static final String POI_CACHE_NAME = "POI_CACHE";
+
+    /** */
+    private static final String POI_SCHEMA_NAME = "DOMAIN";
+
+    /** */
+    private static final String POI_TABLE_NAME = "POI";
+
+    /** */
+    private static final String POI_CLASS_NAME = "PointOfInterest";
+
+    /** */
+    private static final String ID_FIELD_NAME = "id";
+
+    /** */
+    private static final String NAME_FIELD_NAME = "name";
+
+    /** */
+    private static final String LATITUDE_FIELD_NAME = "latitude";
+
+    /** */
+    private static final String LONGITUDE_FIELD_NAME = "longitude";
+
+    /** */
+    private static final int NUM_ENTITIES = 1_000;
+
+    /** Test parameters. */
+    @Parameterized.Parameters(name = "backups_count={0}")
+    public static Iterable<Object[]> params() {
+        return Arrays.asList(
+                new Object[] { 0 },
+                new Object[] { 1 },
+                new Object[] { 2 }
+            );
+    }
+
+    /** */
+    @Parameterized.Parameter
+    public int backupsCount;
+
+    /** {@inheritDoc} */
+    @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception {
+        IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName);
+
+        cfg.setCacheConfiguration(new CacheConfiguration<>(POI_CACHE_NAME)
+            .setAtomicityMode(CacheAtomicityMode.ATOMIC)
+            .setSqlSchema("DOMAIN")
+            .setQueryEntities(Collections.singletonList(queryEntity()))
+            .setWriteSynchronizationMode(CacheWriteSynchronizationMode.FULL_SYNC)
+            .setCacheMode(CacheMode.PARTITIONED)
+            .setBackups(backupsCount)
+        );
+
+        cfg.setActiveOnStart(true);
+
+        return cfg;
+    }
+
+    /** {@inheritDoc} */
+    @Override protected void afterTest() throws Exception {
+        stopAllGrids(true);
+
+        super.afterTest();
+    }
+
+    @Test
+    public void testSqlConsistencyOnEviction() throws Exception {
+        ignitionStart(0);

Review comment:
       I have few additional requests:
   1. If you use backup counter equals to 2, 3 grids is not enough to start eviction. At least 4 grids (3 initial and 1 started
   after loading) are required.
   2. I suppose that we can add latch in remove method of blocking indexing and await for actual eviction start. This measure can assure us that actual eviction begins.
    




----------------------------------------------------------------
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] ivandasch edited a comment on pull request #8439: IGNITE-13572 Don't skip filtering for caches with zero backups.

Posted by GitBox <gi...@apache.org>.
ivandasch edited a comment on pull request #8439:
URL: https://github.com/apache/ignite/pull/8439#issuecomment-723580984


   Hi, you have forgot to include your test to any suite.
   
   Also, please make changes in `org.apache.ignite.internal.processors.cache.index.DynamicEnableIndexingConcurrentSelfTest`.
   Because of this bug, usual `awaitPartitionMapExchange()` was changed to `awaitPartitionMapExchange(true, true, null)`.
   Please, return back simple variant and remove `TODO`


----------------------------------------------------------------
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] ivandasch commented on pull request #8439: IGNITE-13572 Don't skip filtering for caches with zero backups.

Posted by GitBox <gi...@apache.org>.
ivandasch commented on pull request #8439:
URL: https://github.com/apache/ignite/pull/8439#issuecomment-723580984


   Hi, you have forgot to include your test to any suite. Please dont forget to change some tests related to issue (awat for pme and eviction --- just wait for pme)


----------------------------------------------------------------
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] ivandasch commented on a change in pull request #8439: IGNITE-13572 Don't skip filtering for caches with zero backups.

Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #8439:
URL: https://github.com/apache/ignite/pull/8439#discussion_r529418951



##########
File path: modules/indexing/src/test/java/org/apache/ignite/internal/processors/cache/index/SqlPartitionEvictionTest.java
##########
@@ -0,0 +1,210 @@
+/*
+ * 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.index;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Random;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ThreadLocalRandom;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.IgniteCache;
+import org.apache.ignite.IgniteCheckedException;
+import org.apache.ignite.IgniteDataStreamer;
+import org.apache.ignite.binary.BinaryObject;
+import org.apache.ignite.cache.CacheAtomicityMode;
+import org.apache.ignite.cache.CacheMode;
+import org.apache.ignite.cache.CacheWriteSynchronizationMode;
+import org.apache.ignite.cache.QueryEntity;
+import org.apache.ignite.cache.query.SqlFieldsQuery;
+import org.apache.ignite.configuration.CacheConfiguration;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.processors.cache.GridCacheContext;
+import org.apache.ignite.internal.processors.cache.persistence.CacheDataRow;
+import org.apache.ignite.internal.processors.query.GridQueryProcessor;
+import org.apache.ignite.internal.processors.query.GridQueryTypeDescriptor;
+import org.apache.ignite.internal.processors.query.h2.IgniteH2Indexing;
+import org.apache.ignite.internal.util.typedef.G;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+/** */
+@RunWith(Parameterized.class)
+public class SqlPartitionEvictionTest extends GridCommonAbstractTest {
+    /** */
+    private static final String POI_CACHE_NAME = "POI_CACHE";
+
+    /** */
+    private static final String POI_SCHEMA_NAME = "DOMAIN";
+
+    /** */
+    private static final String POI_TABLE_NAME = "POI";
+
+    /** */
+    private static final String POI_CLASS_NAME = "PointOfInterest";
+
+    /** */
+    private static final String ID_FIELD_NAME = "id";
+
+    /** */
+    private static final String NAME_FIELD_NAME = "name";
+
+    /** */
+    private static final String LATITUDE_FIELD_NAME = "latitude";
+
+    /** */
+    private static final String LONGITUDE_FIELD_NAME = "longitude";
+
+    /** */
+    private static final int NUM_ENTITIES = 1_000;
+
+    /** Test parameters. */
+    @Parameterized.Parameters(name = "backups_count={0}")
+    public static Iterable<Object[]> params() {
+        return Arrays.asList(
+                new Object[] { 0 },
+                new Object[] { 1 },
+                new Object[] { 2 }
+            );
+    }
+
+    /** */
+    @Parameterized.Parameter
+    public int backupsCount;
+

Review comment:
       Missing 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



[GitHub] [ignite] ivandasch commented on a change in pull request #8439: IGNITE-13572 Don't skip filtering for caches with zero backups.

Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #8439:
URL: https://github.com/apache/ignite/pull/8439#discussion_r532645690



##########
File path: modules/indexing/src/test/java/org/apache/ignite/internal/processors/cache/index/SqlPartitionEvictionTest.java
##########
@@ -0,0 +1,209 @@
+/*
+ * 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.index;
+
+import static java.util.concurrent.TimeUnit.SECONDS;
+
+import org.apache.ignite.Ignite;
+import org.apache.ignite.IgniteCache;
+import org.apache.ignite.IgniteCheckedException;
+import org.apache.ignite.IgniteDataStreamer;
+import org.apache.ignite.binary.BinaryObject;
+import org.apache.ignite.cache.CacheAtomicityMode;
+import org.apache.ignite.cache.CacheMode;
+import org.apache.ignite.cache.CacheWriteSynchronizationMode;
+import org.apache.ignite.cache.QueryEntity;
+import org.apache.ignite.cache.query.SqlFieldsQuery;
+import org.apache.ignite.configuration.CacheConfiguration;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.processors.cache.GridCacheContext;
+import org.apache.ignite.internal.processors.cache.persistence.CacheDataRow;
+import org.apache.ignite.internal.processors.query.GridQueryProcessor;
+import org.apache.ignite.internal.processors.query.GridQueryTypeDescriptor;
+import org.apache.ignite.internal.processors.query.h2.IgniteH2Indexing;
+import org.apache.ignite.internal.util.typedef.G;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Random;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ThreadLocalRandom;
+
+/** */
+@RunWith(Parameterized.class)
+public class SqlPartitionEvictionTest extends GridCommonAbstractTest {
+    /** */
+    private static final String POI_CACHE_NAME = "POI_CACHE";
+
+    /** */
+    private static final String POI_SCHEMA_NAME = "DOMAIN";
+
+    /** */
+    private static final String POI_TABLE_NAME = "POI";
+
+    /** */
+    private static final String POI_CLASS_NAME = "PointOfInterest";
+
+    /** */
+    private static final String ID_FIELD_NAME = "id";
+
+    /** */
+    private static final String NAME_FIELD_NAME = "name";
+
+    /** */
+    private static final String LATITUDE_FIELD_NAME = "latitude";
+
+    /** */
+    private static final String LONGITUDE_FIELD_NAME = "longitude";
+
+    /** */
+    private static final int NUM_ENTITIES = 1_000;
+
+    /** Test parameters. */
+    @Parameterized.Parameters(name = "backups_count={0}")
+    public static Iterable<Object[]> params() {
+        return Arrays.asList(
+                new Object[] { 0 },
+                new Object[] { 1 },
+                new Object[] { 2 }
+            );
+    }
+
+    /**
+     *  Number of partition backups

Review comment:
       Here and below -- point (dot) at the end of sentence is required.




----------------------------------------------------------------
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] zstan commented on a change in pull request #8439: IGNITE-13572 Don't skip filtering for caches with zero backups.

Posted by GitBox <gi...@apache.org>.
zstan commented on a change in pull request #8439:
URL: https://github.com/apache/ignite/pull/8439#discussion_r525835972



##########
File path: modules/indexing/src/test/java/org/apache/ignite/internal/processors/cache/index/SqlPartitionEvictionTest.java
##########
@@ -0,0 +1,196 @@
+/*
+ * 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.index;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Random;
+import java.util.concurrent.ThreadLocalRandom;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.IgniteCache;
+import org.apache.ignite.IgniteCheckedException;
+import org.apache.ignite.IgniteDataStreamer;
+import org.apache.ignite.binary.BinaryObject;
+import org.apache.ignite.cache.CacheAtomicityMode;
+import org.apache.ignite.cache.CacheMode;
+import org.apache.ignite.cache.CacheWriteSynchronizationMode;
+import org.apache.ignite.cache.QueryEntity;
+import org.apache.ignite.cache.query.SqlFieldsQuery;
+import org.apache.ignite.configuration.CacheConfiguration;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.processors.cache.GridCacheContext;
+import org.apache.ignite.internal.processors.cache.persistence.CacheDataRow;
+import org.apache.ignite.internal.processors.query.GridQueryProcessor;
+import org.apache.ignite.internal.processors.query.GridQueryTypeDescriptor;
+import org.apache.ignite.internal.processors.query.h2.IgniteH2Indexing;
+import org.apache.ignite.internal.util.typedef.G;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+/** */
+@RunWith(Parameterized.class)
+public class SqlPartitionEvictionTest extends GridCommonAbstractTest {
+    /** */
+    private static final String POI_CACHE_NAME = "POI_CACHE";
+
+    /** */
+    private static final String POI_SCHEMA_NAME = "DOMAIN";
+
+    /** */
+    private static final String POI_TABLE_NAME = "POI";
+
+    /** */
+    private static final String POI_CLASS_NAME = "PointOfInterest";
+
+    /** */
+    private static final String ID_FIELD_NAME = "id";
+
+    /** */
+    private static final String NAME_FIELD_NAME = "name";
+
+    /** */
+    private static final String LATITUDE_FIELD_NAME = "latitude";
+
+    /** */
+    private static final String LONGITUDE_FIELD_NAME = "longitude";
+
+    /** */
+    private static final int NUM_ENTITIES = 1_000;
+
+    /** Test parameters. */
+    @Parameterized.Parameters(name = "backups_count={0}")
+    public static Iterable<Object[]> params() {
+        return Arrays.asList(
+                new Object[] { 0 },
+                new Object[] { 1 },
+                new Object[] { 2 }
+            );
+    }
+
+    /** */
+    @Parameterized.Parameter
+    public int backupsCount;
+
+    /** {@inheritDoc} */
+    @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception {
+        IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName);
+
+        cfg.setCacheConfiguration(new CacheConfiguration<>(POI_CACHE_NAME)
+            .setAtomicityMode(CacheAtomicityMode.ATOMIC)
+            .setSqlSchema("DOMAIN")
+            .setQueryEntities(Collections.singletonList(queryEntity()))
+            .setWriteSynchronizationMode(CacheWriteSynchronizationMode.FULL_SYNC)
+            .setCacheMode(CacheMode.PARTITIONED)
+            .setBackups(backupsCount)
+        );
+
+        cfg.setActiveOnStart(true);
+
+        return cfg;
+    }
+
+    /** {@inheritDoc} */
+    @Override protected void afterTest() throws Exception {
+        stopAllGrids(true);
+
+        super.afterTest();
+    }
+
+    @Test
+    public void testSqlConsistencyOnEviction() throws Exception {
+        ignitionStart(0);
+        IgniteEx ig = ignitionStart(1);
+
+        loadData(ig, 0, NUM_ENTITIES);
+
+        ignitionStart(2);
+
+        awaitPartitionMapExchange();
+
+        for (Ignite g: G.allGrids())
+            assertEquals(NUM_ENTITIES, query(g, "SELECT * FROM " + POI_TABLE_NAME).size());
+

Review comment:
       redundant line




----------------------------------------------------------------
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] ivandasch commented on a change in pull request #8439: IGNITE-13572 Don't skip filtering for caches with zero backups.

Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #8439:
URL: https://github.com/apache/ignite/pull/8439#discussion_r519585793



##########
File path: modules/indexing/src/test/java/org/apache/ignite/internal/processors/cache/index/SqlPartitionEvictionTest.java
##########
@@ -0,0 +1,195 @@
+/*
+ * 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.index;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Random;
+import java.util.concurrent.ThreadLocalRandom;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.IgniteCache;
+import org.apache.ignite.IgniteCheckedException;
+import org.apache.ignite.IgniteDataStreamer;
+import org.apache.ignite.binary.BinaryObject;
+import org.apache.ignite.cache.CacheAtomicityMode;
+import org.apache.ignite.cache.CacheMode;
+import org.apache.ignite.cache.CacheWriteSynchronizationMode;
+import org.apache.ignite.cache.QueryEntity;
+import org.apache.ignite.cache.query.SqlFieldsQuery;
+import org.apache.ignite.configuration.CacheConfiguration;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.processors.cache.GridCacheContext;
+import org.apache.ignite.internal.processors.cache.persistence.CacheDataRow;
+import org.apache.ignite.internal.processors.query.GridQueryProcessor;
+import org.apache.ignite.internal.processors.query.GridQueryTypeDescriptor;
+import org.apache.ignite.internal.processors.query.h2.IgniteH2Indexing;
+import org.apache.ignite.internal.util.typedef.G;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+/** */
+@RunWith(Parameterized.class)
+public class SqlPartitionEvictionTest extends GridCommonAbstractTest {
+    /** */
+    private static final String POI_CACHE_NAME = "POI_CACHE";
+
+    /** */
+    private static final String POI_SCHEMA_NAME = "DOMAIN";
+
+    /** */
+    private static final String POI_TABLE_NAME = "POI";
+
+    /** */
+    private static final String POI_CLASS_NAME = "PointOfInterest";
+
+    /** */
+    private static final String ID_FIELD_NAME = "id";
+
+    /** */
+    private static final String NAME_FIELD_NAME = "name";
+
+    /** */
+    private static final String LATITUDE_FIELD_NAME = "latitude";
+
+    /** */
+    private static final String LONGITUDE_FIELD_NAME = "longitude";
+
+    /** */
+    private static final int NUM_ENTITIES = 1_000;
+
+    /** Test parameters. */
+    @Parameterized.Parameters(name = "await_eviction={0}")
+    public static Iterable<Object[]> params() {
+        return Arrays.asList(
+            new Object[] { false },
+            new Object[] { true });
+    }
+
+    /** */
+    @Parameterized.Parameter
+    public boolean awaitEviction;

Review comment:
       I suppose that it's required to remove awaiting for eviction param/
   
   But, I suppose, that we should add parametrization for backups count. 
   Check for zero backups and 1 or 2 backups.




----------------------------------------------------------------
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] asfgit closed pull request #8439: IGNITE-13572 Don't skip filtering for caches with zero backups.

Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #8439:
URL: https://github.com/apache/ignite/pull/8439


   


----------------------------------------------------------------
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] ivandasch commented on a change in pull request #8439: IGNITE-13572 Don't skip filtering for caches with zero backups.

Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #8439:
URL: https://github.com/apache/ignite/pull/8439#discussion_r527719431



##########
File path: modules/indexing/src/test/java/org/apache/ignite/internal/processors/cache/index/SqlPartitionEvictionTest.java
##########
@@ -0,0 +1,196 @@
+/*
+ * 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.index;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Random;
+import java.util.concurrent.ThreadLocalRandom;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.IgniteCache;
+import org.apache.ignite.IgniteCheckedException;
+import org.apache.ignite.IgniteDataStreamer;
+import org.apache.ignite.binary.BinaryObject;
+import org.apache.ignite.cache.CacheAtomicityMode;
+import org.apache.ignite.cache.CacheMode;
+import org.apache.ignite.cache.CacheWriteSynchronizationMode;
+import org.apache.ignite.cache.QueryEntity;
+import org.apache.ignite.cache.query.SqlFieldsQuery;
+import org.apache.ignite.configuration.CacheConfiguration;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.processors.cache.GridCacheContext;
+import org.apache.ignite.internal.processors.cache.persistence.CacheDataRow;
+import org.apache.ignite.internal.processors.query.GridQueryProcessor;
+import org.apache.ignite.internal.processors.query.GridQueryTypeDescriptor;
+import org.apache.ignite.internal.processors.query.h2.IgniteH2Indexing;
+import org.apache.ignite.internal.util.typedef.G;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+/** */
+@RunWith(Parameterized.class)
+public class SqlPartitionEvictionTest extends GridCommonAbstractTest {
+    /** */
+    private static final String POI_CACHE_NAME = "POI_CACHE";
+
+    /** */
+    private static final String POI_SCHEMA_NAME = "DOMAIN";
+
+    /** */
+    private static final String POI_TABLE_NAME = "POI";
+
+    /** */
+    private static final String POI_CLASS_NAME = "PointOfInterest";
+
+    /** */
+    private static final String ID_FIELD_NAME = "id";
+
+    /** */
+    private static final String NAME_FIELD_NAME = "name";
+
+    /** */
+    private static final String LATITUDE_FIELD_NAME = "latitude";
+
+    /** */
+    private static final String LONGITUDE_FIELD_NAME = "longitude";
+
+    /** */
+    private static final int NUM_ENTITIES = 1_000;
+
+    /** Test parameters. */
+    @Parameterized.Parameters(name = "backups_count={0}")
+    public static Iterable<Object[]> params() {
+        return Arrays.asList(
+                new Object[] { 0 },
+                new Object[] { 1 },
+                new Object[] { 2 }
+            );
+    }
+
+    /** */
+    @Parameterized.Parameter
+    public int backupsCount;
+
+    /** {@inheritDoc} */
+    @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception {
+        IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName);
+
+        cfg.setCacheConfiguration(new CacheConfiguration<>(POI_CACHE_NAME)
+            .setAtomicityMode(CacheAtomicityMode.ATOMIC)
+            .setSqlSchema("DOMAIN")
+            .setQueryEntities(Collections.singletonList(queryEntity()))
+            .setWriteSynchronizationMode(CacheWriteSynchronizationMode.FULL_SYNC)
+            .setCacheMode(CacheMode.PARTITIONED)
+            .setBackups(backupsCount)
+        );
+
+        cfg.setActiveOnStart(true);
+
+        return cfg;
+    }
+
+    /** {@inheritDoc} */
+    @Override protected void afterTest() throws Exception {
+        stopAllGrids(true);
+
+        super.afterTest();
+    }
+
+    @Test
+    public void testSqlConsistencyOnEviction() throws Exception {
+        ignitionStart(0);

Review comment:
       Yep, you understood it correctly.




----------------------------------------------------------------
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] ivandasch commented on a change in pull request #8439: IGNITE-13572 Don't skip filtering for caches with zero backups.

Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #8439:
URL: https://github.com/apache/ignite/pull/8439#discussion_r519585793



##########
File path: modules/indexing/src/test/java/org/apache/ignite/internal/processors/cache/index/SqlPartitionEvictionTest.java
##########
@@ -0,0 +1,195 @@
+/*
+ * 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.index;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Random;
+import java.util.concurrent.ThreadLocalRandom;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.IgniteCache;
+import org.apache.ignite.IgniteCheckedException;
+import org.apache.ignite.IgniteDataStreamer;
+import org.apache.ignite.binary.BinaryObject;
+import org.apache.ignite.cache.CacheAtomicityMode;
+import org.apache.ignite.cache.CacheMode;
+import org.apache.ignite.cache.CacheWriteSynchronizationMode;
+import org.apache.ignite.cache.QueryEntity;
+import org.apache.ignite.cache.query.SqlFieldsQuery;
+import org.apache.ignite.configuration.CacheConfiguration;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.processors.cache.GridCacheContext;
+import org.apache.ignite.internal.processors.cache.persistence.CacheDataRow;
+import org.apache.ignite.internal.processors.query.GridQueryProcessor;
+import org.apache.ignite.internal.processors.query.GridQueryTypeDescriptor;
+import org.apache.ignite.internal.processors.query.h2.IgniteH2Indexing;
+import org.apache.ignite.internal.util.typedef.G;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+/** */
+@RunWith(Parameterized.class)
+public class SqlPartitionEvictionTest extends GridCommonAbstractTest {
+    /** */
+    private static final String POI_CACHE_NAME = "POI_CACHE";
+
+    /** */
+    private static final String POI_SCHEMA_NAME = "DOMAIN";
+
+    /** */
+    private static final String POI_TABLE_NAME = "POI";
+
+    /** */
+    private static final String POI_CLASS_NAME = "PointOfInterest";
+
+    /** */
+    private static final String ID_FIELD_NAME = "id";
+
+    /** */
+    private static final String NAME_FIELD_NAME = "name";
+
+    /** */
+    private static final String LATITUDE_FIELD_NAME = "latitude";
+
+    /** */
+    private static final String LONGITUDE_FIELD_NAME = "longitude";
+
+    /** */
+    private static final int NUM_ENTITIES = 1_000;
+
+    /** Test parameters. */
+    @Parameterized.Parameters(name = "await_eviction={0}")
+    public static Iterable<Object[]> params() {
+        return Arrays.asList(
+            new Object[] { false },
+            new Object[] { true });
+    }
+
+    /** */
+    @Parameterized.Parameter
+    public boolean awaitEviction;

Review comment:
       I suppose that it's required to remove awaiting for eviction param.
   
   But, I suppose, that we should add parametrization for backups count. 
   I suppose that a check for zero backups and 1 or 2 backups is required




----------------------------------------------------------------
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] ivandasch commented on pull request #8439: IGNITE-13572 Don't skip filtering for caches with zero backups.

Posted by GitBox <gi...@apache.org>.
ivandasch commented on pull request #8439:
URL: https://github.com/apache/ignite/pull/8439#issuecomment-723793189


   Also, please make changes in `org.apache.ignite.internal.processors.cache.index.DynamicEnableIndexingConcurrentSelfTest`.
   Because of this bug, usual `awaitPartitionMapExchange()` was changed to `awaitPartitionMapExchange(true, true, null)`.
   Please, return back simple variant and remove `TODO`


----------------------------------------------------------------
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] ivandasch commented on a change in pull request #8439: IGNITE-13572 Don't skip filtering for caches with zero backups.

Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #8439:
URL: https://github.com/apache/ignite/pull/8439#discussion_r532639743



##########
File path: modules/indexing/src/test/java/org/apache/ignite/internal/processors/cache/index/SqlPartitionEvictionTest.java
##########
@@ -0,0 +1,210 @@
+/*
+ * 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.index;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Random;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ThreadLocalRandom;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.IgniteCache;
+import org.apache.ignite.IgniteCheckedException;
+import org.apache.ignite.IgniteDataStreamer;
+import org.apache.ignite.binary.BinaryObject;
+import org.apache.ignite.cache.CacheAtomicityMode;
+import org.apache.ignite.cache.CacheMode;
+import org.apache.ignite.cache.CacheWriteSynchronizationMode;
+import org.apache.ignite.cache.QueryEntity;
+import org.apache.ignite.cache.query.SqlFieldsQuery;
+import org.apache.ignite.configuration.CacheConfiguration;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.processors.cache.GridCacheContext;
+import org.apache.ignite.internal.processors.cache.persistence.CacheDataRow;
+import org.apache.ignite.internal.processors.query.GridQueryProcessor;
+import org.apache.ignite.internal.processors.query.GridQueryTypeDescriptor;
+import org.apache.ignite.internal.processors.query.h2.IgniteH2Indexing;
+import org.apache.ignite.internal.util.typedef.G;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+/** */
+@RunWith(Parameterized.class)
+public class SqlPartitionEvictionTest extends GridCommonAbstractTest {
+    /** */
+    private static final String POI_CACHE_NAME = "POI_CACHE";
+
+    /** */
+    private static final String POI_SCHEMA_NAME = "DOMAIN";
+
+    /** */
+    private static final String POI_TABLE_NAME = "POI";
+
+    /** */
+    private static final String POI_CLASS_NAME = "PointOfInterest";
+
+    /** */
+    private static final String ID_FIELD_NAME = "id";
+
+    /** */
+    private static final String NAME_FIELD_NAME = "name";
+
+    /** */
+    private static final String LATITUDE_FIELD_NAME = "latitude";
+
+    /** */
+    private static final String LONGITUDE_FIELD_NAME = "longitude";
+
+    /** */
+    private static final int NUM_ENTITIES = 1_000;
+
+    /** Test parameters. */
+    @Parameterized.Parameters(name = "backups_count={0}")
+    public static Iterable<Object[]> params() {
+        return Arrays.asList(
+                new Object[] { 0 },
+                new Object[] { 1 },
+                new Object[] { 2 }
+            );
+    }
+
+    /** */
+    @Parameterized.Parameter
+    public int backupsCount;
+

Review comment:
       Opps, sorry, I missed it. Ok, there is no variant except this one.




----------------------------------------------------------------
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] ivandasch removed a comment on pull request #8439: IGNITE-13572 Don't skip filtering for caches with zero backups.

Posted by GitBox <gi...@apache.org>.
ivandasch removed a comment on pull request #8439:
URL: https://github.com/apache/ignite/pull/8439#issuecomment-723793189


   Also, please make changes in `org.apache.ignite.internal.processors.cache.index.DynamicEnableIndexingConcurrentSelfTest`.
   Because of this bug, usual `awaitPartitionMapExchange()` was changed to `awaitPartitionMapExchange(true, true, null)`.
   Please, return back simple variant and remove `TODO`


----------------------------------------------------------------
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] ivandasch commented on a change in pull request #8439: IGNITE-13572 Don't skip filtering for caches with zero backups.

Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #8439:
URL: https://github.com/apache/ignite/pull/8439#discussion_r529423896



##########
File path: modules/indexing/src/test/java/org/apache/ignite/internal/processors/cache/index/SqlPartitionEvictionTest.java
##########
@@ -0,0 +1,210 @@
+/*
+ * 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.index;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Random;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ThreadLocalRandom;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.IgniteCache;
+import org.apache.ignite.IgniteCheckedException;
+import org.apache.ignite.IgniteDataStreamer;
+import org.apache.ignite.binary.BinaryObject;
+import org.apache.ignite.cache.CacheAtomicityMode;
+import org.apache.ignite.cache.CacheMode;
+import org.apache.ignite.cache.CacheWriteSynchronizationMode;
+import org.apache.ignite.cache.QueryEntity;
+import org.apache.ignite.cache.query.SqlFieldsQuery;
+import org.apache.ignite.configuration.CacheConfiguration;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.processors.cache.GridCacheContext;
+import org.apache.ignite.internal.processors.cache.persistence.CacheDataRow;
+import org.apache.ignite.internal.processors.query.GridQueryProcessor;
+import org.apache.ignite.internal.processors.query.GridQueryTypeDescriptor;
+import org.apache.ignite.internal.processors.query.h2.IgniteH2Indexing;
+import org.apache.ignite.internal.util.typedef.G;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+/** */
+@RunWith(Parameterized.class)
+public class SqlPartitionEvictionTest extends GridCommonAbstractTest {
+    /** */
+    private static final String POI_CACHE_NAME = "POI_CACHE";
+
+    /** */
+    private static final String POI_SCHEMA_NAME = "DOMAIN";
+
+    /** */
+    private static final String POI_TABLE_NAME = "POI";
+
+    /** */
+    private static final String POI_CLASS_NAME = "PointOfInterest";
+
+    /** */
+    private static final String ID_FIELD_NAME = "id";
+
+    /** */
+    private static final String NAME_FIELD_NAME = "name";
+
+    /** */
+    private static final String LATITUDE_FIELD_NAME = "latitude";
+
+    /** */
+    private static final String LONGITUDE_FIELD_NAME = "longitude";
+
+    /** */
+    private static final int NUM_ENTITIES = 1_000;
+
+    /** Test parameters. */
+    @Parameterized.Parameters(name = "backups_count={0}")
+    public static Iterable<Object[]> params() {
+        return Arrays.asList(
+                new Object[] { 0 },
+                new Object[] { 1 },
+                new Object[] { 2 }
+            );
+    }
+
+    /** */
+    @Parameterized.Parameter
+    public int backupsCount;
+
+    private static CountDownLatch latch;
+
+    /** {@inheritDoc} */
+    @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception {
+        IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName);
+
+        cfg.setCacheConfiguration(new CacheConfiguration<>(POI_CACHE_NAME)
+            .setAtomicityMode(CacheAtomicityMode.ATOMIC)
+            .setSqlSchema("DOMAIN")
+            .setQueryEntities(Collections.singletonList(queryEntity()))
+            .setWriteSynchronizationMode(CacheWriteSynchronizationMode.FULL_SYNC)
+            .setCacheMode(CacheMode.PARTITIONED)
+            .setBackups(backupsCount)
+        );
+
+        cfg.setActiveOnStart(true);
+
+        return cfg;
+    }
+
+    @Override protected void beforeTest() throws Exception {
+        latch = new CountDownLatch(1);
+
+        super.beforeTest();
+    }
+
+    /** {@inheritDoc} */
+    @Override protected void afterTest() throws Exception {
+        stopAllGrids(true);
+
+        super.afterTest();
+    }
+
+    @Test
+    public void testSqlConsistencyOnEviction() throws Exception {
+        IgniteEx ig = null;
+        int idx = 0;

Review comment:
       But I'm not inist, just extra empty line as separator is enough (after ig definition)




----------------------------------------------------------------
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] ivandasch commented on pull request #8439: IGNITE-13572 Don't skip filtering for caches with zero backups.

Posted by GitBox <gi...@apache.org>.
ivandasch commented on pull request #8439:
URL: https://github.com/apache/ignite/pull/8439#issuecomment-735823576


   Small advice -- run checkstyle locally, before push.
   ```
    mvn test-compile -Pall-java,licenses,lgpl,checkstyle,examples,all-scala,scala,check-licenses -B -V
   ```


----------------------------------------------------------------
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] ivandasch commented on a change in pull request #8439: IGNITE-13572 Don't skip filtering for caches with zero backups.

Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #8439:
URL: https://github.com/apache/ignite/pull/8439#discussion_r529419255



##########
File path: modules/indexing/src/test/java/org/apache/ignite/internal/processors/cache/index/SqlPartitionEvictionTest.java
##########
@@ -0,0 +1,210 @@
+/*
+ * 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.index;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Random;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ThreadLocalRandom;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.IgniteCache;
+import org.apache.ignite.IgniteCheckedException;
+import org.apache.ignite.IgniteDataStreamer;
+import org.apache.ignite.binary.BinaryObject;
+import org.apache.ignite.cache.CacheAtomicityMode;
+import org.apache.ignite.cache.CacheMode;
+import org.apache.ignite.cache.CacheWriteSynchronizationMode;
+import org.apache.ignite.cache.QueryEntity;
+import org.apache.ignite.cache.query.SqlFieldsQuery;
+import org.apache.ignite.configuration.CacheConfiguration;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.processors.cache.GridCacheContext;
+import org.apache.ignite.internal.processors.cache.persistence.CacheDataRow;
+import org.apache.ignite.internal.processors.query.GridQueryProcessor;
+import org.apache.ignite.internal.processors.query.GridQueryTypeDescriptor;
+import org.apache.ignite.internal.processors.query.h2.IgniteH2Indexing;
+import org.apache.ignite.internal.util.typedef.G;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+/** */
+@RunWith(Parameterized.class)
+public class SqlPartitionEvictionTest extends GridCommonAbstractTest {
+    /** */
+    private static final String POI_CACHE_NAME = "POI_CACHE";
+
+    /** */
+    private static final String POI_SCHEMA_NAME = "DOMAIN";
+
+    /** */
+    private static final String POI_TABLE_NAME = "POI";
+
+    /** */
+    private static final String POI_CLASS_NAME = "PointOfInterest";
+
+    /** */
+    private static final String ID_FIELD_NAME = "id";
+
+    /** */
+    private static final String NAME_FIELD_NAME = "name";
+
+    /** */
+    private static final String LATITUDE_FIELD_NAME = "latitude";
+
+    /** */
+    private static final String LONGITUDE_FIELD_NAME = "longitude";
+
+    /** */
+    private static final int NUM_ENTITIES = 1_000;
+
+    /** Test parameters. */
+    @Parameterized.Parameters(name = "backups_count={0}")
+    public static Iterable<Object[]> params() {
+        return Arrays.asList(
+                new Object[] { 0 },
+                new Object[] { 1 },
+                new Object[] { 2 }
+            );
+    }
+
+    /** */
+    @Parameterized.Parameter
+    public int backupsCount;
+
+    private static CountDownLatch latch;
+
+    /** {@inheritDoc} */
+    @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception {
+        IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName);
+
+        cfg.setCacheConfiguration(new CacheConfiguration<>(POI_CACHE_NAME)
+            .setAtomicityMode(CacheAtomicityMode.ATOMIC)
+            .setSqlSchema("DOMAIN")
+            .setQueryEntities(Collections.singletonList(queryEntity()))
+            .setWriteSynchronizationMode(CacheWriteSynchronizationMode.FULL_SYNC)
+            .setCacheMode(CacheMode.PARTITIONED)
+            .setBackups(backupsCount)
+        );
+
+        cfg.setActiveOnStart(true);
+
+        return cfg;
+    }
+

Review comment:
       Missing {@inheritDoc}




----------------------------------------------------------------
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] ivandasch commented on a change in pull request #8439: IGNITE-13572 Don't skip filtering for caches with zero backups.

Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #8439:
URL: https://github.com/apache/ignite/pull/8439#discussion_r529418951



##########
File path: modules/indexing/src/test/java/org/apache/ignite/internal/processors/cache/index/SqlPartitionEvictionTest.java
##########
@@ -0,0 +1,210 @@
+/*
+ * 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.index;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Random;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ThreadLocalRandom;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.IgniteCache;
+import org.apache.ignite.IgniteCheckedException;
+import org.apache.ignite.IgniteDataStreamer;
+import org.apache.ignite.binary.BinaryObject;
+import org.apache.ignite.cache.CacheAtomicityMode;
+import org.apache.ignite.cache.CacheMode;
+import org.apache.ignite.cache.CacheWriteSynchronizationMode;
+import org.apache.ignite.cache.QueryEntity;
+import org.apache.ignite.cache.query.SqlFieldsQuery;
+import org.apache.ignite.configuration.CacheConfiguration;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.processors.cache.GridCacheContext;
+import org.apache.ignite.internal.processors.cache.persistence.CacheDataRow;
+import org.apache.ignite.internal.processors.query.GridQueryProcessor;
+import org.apache.ignite.internal.processors.query.GridQueryTypeDescriptor;
+import org.apache.ignite.internal.processors.query.h2.IgniteH2Indexing;
+import org.apache.ignite.internal.util.typedef.G;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+/** */
+@RunWith(Parameterized.class)
+public class SqlPartitionEvictionTest extends GridCommonAbstractTest {
+    /** */
+    private static final String POI_CACHE_NAME = "POI_CACHE";
+
+    /** */
+    private static final String POI_SCHEMA_NAME = "DOMAIN";
+
+    /** */
+    private static final String POI_TABLE_NAME = "POI";
+
+    /** */
+    private static final String POI_CLASS_NAME = "PointOfInterest";
+
+    /** */
+    private static final String ID_FIELD_NAME = "id";
+
+    /** */
+    private static final String NAME_FIELD_NAME = "name";
+
+    /** */
+    private static final String LATITUDE_FIELD_NAME = "latitude";
+
+    /** */
+    private static final String LONGITUDE_FIELD_NAME = "longitude";
+
+    /** */
+    private static final int NUM_ENTITIES = 1_000;
+
+    /** Test parameters. */
+    @Parameterized.Parameters(name = "backups_count={0}")
+    public static Iterable<Object[]> params() {
+        return Arrays.asList(
+                new Object[] { 0 },
+                new Object[] { 1 },
+                new Object[] { 2 }
+            );
+    }
+
+    /** */
+    @Parameterized.Parameter
+    public int backupsCount;
+

Review comment:
       Missing javadoc. Also, this variable (`latch`) should not be static. It can be final member (in Junit, runner create test class for every test method run.). Additionally, overriding beforeTest and afterTest not needed (grids are stopped automatically)




----------------------------------------------------------------
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] ivandasch commented on a change in pull request #8439: IGNITE-13572 Don't skip filtering for caches with zero backups.

Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #8439:
URL: https://github.com/apache/ignite/pull/8439#discussion_r529418951



##########
File path: modules/indexing/src/test/java/org/apache/ignite/internal/processors/cache/index/SqlPartitionEvictionTest.java
##########
@@ -0,0 +1,210 @@
+/*
+ * 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.index;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Random;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ThreadLocalRandom;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.IgniteCache;
+import org.apache.ignite.IgniteCheckedException;
+import org.apache.ignite.IgniteDataStreamer;
+import org.apache.ignite.binary.BinaryObject;
+import org.apache.ignite.cache.CacheAtomicityMode;
+import org.apache.ignite.cache.CacheMode;
+import org.apache.ignite.cache.CacheWriteSynchronizationMode;
+import org.apache.ignite.cache.QueryEntity;
+import org.apache.ignite.cache.query.SqlFieldsQuery;
+import org.apache.ignite.configuration.CacheConfiguration;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.processors.cache.GridCacheContext;
+import org.apache.ignite.internal.processors.cache.persistence.CacheDataRow;
+import org.apache.ignite.internal.processors.query.GridQueryProcessor;
+import org.apache.ignite.internal.processors.query.GridQueryTypeDescriptor;
+import org.apache.ignite.internal.processors.query.h2.IgniteH2Indexing;
+import org.apache.ignite.internal.util.typedef.G;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+/** */
+@RunWith(Parameterized.class)
+public class SqlPartitionEvictionTest extends GridCommonAbstractTest {
+    /** */
+    private static final String POI_CACHE_NAME = "POI_CACHE";
+
+    /** */
+    private static final String POI_SCHEMA_NAME = "DOMAIN";
+
+    /** */
+    private static final String POI_TABLE_NAME = "POI";
+
+    /** */
+    private static final String POI_CLASS_NAME = "PointOfInterest";
+
+    /** */
+    private static final String ID_FIELD_NAME = "id";
+
+    /** */
+    private static final String NAME_FIELD_NAME = "name";
+
+    /** */
+    private static final String LATITUDE_FIELD_NAME = "latitude";
+
+    /** */
+    private static final String LONGITUDE_FIELD_NAME = "longitude";
+
+    /** */
+    private static final int NUM_ENTITIES = 1_000;
+
+    /** Test parameters. */
+    @Parameterized.Parameters(name = "backups_count={0}")
+    public static Iterable<Object[]> params() {
+        return Arrays.asList(
+                new Object[] { 0 },
+                new Object[] { 1 },
+                new Object[] { 2 }
+            );
+    }
+
+    /** */
+    @Parameterized.Parameter
+    public int backupsCount;
+

Review comment:
       Missing javadoc. Also, this variable (`latch`) should not be static. It can be final member of class (in Junit, runner create test class for every test method run.)

##########
File path: modules/indexing/src/test/java/org/apache/ignite/internal/processors/cache/index/SqlPartitionEvictionTest.java
##########
@@ -0,0 +1,210 @@
+/*
+ * 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.index;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Random;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ThreadLocalRandom;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.IgniteCache;
+import org.apache.ignite.IgniteCheckedException;
+import org.apache.ignite.IgniteDataStreamer;
+import org.apache.ignite.binary.BinaryObject;
+import org.apache.ignite.cache.CacheAtomicityMode;
+import org.apache.ignite.cache.CacheMode;
+import org.apache.ignite.cache.CacheWriteSynchronizationMode;
+import org.apache.ignite.cache.QueryEntity;
+import org.apache.ignite.cache.query.SqlFieldsQuery;
+import org.apache.ignite.configuration.CacheConfiguration;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.processors.cache.GridCacheContext;
+import org.apache.ignite.internal.processors.cache.persistence.CacheDataRow;
+import org.apache.ignite.internal.processors.query.GridQueryProcessor;
+import org.apache.ignite.internal.processors.query.GridQueryTypeDescriptor;
+import org.apache.ignite.internal.processors.query.h2.IgniteH2Indexing;
+import org.apache.ignite.internal.util.typedef.G;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+/** */
+@RunWith(Parameterized.class)
+public class SqlPartitionEvictionTest extends GridCommonAbstractTest {
+    /** */
+    private static final String POI_CACHE_NAME = "POI_CACHE";
+
+    /** */
+    private static final String POI_SCHEMA_NAME = "DOMAIN";
+
+    /** */
+    private static final String POI_TABLE_NAME = "POI";
+
+    /** */
+    private static final String POI_CLASS_NAME = "PointOfInterest";
+
+    /** */
+    private static final String ID_FIELD_NAME = "id";
+
+    /** */
+    private static final String NAME_FIELD_NAME = "name";
+
+    /** */
+    private static final String LATITUDE_FIELD_NAME = "latitude";
+
+    /** */
+    private static final String LONGITUDE_FIELD_NAME = "longitude";
+
+    /** */
+    private static final int NUM_ENTITIES = 1_000;
+
+    /** Test parameters. */
+    @Parameterized.Parameters(name = "backups_count={0}")
+    public static Iterable<Object[]> params() {
+        return Arrays.asList(
+                new Object[] { 0 },
+                new Object[] { 1 },
+                new Object[] { 2 }
+            );
+    }
+
+    /** */
+    @Parameterized.Parameter
+    public int backupsCount;
+

Review comment:
       Missing javadoc. Also, this variable (`latch`) should not be static. It can be final member (in Junit, runner create test class for every test method run.)




----------------------------------------------------------------
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] konstantins304 commented on a change in pull request #8439: IGNITE-13572 Don't skip filtering for caches with zero backups.

Posted by GitBox <gi...@apache.org>.
konstantins304 commented on a change in pull request #8439:
URL: https://github.com/apache/ignite/pull/8439#discussion_r527695739



##########
File path: modules/indexing/src/test/java/org/apache/ignite/internal/processors/cache/index/SqlPartitionEvictionTest.java
##########
@@ -0,0 +1,196 @@
+/*
+ * 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.index;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Random;
+import java.util.concurrent.ThreadLocalRandom;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.IgniteCache;
+import org.apache.ignite.IgniteCheckedException;
+import org.apache.ignite.IgniteDataStreamer;
+import org.apache.ignite.binary.BinaryObject;
+import org.apache.ignite.cache.CacheAtomicityMode;
+import org.apache.ignite.cache.CacheMode;
+import org.apache.ignite.cache.CacheWriteSynchronizationMode;
+import org.apache.ignite.cache.QueryEntity;
+import org.apache.ignite.cache.query.SqlFieldsQuery;
+import org.apache.ignite.configuration.CacheConfiguration;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.processors.cache.GridCacheContext;
+import org.apache.ignite.internal.processors.cache.persistence.CacheDataRow;
+import org.apache.ignite.internal.processors.query.GridQueryProcessor;
+import org.apache.ignite.internal.processors.query.GridQueryTypeDescriptor;
+import org.apache.ignite.internal.processors.query.h2.IgniteH2Indexing;
+import org.apache.ignite.internal.util.typedef.G;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+/** */
+@RunWith(Parameterized.class)
+public class SqlPartitionEvictionTest extends GridCommonAbstractTest {
+    /** */
+    private static final String POI_CACHE_NAME = "POI_CACHE";
+
+    /** */
+    private static final String POI_SCHEMA_NAME = "DOMAIN";
+
+    /** */
+    private static final String POI_TABLE_NAME = "POI";
+
+    /** */
+    private static final String POI_CLASS_NAME = "PointOfInterest";
+
+    /** */
+    private static final String ID_FIELD_NAME = "id";
+
+    /** */
+    private static final String NAME_FIELD_NAME = "name";
+
+    /** */
+    private static final String LATITUDE_FIELD_NAME = "latitude";
+
+    /** */
+    private static final String LONGITUDE_FIELD_NAME = "longitude";
+
+    /** */
+    private static final int NUM_ENTITIES = 1_000;
+
+    /** Test parameters. */
+    @Parameterized.Parameters(name = "backups_count={0}")
+    public static Iterable<Object[]> params() {
+        return Arrays.asList(
+                new Object[] { 0 },
+                new Object[] { 1 },
+                new Object[] { 2 }
+            );
+    }
+
+    /** */
+    @Parameterized.Parameter
+    public int backupsCount;
+
+    /** {@inheritDoc} */
+    @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception {
+        IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName);
+
+        cfg.setCacheConfiguration(new CacheConfiguration<>(POI_CACHE_NAME)
+            .setAtomicityMode(CacheAtomicityMode.ATOMIC)
+            .setSqlSchema("DOMAIN")
+            .setQueryEntities(Collections.singletonList(queryEntity()))
+            .setWriteSynchronizationMode(CacheWriteSynchronizationMode.FULL_SYNC)
+            .setCacheMode(CacheMode.PARTITIONED)
+            .setBackups(backupsCount)
+        );
+
+        cfg.setActiveOnStart(true);
+
+        return cfg;
+    }
+
+    /** {@inheritDoc} */
+    @Override protected void afterTest() throws Exception {
+        stopAllGrids(true);
+
+        super.afterTest();
+    }
+
+    @Test
+    public void testSqlConsistencyOnEviction() throws Exception {
+        ignitionStart(0);

Review comment:
       Ivan, did I understand you correctly that I need to put a latch.getCount() to check in the remove method and add an await in the test before assert, just to make sure that the eviction was triggered to avoid my error with the wrong number of nodes for backup count?




----------------------------------------------------------------
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] ivandasch commented on a change in pull request #8439: IGNITE-13572 Don't skip filtering for caches with zero backups.

Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #8439:
URL: https://github.com/apache/ignite/pull/8439#discussion_r528172277



##########
File path: modules/indexing/src/test/java/org/apache/ignite/internal/processors/cache/index/SqlPartitionEvictionTest.java
##########
@@ -0,0 +1,210 @@
+/*
+ * 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.index;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Random;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ThreadLocalRandom;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.IgniteCache;
+import org.apache.ignite.IgniteCheckedException;
+import org.apache.ignite.IgniteDataStreamer;
+import org.apache.ignite.binary.BinaryObject;
+import org.apache.ignite.cache.CacheAtomicityMode;
+import org.apache.ignite.cache.CacheMode;
+import org.apache.ignite.cache.CacheWriteSynchronizationMode;
+import org.apache.ignite.cache.QueryEntity;
+import org.apache.ignite.cache.query.SqlFieldsQuery;
+import org.apache.ignite.configuration.CacheConfiguration;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.processors.cache.GridCacheContext;
+import org.apache.ignite.internal.processors.cache.persistence.CacheDataRow;
+import org.apache.ignite.internal.processors.query.GridQueryProcessor;
+import org.apache.ignite.internal.processors.query.GridQueryTypeDescriptor;
+import org.apache.ignite.internal.processors.query.h2.IgniteH2Indexing;
+import org.apache.ignite.internal.util.typedef.G;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+/** */
+@RunWith(Parameterized.class)
+public class SqlPartitionEvictionTest extends GridCommonAbstractTest {
+    /** */
+    private static final String POI_CACHE_NAME = "POI_CACHE";
+
+    /** */
+    private static final String POI_SCHEMA_NAME = "DOMAIN";
+
+    /** */
+    private static final String POI_TABLE_NAME = "POI";
+
+    /** */
+    private static final String POI_CLASS_NAME = "PointOfInterest";
+
+    /** */
+    private static final String ID_FIELD_NAME = "id";
+
+    /** */
+    private static final String NAME_FIELD_NAME = "name";
+
+    /** */
+    private static final String LATITUDE_FIELD_NAME = "latitude";
+
+    /** */
+    private static final String LONGITUDE_FIELD_NAME = "longitude";
+
+    /** */
+    private static final int NUM_ENTITIES = 1_000;
+
+    /** Test parameters. */
+    @Parameterized.Parameters(name = "backups_count={0}")
+    public static Iterable<Object[]> params() {
+        return Arrays.asList(
+                new Object[] { 0 },
+                new Object[] { 1 },
+                new Object[] { 2 }
+            );
+    }
+
+    /** */
+    @Parameterized.Parameter
+    public int backupsCount;
+
+    private static CountDownLatch latch;
+
+    /** {@inheritDoc} */
+    @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception {
+        IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName);
+
+        cfg.setCacheConfiguration(new CacheConfiguration<>(POI_CACHE_NAME)
+            .setAtomicityMode(CacheAtomicityMode.ATOMIC)
+            .setSqlSchema("DOMAIN")
+            .setQueryEntities(Collections.singletonList(queryEntity()))
+            .setWriteSynchronizationMode(CacheWriteSynchronizationMode.FULL_SYNC)
+            .setCacheMode(CacheMode.PARTITIONED)
+            .setBackups(backupsCount)
+        );
+
+        cfg.setActiveOnStart(true);
+
+        return cfg;
+    }
+
+    @Override protected void beforeTest() throws Exception {
+        latch = new CountDownLatch(1);
+
+        super.beforeTest();
+    }
+
+    /** {@inheritDoc} */
+    @Override protected void afterTest() throws Exception {
+        stopAllGrids(true);
+
+        super.afterTest();
+    }
+
+    @Test
+    public void testSqlConsistencyOnEviction() throws Exception {
+        IgniteEx ig = null;
+        int idx = 0;
+        while (idx <= backupsCount)
+            ig = ignitionStart(idx++);
+
+        loadData(ig, 0, NUM_ENTITIES);
+
+        ignitionStart(idx);
+
+        awaitPartitionMapExchange();
+
+        U.await(latch);

Review comment:
       I suppose it's better to wait with timeout, let it be  around 10 secs.




----------------------------------------------------------------
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] ivandasch commented on a change in pull request #8439: IGNITE-13572 Don't skip filtering for caches with zero backups.

Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #8439:
URL: https://github.com/apache/ignite/pull/8439#discussion_r532642776



##########
File path: modules/indexing/src/test/java/org/apache/ignite/internal/processors/cache/index/SqlPartitionEvictionTest.java
##########
@@ -0,0 +1,220 @@
+/*
+ * 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.index;
+
+import static java.util.concurrent.TimeUnit.SECONDS;
+
+import org.apache.ignite.Ignite;
+import org.apache.ignite.IgniteCache;
+import org.apache.ignite.IgniteCheckedException;
+import org.apache.ignite.IgniteDataStreamer;
+import org.apache.ignite.binary.BinaryObject;
+import org.apache.ignite.cache.CacheAtomicityMode;
+import org.apache.ignite.cache.CacheMode;
+import org.apache.ignite.cache.CacheWriteSynchronizationMode;
+import org.apache.ignite.cache.QueryEntity;
+import org.apache.ignite.cache.query.SqlFieldsQuery;
+import org.apache.ignite.configuration.CacheConfiguration;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.processors.cache.GridCacheContext;
+import org.apache.ignite.internal.processors.cache.persistence.CacheDataRow;
+import org.apache.ignite.internal.processors.query.GridQueryProcessor;
+import org.apache.ignite.internal.processors.query.GridQueryTypeDescriptor;
+import org.apache.ignite.internal.processors.query.h2.IgniteH2Indexing;
+import org.apache.ignite.internal.util.typedef.G;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+import java.util.Arrays;

Review comment:
       Invalid order, please run checkstyle locally




----------------------------------------------------------------
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] ivandasch commented on a change in pull request #8439: IGNITE-13572 Don't skip filtering for caches with zero backups.

Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #8439:
URL: https://github.com/apache/ignite/pull/8439#discussion_r525842284



##########
File path: modules/indexing/src/test/java/org/apache/ignite/internal/processors/cache/index/SqlPartitionEvictionTest.java
##########
@@ -0,0 +1,196 @@
+/*
+ * 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.index;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Random;
+import java.util.concurrent.ThreadLocalRandom;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.IgniteCache;
+import org.apache.ignite.IgniteCheckedException;
+import org.apache.ignite.IgniteDataStreamer;
+import org.apache.ignite.binary.BinaryObject;
+import org.apache.ignite.cache.CacheAtomicityMode;
+import org.apache.ignite.cache.CacheMode;
+import org.apache.ignite.cache.CacheWriteSynchronizationMode;
+import org.apache.ignite.cache.QueryEntity;
+import org.apache.ignite.cache.query.SqlFieldsQuery;
+import org.apache.ignite.configuration.CacheConfiguration;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.processors.cache.GridCacheContext;
+import org.apache.ignite.internal.processors.cache.persistence.CacheDataRow;
+import org.apache.ignite.internal.processors.query.GridQueryProcessor;
+import org.apache.ignite.internal.processors.query.GridQueryTypeDescriptor;
+import org.apache.ignite.internal.processors.query.h2.IgniteH2Indexing;
+import org.apache.ignite.internal.util.typedef.G;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+/** */
+@RunWith(Parameterized.class)
+public class SqlPartitionEvictionTest extends GridCommonAbstractTest {
+    /** */
+    private static final String POI_CACHE_NAME = "POI_CACHE";
+
+    /** */
+    private static final String POI_SCHEMA_NAME = "DOMAIN";
+
+    /** */
+    private static final String POI_TABLE_NAME = "POI";
+
+    /** */
+    private static final String POI_CLASS_NAME = "PointOfInterest";
+
+    /** */
+    private static final String ID_FIELD_NAME = "id";
+
+    /** */
+    private static final String NAME_FIELD_NAME = "name";
+
+    /** */
+    private static final String LATITUDE_FIELD_NAME = "latitude";
+
+    /** */
+    private static final String LONGITUDE_FIELD_NAME = "longitude";
+
+    /** */
+    private static final int NUM_ENTITIES = 1_000;
+
+    /** Test parameters. */
+    @Parameterized.Parameters(name = "backups_count={0}")
+    public static Iterable<Object[]> params() {
+        return Arrays.asList(
+                new Object[] { 0 },
+                new Object[] { 1 },
+                new Object[] { 2 }
+            );
+    }
+
+    /** */
+    @Parameterized.Parameter
+    public int backupsCount;
+
+    /** {@inheritDoc} */
+    @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception {
+        IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName);
+
+        cfg.setCacheConfiguration(new CacheConfiguration<>(POI_CACHE_NAME)
+            .setAtomicityMode(CacheAtomicityMode.ATOMIC)
+            .setSqlSchema("DOMAIN")
+            .setQueryEntities(Collections.singletonList(queryEntity()))
+            .setWriteSynchronizationMode(CacheWriteSynchronizationMode.FULL_SYNC)
+            .setCacheMode(CacheMode.PARTITIONED)
+            .setBackups(backupsCount)
+        );
+
+        cfg.setActiveOnStart(true);
+
+        return cfg;
+    }
+
+    /** {@inheritDoc} */
+    @Override protected void afterTest() throws Exception {
+        stopAllGrids(true);
+
+        super.afterTest();
+    }
+
+    @Test
+    public void testSqlConsistencyOnEviction() throws Exception {
+        ignitionStart(0);

Review comment:
       I have few additional requests:
   1. If you use backup counter equals to 2, 3 grids is not enough to start eviction. At least 4 grids (3 initial and 1 started
   after loading) are required.
   2. I suppose that we can add latch or even better counter in remove method of blocking indexing and await for actual eviction 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] konstantins304 commented on pull request #8439: IGNITE-13572 Don't skip filtering for caches with zero backups.

Posted by GitBox <gi...@apache.org>.
konstantins304 commented on pull request #8439:
URL: https://github.com/apache/ignite/pull/8439#issuecomment-733755761


   > Some minor format issues that need to be changed.
   
   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] ivandasch commented on a change in pull request #8439: IGNITE-13572 Don't skip filtering for caches with zero backups.

Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #8439:
URL: https://github.com/apache/ignite/pull/8439#discussion_r532639743



##########
File path: modules/indexing/src/test/java/org/apache/ignite/internal/processors/cache/index/SqlPartitionEvictionTest.java
##########
@@ -0,0 +1,210 @@
+/*
+ * 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.index;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Random;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ThreadLocalRandom;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.IgniteCache;
+import org.apache.ignite.IgniteCheckedException;
+import org.apache.ignite.IgniteDataStreamer;
+import org.apache.ignite.binary.BinaryObject;
+import org.apache.ignite.cache.CacheAtomicityMode;
+import org.apache.ignite.cache.CacheMode;
+import org.apache.ignite.cache.CacheWriteSynchronizationMode;
+import org.apache.ignite.cache.QueryEntity;
+import org.apache.ignite.cache.query.SqlFieldsQuery;
+import org.apache.ignite.configuration.CacheConfiguration;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.processors.cache.GridCacheContext;
+import org.apache.ignite.internal.processors.cache.persistence.CacheDataRow;
+import org.apache.ignite.internal.processors.query.GridQueryProcessor;
+import org.apache.ignite.internal.processors.query.GridQueryTypeDescriptor;
+import org.apache.ignite.internal.processors.query.h2.IgniteH2Indexing;
+import org.apache.ignite.internal.util.typedef.G;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+/** */
+@RunWith(Parameterized.class)
+public class SqlPartitionEvictionTest extends GridCommonAbstractTest {
+    /** */
+    private static final String POI_CACHE_NAME = "POI_CACHE";
+
+    /** */
+    private static final String POI_SCHEMA_NAME = "DOMAIN";
+
+    /** */
+    private static final String POI_TABLE_NAME = "POI";
+
+    /** */
+    private static final String POI_CLASS_NAME = "PointOfInterest";
+
+    /** */
+    private static final String ID_FIELD_NAME = "id";
+
+    /** */
+    private static final String NAME_FIELD_NAME = "name";
+
+    /** */
+    private static final String LATITUDE_FIELD_NAME = "latitude";
+
+    /** */
+    private static final String LONGITUDE_FIELD_NAME = "longitude";
+
+    /** */
+    private static final int NUM_ENTITIES = 1_000;
+
+    /** Test parameters. */
+    @Parameterized.Parameters(name = "backups_count={0}")
+    public static Iterable<Object[]> params() {
+        return Arrays.asList(
+                new Object[] { 0 },
+                new Object[] { 1 },
+                new Object[] { 2 }
+            );
+    }
+
+    /** */
+    @Parameterized.Parameter
+    public int backupsCount;
+

Review comment:
       Opps, sorry, I missed it. Ok, there is no variant except this one. So let's add 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



[GitHub] [ignite] ivandasch commented on a change in pull request #8439: IGNITE-13572 Don't skip filtering for caches with zero backups.

Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #8439:
URL: https://github.com/apache/ignite/pull/8439#discussion_r529423077



##########
File path: modules/indexing/src/test/java/org/apache/ignite/internal/processors/cache/index/SqlPartitionEvictionTest.java
##########
@@ -0,0 +1,210 @@
+/*
+ * 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.index;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Random;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ThreadLocalRandom;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.IgniteCache;
+import org.apache.ignite.IgniteCheckedException;
+import org.apache.ignite.IgniteDataStreamer;
+import org.apache.ignite.binary.BinaryObject;
+import org.apache.ignite.cache.CacheAtomicityMode;
+import org.apache.ignite.cache.CacheMode;
+import org.apache.ignite.cache.CacheWriteSynchronizationMode;
+import org.apache.ignite.cache.QueryEntity;
+import org.apache.ignite.cache.query.SqlFieldsQuery;
+import org.apache.ignite.configuration.CacheConfiguration;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.processors.cache.GridCacheContext;
+import org.apache.ignite.internal.processors.cache.persistence.CacheDataRow;
+import org.apache.ignite.internal.processors.query.GridQueryProcessor;
+import org.apache.ignite.internal.processors.query.GridQueryTypeDescriptor;
+import org.apache.ignite.internal.processors.query.h2.IgniteH2Indexing;
+import org.apache.ignite.internal.util.typedef.G;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+/** */
+@RunWith(Parameterized.class)
+public class SqlPartitionEvictionTest extends GridCommonAbstractTest {
+    /** */
+    private static final String POI_CACHE_NAME = "POI_CACHE";
+
+    /** */
+    private static final String POI_SCHEMA_NAME = "DOMAIN";
+
+    /** */
+    private static final String POI_TABLE_NAME = "POI";
+
+    /** */
+    private static final String POI_CLASS_NAME = "PointOfInterest";
+
+    /** */
+    private static final String ID_FIELD_NAME = "id";
+
+    /** */
+    private static final String NAME_FIELD_NAME = "name";
+
+    /** */
+    private static final String LATITUDE_FIELD_NAME = "latitude";
+
+    /** */
+    private static final String LONGITUDE_FIELD_NAME = "longitude";
+
+    /** */
+    private static final int NUM_ENTITIES = 1_000;
+
+    /** Test parameters. */
+    @Parameterized.Parameters(name = "backups_count={0}")
+    public static Iterable<Object[]> params() {
+        return Arrays.asList(
+                new Object[] { 0 },
+                new Object[] { 1 },
+                new Object[] { 2 }
+            );
+    }
+
+    /** */
+    @Parameterized.Parameter
+    public int backupsCount;
+
+    private static CountDownLatch latch;
+
+    /** {@inheritDoc} */
+    @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception {
+        IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName);
+
+        cfg.setCacheConfiguration(new CacheConfiguration<>(POI_CACHE_NAME)
+            .setAtomicityMode(CacheAtomicityMode.ATOMIC)
+            .setSqlSchema("DOMAIN")
+            .setQueryEntities(Collections.singletonList(queryEntity()))
+            .setWriteSynchronizationMode(CacheWriteSynchronizationMode.FULL_SYNC)
+            .setCacheMode(CacheMode.PARTITIONED)
+            .setBackups(backupsCount)
+        );
+
+        cfg.setActiveOnStart(true);
+
+        return cfg;
+    }
+
+    @Override protected void beforeTest() throws Exception {
+        latch = new CountDownLatch(1);
+
+        super.beforeTest();
+    }
+
+    /** {@inheritDoc} */
+    @Override protected void afterTest() throws Exception {
+        stopAllGrids(true);
+
+        super.afterTest();
+    }
+
+    @Test
+    public void testSqlConsistencyOnEviction() throws Exception {
+        IgniteEx ig = null;
+        int idx = 0;

Review comment:
       May be just
   ```
   IgniteEx ig = null;
   
    for(int idx = 0; idx <= backupsCount; idx++)
           ig = ignitionStart(idx)
   ```




----------------------------------------------------------------
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] konstantins304 commented on a change in pull request #8439: IGNITE-13572 Don't skip filtering for caches with zero backups.

Posted by GitBox <gi...@apache.org>.
konstantins304 commented on a change in pull request #8439:
URL: https://github.com/apache/ignite/pull/8439#discussion_r532637897



##########
File path: modules/indexing/src/test/java/org/apache/ignite/internal/processors/cache/index/SqlPartitionEvictionTest.java
##########
@@ -0,0 +1,210 @@
+/*
+ * 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.index;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Random;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ThreadLocalRandom;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.IgniteCache;
+import org.apache.ignite.IgniteCheckedException;
+import org.apache.ignite.IgniteDataStreamer;
+import org.apache.ignite.binary.BinaryObject;
+import org.apache.ignite.cache.CacheAtomicityMode;
+import org.apache.ignite.cache.CacheMode;
+import org.apache.ignite.cache.CacheWriteSynchronizationMode;
+import org.apache.ignite.cache.QueryEntity;
+import org.apache.ignite.cache.query.SqlFieldsQuery;
+import org.apache.ignite.configuration.CacheConfiguration;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.processors.cache.GridCacheContext;
+import org.apache.ignite.internal.processors.cache.persistence.CacheDataRow;
+import org.apache.ignite.internal.processors.query.GridQueryProcessor;
+import org.apache.ignite.internal.processors.query.GridQueryTypeDescriptor;
+import org.apache.ignite.internal.processors.query.h2.IgniteH2Indexing;
+import org.apache.ignite.internal.util.typedef.G;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+/** */
+@RunWith(Parameterized.class)
+public class SqlPartitionEvictionTest extends GridCommonAbstractTest {
+    /** */
+    private static final String POI_CACHE_NAME = "POI_CACHE";
+
+    /** */
+    private static final String POI_SCHEMA_NAME = "DOMAIN";
+
+    /** */
+    private static final String POI_TABLE_NAME = "POI";
+
+    /** */
+    private static final String POI_CLASS_NAME = "PointOfInterest";
+
+    /** */
+    private static final String ID_FIELD_NAME = "id";
+
+    /** */
+    private static final String NAME_FIELD_NAME = "name";
+
+    /** */
+    private static final String LATITUDE_FIELD_NAME = "latitude";
+
+    /** */
+    private static final String LONGITUDE_FIELD_NAME = "longitude";
+
+    /** */
+    private static final int NUM_ENTITIES = 1_000;
+
+    /** Test parameters. */
+    @Parameterized.Parameters(name = "backups_count={0}")
+    public static Iterable<Object[]> params() {
+        return Arrays.asList(
+                new Object[] { 0 },
+                new Object[] { 1 },
+                new Object[] { 2 }
+            );
+    }
+
+    /** */
+    @Parameterized.Parameter
+    public int backupsCount;
+

Review comment:
       Latch is used in static context of BlockingIndexing




----------------------------------------------------------------
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] ivandasch commented on a change in pull request #8439: IGNITE-13572 Don't skip filtering for caches with zero backups.

Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #8439:
URL: https://github.com/apache/ignite/pull/8439#discussion_r525842284



##########
File path: modules/indexing/src/test/java/org/apache/ignite/internal/processors/cache/index/SqlPartitionEvictionTest.java
##########
@@ -0,0 +1,196 @@
+/*
+ * 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.index;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Random;
+import java.util.concurrent.ThreadLocalRandom;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.IgniteCache;
+import org.apache.ignite.IgniteCheckedException;
+import org.apache.ignite.IgniteDataStreamer;
+import org.apache.ignite.binary.BinaryObject;
+import org.apache.ignite.cache.CacheAtomicityMode;
+import org.apache.ignite.cache.CacheMode;
+import org.apache.ignite.cache.CacheWriteSynchronizationMode;
+import org.apache.ignite.cache.QueryEntity;
+import org.apache.ignite.cache.query.SqlFieldsQuery;
+import org.apache.ignite.configuration.CacheConfiguration;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.processors.cache.GridCacheContext;
+import org.apache.ignite.internal.processors.cache.persistence.CacheDataRow;
+import org.apache.ignite.internal.processors.query.GridQueryProcessor;
+import org.apache.ignite.internal.processors.query.GridQueryTypeDescriptor;
+import org.apache.ignite.internal.processors.query.h2.IgniteH2Indexing;
+import org.apache.ignite.internal.util.typedef.G;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+/** */
+@RunWith(Parameterized.class)
+public class SqlPartitionEvictionTest extends GridCommonAbstractTest {
+    /** */
+    private static final String POI_CACHE_NAME = "POI_CACHE";
+
+    /** */
+    private static final String POI_SCHEMA_NAME = "DOMAIN";
+
+    /** */
+    private static final String POI_TABLE_NAME = "POI";
+
+    /** */
+    private static final String POI_CLASS_NAME = "PointOfInterest";
+
+    /** */
+    private static final String ID_FIELD_NAME = "id";
+
+    /** */
+    private static final String NAME_FIELD_NAME = "name";
+
+    /** */
+    private static final String LATITUDE_FIELD_NAME = "latitude";
+
+    /** */
+    private static final String LONGITUDE_FIELD_NAME = "longitude";
+
+    /** */
+    private static final int NUM_ENTITIES = 1_000;
+
+    /** Test parameters. */
+    @Parameterized.Parameters(name = "backups_count={0}")
+    public static Iterable<Object[]> params() {
+        return Arrays.asList(
+                new Object[] { 0 },
+                new Object[] { 1 },
+                new Object[] { 2 }
+            );
+    }
+
+    /** */
+    @Parameterized.Parameter
+    public int backupsCount;
+
+    /** {@inheritDoc} */
+    @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception {
+        IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName);
+
+        cfg.setCacheConfiguration(new CacheConfiguration<>(POI_CACHE_NAME)
+            .setAtomicityMode(CacheAtomicityMode.ATOMIC)
+            .setSqlSchema("DOMAIN")
+            .setQueryEntities(Collections.singletonList(queryEntity()))
+            .setWriteSynchronizationMode(CacheWriteSynchronizationMode.FULL_SYNC)
+            .setCacheMode(CacheMode.PARTITIONED)
+            .setBackups(backupsCount)
+        );
+
+        cfg.setActiveOnStart(true);
+
+        return cfg;
+    }
+
+    /** {@inheritDoc} */
+    @Override protected void afterTest() throws Exception {
+        stopAllGrids(true);
+
+        super.afterTest();
+    }
+
+    @Test
+    public void testSqlConsistencyOnEviction() throws Exception {
+        ignitionStart(0);

Review comment:
       I have few additional requests:
   1. If you use backup counter equals to 2, 3 grids is not enough to start eviction. At least 4 grids (3 initial and 1 started
   after loading) are required.
   2. I suppose that we can add latch or even better counter in remove method of blocking indexing and await for actual eviction start. This measure can assure us that actual eviction begins.
    




----------------------------------------------------------------
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