oleg-vlsk commented on code in PR #12573:
URL: https://github.com/apache/ignite/pull/12573#discussion_r2608847476


##########
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/SqlDiagnosticIntegrationTest.java:
##########
@@ -1136,6 +1145,16 @@ public static boolean waitLatch(long time) {
             return true;
         }
 
+        /** */
+        @QuerySqlFunction
+        public static boolean sleep(int sleep) {

Review Comment:
   Do we need to add this function for this particular test? Can't we re-use 
the latch mechanism like it is done in other tests in this class?



##########
modules/indexing/src/test/java/org/apache/ignite/internal/processors/query/LongRunningQueryTest.java:
##########
@@ -517,6 +520,36 @@ public void 
testEmptyHeavyQueriesTrackerWithMultipleCancelledQueries() {
             cancelQuery(qryIds.poll());
     }
 
+    /**
+     * Verifies query initiator id information in logs.
+     */
+    @Test
+    @MultiNodeTest
+    public void testQueryInitiatorId() throws Exception {

Review Comment:
   Redundant exception?



##########
modules/indexing/src/test/java/org/apache/ignite/internal/processors/query/LongRunningQueryTest.java:
##########
@@ -517,6 +520,36 @@ public void 
testEmptyHeavyQueriesTrackerWithMultipleCancelledQueries() {
             cancelQuery(qryIds.poll());
     }
 
+    /**
+     * Verifies query initiator id information in logs.
+     */
+    @Test
+    @MultiNodeTest
+    public void testQueryInitiatorId() throws Exception {
+        ListeningTestLogger testLog = testLog();
+
+        checkInitiatorId(testLog, "LOCAL", "SELECT sleep_func(?, 0)", 
LONG_QUERY_WARNING_TIMEOUT);
+
+        checkInitiatorId(testLog, "MAP", "SELECT val FROM test WHERE id = 
sleep_func(?, 0)",
+            LONG_QUERY_WARNING_TIMEOUT);
+
+        checkInitiatorId(testLog, "REDUCE", "SELECT sleep_func(?, sum(val)) 
FROM test WHERE id + 1 = 1",
+            LONG_QUERY_WARNING_TIMEOUT);
+    }
+
+    /** */
+    private void checkInitiatorId(ListeningTestLogger log, String type, String 
sql, Object... args) {
+        String initiatorId = UUID.randomUUID().toString();
+
+        LogListener lsnr = 
LogListener.matches(LONG_QUERY_FINISHED_MSG).andMatches(type).andMatches(initiatorId).build();

Review Comment:
   Wouldn't be better to use `"type=" + type` to explicitly indicate that we 
need to verify the type of the query?



##########
modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/H2QueryInfo.java:
##########
@@ -72,7 +72,10 @@ public class H2QueryInfo implements TrackableQuery {
     private final UUID nodeId;
 
     /** Query id. */
-    private final long queryId;
+    private final long qryId;
+
+    /** Query initiator ID. */
+    private final String initiatorId;

Review Comment:
   Do we also need to add `initiatorId` to `H2DmlInfo`?



##########
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/SqlDiagnosticIntegrationTest.java:
##########
@@ -1027,6 +1027,15 @@ public void testSqlFieldsQueryWithInitiatorId() throws 
Exception {
                 return testId.equals(view.initiatorId());
             }, 3_000));
         }
+
+        LogListener logLsnr = 
LogListener.matches(LONG_QUERY_FINISHED_MSG).andMatches("initiatorId=testId2").build();

Review Comment:
   Wouldn't it be better to move `testId2` to a local variable?



##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/RootQuery.java:
##########
@@ -444,6 +451,7 @@ else if (state == QueryState.CLOSING)
             .append(", type=CALCITE")
             .append(", state=").append(state)
             .append(", schema=").append(ctx.schemaName())
+            .append(", initiatorId=").append(initiatorId)

Review Comment:
   Do we need to check if `initiatorId` in `null` here? Or do we add it to the 
query info even if it is `null`?



##########
modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/H2QueryInfo.java:
##########
@@ -197,6 +202,7 @@ public synchronized void resumeTracking() {
                 .append(", enforceJoinOrder=").append(enforceJoinOrder)
                 .append(", lazy=").append(lazy)
                 .append(", schema=").append(schema)
+                .append(", initiatorId=").append(initiatorId)

Review Comment:
   Same here, do we need the `null` check, or do we add initiatorId to query 
info regardless?



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

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to