alex-plekhanov commented on code in PR #12544:
URL: https://github.com/apache/ignite/pull/12544#discussion_r2577219890
##########
modules/indexing/src/test/java/org/apache/ignite/internal/processors/query/SqlQueryHistorySelfTest.java:
##########
@@ -253,6 +254,39 @@ public void testSqlFieldsQueryHistoryWithInitiatorId() {
assertEquals(testId1, history.initiatorId());
}
+ /**
+ * Test total duration of SQL queries.
+ */
+ @Test
+ public void testSqlFieldsQueryTotalDuration() {
+ SqlFieldsQuery qry = new SqlFieldsQuery("select * from A.String where
_key=sleep_func(?, ?)").setArgs(500, 1);
+
+ IgniteCache<Integer, String> cache =
queryNode().context().cache().jcache("A");
+
+ long[] totalTimeArr = new long[2];
+
+ for (int i = 0; i < totalTimeArr.length; i++) {
+ cache.query(qry).getAll();
+
+ Collection<QueryHistory> historyCol =
queryNode().context().query().runningQueryManager()
+ .queryHistoryMetrics().values();
+
+ assertFalse(F.isEmpty(historyCol));
+ assertEquals(1, historyCol.size());
+
+ QueryHistory history = first(historyCol);
+
+ assertNotNull(history);
+
+ totalTimeArr[i] = history.totalTime();
+ }
+
+ long expTotalTime = totalTimeArr[0] * totalTimeArr.length;
+ long actTotalTime = totalTimeArr[1];
+
+ assertEquals(expTotalTime, actTotalTime, expTotalTime * 0.2);
Review Comment:
Maybe it's better to rely on original sleep time? Something like:
```
assertTrue(totalTimeArr[0] >= 500);
assertTrue(totalTimeArr[1] >= totalTimeArr[0] + 500);
```
Relying on `totalTimeArr[0] * totalTimeArr.length` looks fragile.
##########
modules/indexing/src/test/java/org/apache/ignite/internal/processors/query/SqlQueryHistorySelfTest.java:
##########
@@ -669,6 +703,21 @@ public static class Functions {
public static int fail() {
throw new IgniteSQLException("SQL function fail for test
purpuses");
}
+
+ /**
+ *
+ */
+ @QuerySqlFunction
+ public static int sleep_func(int sleep, int val) {
+ try {
+ Thread.sleep(sleep);
+ }
+ catch (InterruptedException ignored) {
+ // No-op
+ }
Review Comment:
```
doSleep(sleep);
```
Let's also add:
```
GridTestClockTimer.update();
```
To ensure that U.currentTimeMillis() is updated and difference between
currentTimeMillis calls always exceeds sleep time.
##########
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/SqlDiagnosticIntegrationTest.java:
##########
@@ -1028,6 +1028,66 @@ public void testSqlFieldsQueryWithInitiatorId() throws
Exception {
}
}
+ /**
+ * Verifies that query total execution time is correctly accumulated in
the DURATION_TOTAL field of the
+ * SQL_QUERIES_HISTORY system view.
+ */
+ @Test
+ public void testSqlQueryTotalDuration() throws Exception {
+ IgniteEx grid = grid(0);
+
+ IgniteCache<Long, Long> cache = prepareTestCache(grid);
+
+ long[] totalTimeArr = new long[2];
+
+ AtomicLong curTotalTime = new AtomicLong();
+
+ for (int i = 0; i < totalTimeArr.length; i++) {
+ FunctionsLibrary.latch = new CountDownLatch(1);
+
+ IgniteInternalFuture<?> fut = GridTestUtils.runAsync(
+ () -> cache.query(new SqlFieldsQuery("select * from test where
waitLatch(10000)")).getAll());
+
+ U.sleep(500);
+
+ FunctionsLibrary.latch.countDown();
+
+ fut.get();
+
+ int finI = i;
+
+ assertTrue(waitForCondition(() -> {
+ SystemView<SqlQueryHistoryView> history =
grid.context().systemView().view(SQL_QRY_HIST_VIEW);
+
+ assertNotNull(history);
+
+ if (history.size() != 1)
+ return false;
+
+ SqlQueryHistoryView view =
first(grid.context().systemView().view(SQL_QRY_HIST_VIEW));
+
+ assertNotNull(view);
+
+ long totalTime = view.durationTotal();
+
+ if (totalTime > curTotalTime.get()) {
+ curTotalTime.set(totalTime);
+
+ totalTimeArr[finI] = totalTime;
+
+ return true;
+ }
Review Comment:
Let's simplify checks:
```
if (totalTime >= curTotalTime.get() + 500) {
curTotalTime.set(totalTime);
return true;
}
```
totalTimeArr is not required anymore, assertEquals at the end of the method
is not required too.
##########
modules/indexing/src/test/java/org/apache/ignite/internal/processors/query/SqlQueryHistorySelfTest.java:
##########
@@ -669,6 +703,21 @@ public static class Functions {
public static int fail() {
throw new IgniteSQLException("SQL function fail for test
purpuses");
}
+
+ /**
+ *
+ */
+ @QuerySqlFunction
+ public static int sleep_func(int sleep, int val) {
Review Comment:
Let's return boolean without second parameter.
##########
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/SqlDiagnosticIntegrationTest.java:
##########
@@ -1028,6 +1028,66 @@ public void testSqlFieldsQueryWithInitiatorId() throws
Exception {
}
}
+ /**
+ * Verifies that query total execution time is correctly accumulated in
the DURATION_TOTAL field of the
+ * SQL_QUERIES_HISTORY system view.
+ */
+ @Test
+ public void testSqlQueryTotalDuration() throws Exception {
+ IgniteEx grid = grid(0);
+
+ IgniteCache<Long, Long> cache = prepareTestCache(grid);
+
+ long[] totalTimeArr = new long[2];
+
+ AtomicLong curTotalTime = new AtomicLong();
+
+ for (int i = 0; i < totalTimeArr.length; i++) {
+ FunctionsLibrary.latch = new CountDownLatch(1);
+
+ IgniteInternalFuture<?> fut = GridTestUtils.runAsync(
+ () -> cache.query(new SqlFieldsQuery("select * from test where
waitLatch(10000)")).getAll());
+
+ U.sleep(500);
Review Comment:
Let's add:
```
GridTestClockTimer.update();
```
To ensure that U.currentTimeMillis() is updated and difference between
currentTimeMillis calls always exceeds sleep time.
##########
modules/indexing/src/test/java/org/apache/ignite/internal/processors/query/SqlQueryHistorySelfTest.java:
##########
@@ -253,6 +254,39 @@ public void testSqlFieldsQueryHistoryWithInitiatorId() {
assertEquals(testId1, history.initiatorId());
}
+ /**
+ * Test total duration of SQL queries.
+ */
+ @Test
+ public void testSqlFieldsQueryTotalDuration() {
+ SqlFieldsQuery qry = new SqlFieldsQuery("select * from A.String where
_key=sleep_func(?, ?)").setArgs(500, 1);
Review Comment:
Whis query has no guarantees that sleep_func will be executed only once. At
least sleep_func will be executed on each node. Let's rewrite to something like:
```
SqlFieldsQuery qry = new SqlFieldsQuery("select * from A.String where _key=0
and sleep_func(?)").setArgs(500);
```
Or
```
SqlFieldsQuery qry = new SqlFieldsQuery("select sleep_func(?) from A.String
where _key=0").setArgs(500);
```
In this case partition pruning will be applied, and only one node with only
one entry will be selected.
--
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]
