IGNITE-21445 New implementation for local IndexQuery #11317

oleg-vlsk · 2024-04-17T01:25:31Z

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)
The pull request has been checked by the Teamcity Bot and
the green visa attached to the JIRA ticket (see TC.Bot: Check PR)

Notes

If you need any help, please email dev@ignite.apache.org or ask anу advice on http://asf.slack.com #ignite channel.

timoninmaxim · 2024-04-26T11:36:41Z

...e/src/main/java/org/apache/ignite/internal/processors/cache/query/GridCacheQueryAdapter.java


        GridDhtCacheAdapter<?, ?> cacheAdapter = cctx.isNear() ? cctx.near().dht() : cctx.dht();

        Set<Integer> lostParts = cacheAdapter.topology().lostPartitions();

        if (!lostParts.isEmpty()) {
            if (part == null || lostParts.contains(part)) {
-                throw new CacheException(new CacheInvalidStateException("Failed to execute query because cache partition " +


Let's revert these changes

timoninmaxim · 2024-04-26T11:37:31Z

...e/src/main/java/org/apache/ignite/internal/processors/cache/query/GridCacheQueryAdapter.java

@@ -607,7 +611,7 @@ private <R> CacheQueryFuture<R> execute0(@Nullable IgniteReducer<T, R> rmtReduce
                }
            }

-            return new GridEmptyCloseableIterator();
+            throw new IgniteException(new ClusterGroupEmptyException());


Why do you change the behavior here?

If this kind of Exception is not thrown here then the IndexQueryLocalTest#testClientNodeReplicatedCache() will fail.

timoninmaxim · 2024-04-26T14:15:19Z

...les/core/src/main/java/org/apache/ignite/internal/processors/cache/IgniteCacheProxyImpl.java

@@ -818,8 +840,8 @@ private QueryCursor<Cache.Entry<K, V>> queryContinuous(AbstractContinuousQuery q
                return (FieldsQueryCursor<R>)ctx.kernalContext().query().querySqlFields(ctx, (SqlFieldsQuery)qry,
                    null, keepBinary, true).get(0);

-            if (qry instanceof ScanQuery)
-                return query((ScanQuery)qry, null, projection(qry.isLocal()));
+            if (qry instanceof ScanQuery || (qry instanceof IndexQuery && qry.isLocal()))


Looks like if for IndexQuery can be moved inside the call on the 846L.

timoninmaxim · 2024-04-26T14:16:16Z

...les/core/src/main/java/org/apache/ignite/internal/processors/cache/IgniteCacheProxyImpl.java

+                p, transformer, scanQry.getPartition(), isKeepBinary, qry.isLocal(), null);
+        }
+        else {
+            IndexQuery idxQryLoc = (IndexQuery)qry;


This logic is already specified in IgniteCacheProxyImpl#qyery(Query, ClusterGroup). Let's move handle of IndexQuery in the method.

timoninmaxim · 2024-04-26T14:22:19Z

...e/src/main/java/org/apache/ignite/internal/processors/cache/query/GridCacheQueryAdapter.java

@@ -625,7 +629,7 @@ private <R> CacheQueryFuture<R> execute0(@Nullable IgniteReducer<T, R> rmtReduce
        GridCloseableIterator it;

        if (loc)
-            it = qryMgr.scanQueryLocal(this, true);
+            it = qryMgr.queryLocal(this, true);


It looks strange for non-local query we call scanQueryDistributed while for local it calls generic method. Let's separate scanQueryLocal and indexQueryLocal.

timoninmaxim · 2024-04-26T14:23:09Z

...e/src/main/java/org/apache/ignite/internal/processors/cache/query/GridCacheQueryManager.java

     *
     * @param qry Query.
     * @param updateStatistics Update statistics flag.
     */
    @SuppressWarnings({"unchecked"})
-    protected GridCloseableIterator scanQueryLocal(final GridCacheQueryAdapter qry,
+    protected GridCloseableIterator queryLocal(final GridCacheQueryAdapter qry,


Let's create 2 methods - scanQueryLocal and indexQueryLocal. They should reuse some common code.

timoninmaxim · 2024-05-02T07:45:04Z

...les/core/src/main/java/org/apache/ignite/internal/processors/cache/query/CacheQueryType.java

+    SPI,
+
+    /** Index query. */
+    INDEX


Looks like now we need a test for new QueryType

Test added, new functionality for recording index query events added.

...e/src/main/java/org/apache/ignite/internal/processors/cache/query/GridCacheQueryManager.java

timoninmaxim · 2024-05-02T07:46:47Z

...e/src/main/java/org/apache/ignite/internal/processors/cache/query/GridCacheQueryManager.java

                    EVT_CACHE_QUERY_EXECUTED,
-                    CacheQueryType.SCAN.name(),
+                    qry.type() == SCAN ? CacheQueryType.SCAN.name() : CacheQueryType.INDEX.name(),


qry.type().name()?

timoninmaxim · 2024-05-02T07:46:58Z

...e/src/main/java/org/apache/ignite/internal/processors/cache/query/GridCacheQueryManager.java

                    namex,
-                    null,
+                    clsName,


Why do you need this change?

Before this code was used for scan queries only, so the class name was irrelevant for cache query events and therefore was always ‘null’. Now this code processes both scan and index queries, and with index queries we need to record the name of the queried class to the event.

...e/src/main/java/org/apache/ignite/internal/processors/cache/query/GridCacheQueryManager.java

timoninmaxim · 2024-05-02T07:49:05Z

...e/src/main/java/org/apache/ignite/internal/processors/cache/query/GridCacheQueryManager.java

+                return scanIterator(qry, transformer, true);
+            }
+            catch (Exception e) {
+                throw new RuntimeException(e);


It looks like you change exception class.

Got rid of 'try-catch' in scanQueryLocal and indexQueryLocal by using IgniteThrowableSupplier.

…rtition checks added + refactoring

…to one sequence of methods

…l#query(Query, ClusterGroup) + separate methods for local Scan and Index Queries

…tion correction, minor changes

…ording Index Query events added

…s moved to a different patch

…dCacheQueryManager

oleg-vlsk changed the title ~~Ignite 21445~~ IGNITE-21445 IndexQuery ignores pageSize if setLocal=true is set Apr 17, 2024

oleg-vlsk changed the title ~~IGNITE-21445 IndexQuery ignores pageSize if setLocal=true is set~~ IGNITE-21445 New implementation for local IndexQuery Apr 17, 2024

timoninmaxim requested changes Apr 26, 2024

View reviewed changes

timoninmaxim requested changes May 2, 2024

View reviewed changes

oleg-vlsk added 13 commits May 17, 2024 09:30

IGNITE-21445 Methods for executing local Index Queries added

b4005b8

IGNITE-21445 IndexQueryIterator added

9d082c0

IGNITE-21445 IndexQueryIterator removed, lost partitions and wrong pa…

d7b8b24

…rtition checks added + refactoring

IGNITE-21445 ClusterGroupEmptyException added

40fbfd4

IGNITE-21445 Processing of Scan and local Index Queries integrated in…

34261c7

…to one sequence of methods

IGNITE-21445 Code style fix

f65ecd1

IGNITE-21445 Event processing and statistics added for Index queries

a911945

IGNITE-21445 Processing of Index Queries moved to IgniteCacheProxyImp…

df163ff

…l#query(Query, ClusterGroup) + separate methods for local Scan and Index Queries

IGNITE-21445 Minor javadoc change

1f0f728

IGNITE-21445 Wrapper method for local queries, ClusterGroupEmptyExcep…

0878e16

…tion correction, minor changes

IGNITE-21445 Test for Index Query events added, functionality for rec…

82a1464

…ording Index Query events added

IGNITE-21445 Recording of Index query events and the test those event…

c343ef6

…s moved to a different patch

IGNITE-21445 Events removed from IndexQueryIterator

2abc8ce

oleg-vlsk force-pushed the ignite-21445 branch from 7dd8385 to 2abc8ce Compare May 17, 2024 00:07

oleg-vlsk added 7 commits May 17, 2024 10:14

IGNITE-21445 Events for distributed index queries brought back in Gri…

a0b21f7

…dCacheQueryManager

IGNITE-21445 Minor fix for GridCacheQueryManager#indexQueryLocal

6ea14ee

IGNITE-21445 GridCacheQueryManager#queryFunc removed

5a04ff1

IGNITE-21445 Minor fixes

d51bd3d

IGNITE-21445 GridCacheQueryAdapter#executeQueryWithIterator removed

05723a2

IGNITE-21445 Minor refactoring

f8718c4

IGNITE-21445 Minor refactoring

624eea5

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

IGNITE-21445 New implementation for local IndexQuery #11317

IGNITE-21445 New implementation for local IndexQuery #11317

oleg-vlsk commented Apr 17, 2024

timoninmaxim Apr 26, 2024

oleg-vlsk Apr 27, 2024

timoninmaxim Apr 26, 2024

oleg-vlsk Apr 27, 2024

timoninmaxim Apr 26, 2024

oleg-vlsk Apr 27, 2024

timoninmaxim Apr 26, 2024

oleg-vlsk Apr 27, 2024

timoninmaxim Apr 26, 2024

oleg-vlsk Apr 27, 2024

timoninmaxim Apr 26, 2024

oleg-vlsk Apr 27, 2024

timoninmaxim May 2, 2024

oleg-vlsk May 7, 2024

timoninmaxim May 2, 2024

oleg-vlsk May 7, 2024

timoninmaxim May 2, 2024

oleg-vlsk May 7, 2024

timoninmaxim May 2, 2024

oleg-vlsk May 7, 2024

IGNITE-21445 New implementation for local IndexQuery #11317

Are you sure you want to change the base?

IGNITE-21445 New implementation for local IndexQuery #11317

Conversation

oleg-vlsk commented Apr 17, 2024

The Contribution Checklist

Notes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment