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]