You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2022/11/16 12:33:02 UTC
[GitHub] [druid] abhagraw opened a new pull request, #13374: Migrate current integration batch tests to equivalent MSQ tests
abhagraw opened a new pull request, #13374:
URL: https://github.com/apache/druid/pull/13374
### Description
Migrate current integration batch tests to equivalent MSQ tests in the new IT Framework. A new MSQ IT test was added which will pick up any json files present in `integration-tests-ex/cases/src/test/resources/multi-stage-query/batch-index`.
Also increased the verbosity of the tests running in the new IT framework.
This PR has:
- [ ] been self-reviewed.
- [ ] using the [concurrency checklist](https://github.com/apache/druid/blob/master/dev/code-review/concurrency.md) (Remove this item if the PR doesn't have any relation to concurrency.)
- [ ] added documentation for new or modified features or behaviors.
- [ ] a release note entry in the PR description.
- [ ] added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
- [ ] added or updated version, license, or notice information in [licenses.yaml](https://github.com/apache/druid/blob/master/dev/license.md)
- [ ] added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
- [ ] added unit tests or modified existing tests to cover new code paths, ensuring the threshold for [code coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md) is met.
- [ ] added integration tests.
- [ ] been tested in a test Druid cluster.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] abhagraw commented on pull request #13374: Migrate current integration batch tests to equivalent MSQ tests
Posted by GitBox <gi...@apache.org>.
abhagraw commented on PR #13374:
URL: https://github.com/apache/druid/pull/13374#issuecomment-1320809774
> Another nit we should fix in this PR :
>
> ```
> 2022-11-18T19:26:19,074 INFO [main] org.apache.druid.testing.utils.TestQueryHelper - Sql Task submitted with task Id - query-77b14a7e-999d-4756-92eb-9fba59f68836
>
> 2022-11-18T19:26:19,105 WARN [main] org.apache.druid.java.util.common.RetryUtils - Retrying (1 of 99) in 895ms.
>
> org.apache.druid.testing.utils.MsqTestQueryHelper$TaskStillRunningException: null
>
> at org.apache.druid.testing.utils.MsqTestQueryHelper.lambda$pollTaskIdForCompletion$0(MsqTestQueryHelper.java:163) ~[druid-integration-tests-25.0.0-SNAPSHOT.jar:25.0.0-SNAPSHOT]
>
> at org.apache.druid.java.util.common.RetryUtils.retry(RetryUtils.java:129) ~[druid-core-25.0.0-SNAPSHOT.jar:25.0.0-SNAPSHOT]
>
> at org.apache.druid.java.util.common.RetryUtils.retry(RetryUtils.java:81) ~[druid-core-25.0.0-SNAPSHOT.jar:25.0.0-SNAPSHOT]
>
> at org.apache.druid.java.util.common.RetryUtils.retry(RetryUtils.java:163) ~[druid-core-25.0.0-SNAPSHOT.jar:25.0.0-SNAPSHOT]
>
> at org.apache.druid.java.util.common.RetryUtils.retry(RetryUtils.java:153) ~[druid-core-25.0.0-SNAPSHOT.jar:25.0.0-SNAPSHOT]
>
> ```
>
> ```java
> public TaskState pollTaskIdForCompletion(String taskId) throws Exception
> {
> return RetryUtils.retry(
> () -> {
> TaskStatusPlus taskStatusPlus = overlordClient.getTaskStatus(taskId);
> TaskState statusCode = taskStatusPlus.getStatusCode();
> if (statusCode != null && statusCode.isComplete()) {
> return taskStatusPlus.getStatusCode();
> }
> throw new TaskStillRunningException();
> },
> (Throwable t) -> t instanceof TaskStillRunningException,
> 100
> );
> }
> ```
>
> Let's change this method to not throw an exception but just log unable to get the status of the task. We can do this as part of another PR as well.
Used quite retry - so that not all retries throw the exception + stacktrace.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] abhagraw commented on a diff in pull request #13374: Migrate current integration batch tests to equivalent MSQ tests
Posted by GitBox <gi...@apache.org>.
abhagraw commented on code in PR #13374:
URL: https://github.com/apache/druid/pull/13374#discussion_r1025757408
##########
integration-tests-ex/cases/src/test/resources/multi-stage-query/batch-index/wikipedia_index_msq.json:
##########
@@ -0,0 +1,4 @@
+{
+ "sqlTask" : "REPLACE INTO \"%%DATASOURCE%%\" OVERWRITE ALL\nWITH \"source\" as (SELECT * FROM TABLE(\n EXTERN(\n '{\"type\":\"local\",\"files\":[\"/resources/data/batch_index/json/wikipedia_index_data1.json\",\"/resources/data/batch_index/json/wikipedia_index_data2.json\",\"/resources/data/batch_index/json/wikipedia_index_data3.json\"]}',\n '{\"type\":\"json\"}',\n '[{\"name\":\"timestamp\",\"type\":\"string\"},{\"name\":\"page\",\"type\":\"string\"},{\"name\":\"language\",\"type\":\"string\"},{\"name\":\"user\",\"type\":\"string\"},{\"name\":\"unpatrolled\",\"type\":\"string\"},{\"name\":\"newPage\",\"type\":\"string\"},{\"name\":\"robot\",\"type\":\"string\"},{\"name\":\"anonymous\",\"type\":\"string\"},{\"name\":\"namespace\",\"type\":\"string\"},{\"name\":\"continent\",\"type\":\"string\"},{\"name\":\"country\",\"type\":\"string\"},{\"name\":\"region\",\"type\":\"string\"},{\"name\":\"city\",\"type\":\"string\"},{\"name\":\"added\",\"type\":\"double\"},{\"name\":\"de
leted\",\"type\":\"double\"},{\"name\":\"delta\",\"type\":\"double\"}]'\n )\n))\nSELECT\n TIME_FLOOR(CASE WHEN CAST(\"timestamp\" AS BIGINT) > 0 THEN MILLIS_TO_TIMESTAMP(CAST(\"timestamp\" AS BIGINT)) ELSE TIME_PARSE(\"timestamp\") END, 'PT1S') AS __time,\n \"page\",\n \"language\",\n \"user\",\n \"unpatrolled\",\n \"newPage\",\n \"robot\",\n \"anonymous\",\n \"namespace\",\n \"continent\",\n \"country\",\n \"region\",\n \"city\",\n COUNT(*) AS \"count\",\n SUM(\"added\") AS \"added\",\n SUM(\"deleted\") AS \"deleted\",\n SUM(\"delta\") AS \"delta\",\n APPROX_COUNT_DISTINCT_DS_THETA(\"user\") AS \"thetaSketch\",\n DS_QUANTILES_SKETCH(\"delta\") AS \"quantilesDoublesSketch\",\n APPROX_COUNT_DISTINCT_DS_HLL(\"user\") AS \"HLLSketchBuild\"\nFROM \"source\"\nGROUP BY 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13\nPARTITIONED BY DAY",
Review Comment:
json files does not support pretty multi-line string syntax.
ref - https://stackoverflow.com/a/52557585
Have changed structure, and not MSQ sql ingestion queries are placed in separate sql files which are more readable.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] cryptoe commented on a diff in pull request #13374: Migrate current integration batch tests to equivalent MSQ tests
Posted by GitBox <gi...@apache.org>.
cryptoe commented on code in PR #13374:
URL: https://github.com/apache/druid/pull/13374#discussion_r1027018307
##########
integration-tests-ex/cases/src/test/java/org/apache/druid/testsEx/msq/ITSQLBasedBatchIngestion.java:
##########
@@ -0,0 +1,70 @@
+/*
+ * 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.druid.testsEx.msq;
+
+import junitparams.Parameters;
+import org.apache.commons.io.FilenameUtils;
+import org.apache.curator.shaded.com.google.common.collect.ImmutableMap;
+import org.apache.druid.testsEx.categories.MultiStageQuery;
+import org.apache.druid.testsEx.config.DruidTestRunner;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.runner.RunWith;
+
+import java.util.Arrays;
+import java.util.List;
+
+@RunWith(DruidTestRunner.class)
+@Category(MultiStageQuery.class)
+public class ITSQLBasedBatchIngestion extends AbstractITSQLBasedBatchIngestion
+{
+ private static final String BATCH_INDEX_TASKS_DIR = "/multi-stage-query/batch-index/";
+
+ public static List<List<String>> test_cases()
+ {
+ return Arrays.asList(
+ Arrays.asList("msq_inline.sql", "json_path_index_queries.json"),
+ Arrays.asList("sparse_column_msq.sql", "sparse_column_msq.json"),
+ Arrays.asList("wikipedia_http_inputsource_msq.sql", "wikipedia_http_inputsource_queries.json"),
+ Arrays.asList("wikipedia_index_msq.sql", "wikipedia_index_queries.json"),
+ Arrays.asList("wikipedia_merge_index_task.sql", "wikipedia_index_queries.json"),
+ Arrays.asList("wikipedia_index_task_with_transform.sql", "wikipedia_index_queries_with_transform.json")
+ );
+
+ }
+
+ @Test
+ @Parameters(method = "test_cases")
+ public void testSQLBasedBatchIngestion(String sqlFileName, String queryFileName)
+ {
+ try {
+ runMSQTaskandTestQueries(BATCH_INDEX_TASKS_DIR + sqlFileName,
+ BATCH_INDEX_TASKS_DIR + queryFileName,
+ FilenameUtils.removeExtension(sqlFileName),
+ ImmutableMap.of("finalizeAggregations", false,
+ "maxNumTasks", 10,
+ "groupByEnableMultiValueUnnesting", false
+ ));
+ }
+ catch (Exception e) {
Review Comment:
We might have to throw exception here.
Please log the current sql file and the query file name also.
##########
integration-tests-ex/cases/src/test/java/org/apache/druid/testsEx/msq/ITSQLBasedBatchIngestion.java:
##########
@@ -0,0 +1,70 @@
+/*
+ * 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.druid.testsEx.msq;
+
+import junitparams.Parameters;
+import org.apache.commons.io.FilenameUtils;
+import org.apache.curator.shaded.com.google.common.collect.ImmutableMap;
+import org.apache.druid.testsEx.categories.MultiStageQuery;
+import org.apache.druid.testsEx.config.DruidTestRunner;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.runner.RunWith;
+
+import java.util.Arrays;
+import java.util.List;
+
+@RunWith(DruidTestRunner.class)
+@Category(MultiStageQuery.class)
+public class ITSQLBasedBatchIngestion extends AbstractITSQLBasedBatchIngestion
+{
+ private static final String BATCH_INDEX_TASKS_DIR = "/multi-stage-query/batch-index/";
+
+ public static List<List<String>> test_cases()
+ {
+ return Arrays.asList(
+ Arrays.asList("msq_inline.sql", "json_path_index_queries.json"),
+ Arrays.asList("sparse_column_msq.sql", "sparse_column_msq.json"),
+ Arrays.asList("wikipedia_http_inputsource_msq.sql", "wikipedia_http_inputsource_queries.json"),
+ Arrays.asList("wikipedia_index_msq.sql", "wikipedia_index_queries.json"),
+ Arrays.asList("wikipedia_merge_index_task.sql", "wikipedia_index_queries.json"),
+ Arrays.asList("wikipedia_index_task_with_transform.sql", "wikipedia_index_queries_with_transform.json")
+ );
+
+ }
+
+ @Test
+ @Parameters(method = "test_cases")
+ public void testSQLBasedBatchIngestion(String sqlFileName, String queryFileName)
+ {
+ try {
+ runMSQTaskandTestQueries(BATCH_INDEX_TASKS_DIR + sqlFileName,
+ BATCH_INDEX_TASKS_DIR + queryFileName,
+ FilenameUtils.removeExtension(sqlFileName),
+ ImmutableMap.of("finalizeAggregations", false,
+ "maxNumTasks", 10,
Review Comment:
Should this be 3 ? I donot think we need so many slots for IT's
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] cryptoe commented on pull request #13374: Migrate current integration batch tests to equivalent MSQ tests
Posted by GitBox <gi...@apache.org>.
cryptoe commented on PR #13374:
URL: https://github.com/apache/druid/pull/13374#issuecomment-1320760129
Another nit we should fix in this PR :
```
2022-11-18T19:26:19,074 INFO [main] org.apache.druid.testing.utils.TestQueryHelper - Sql Task submitted with task Id - query-77b14a7e-999d-4756-92eb-9fba59f68836
2022-11-18T19:26:19,105 WARN [main] org.apache.druid.java.util.common.RetryUtils - Retrying (1 of 99) in 895ms.
org.apache.druid.testing.utils.MsqTestQueryHelper$TaskStillRunningException: null
at org.apache.druid.testing.utils.MsqTestQueryHelper.lambda$pollTaskIdForCompletion$0(MsqTestQueryHelper.java:163) ~[druid-integration-tests-25.0.0-SNAPSHOT.jar:25.0.0-SNAPSHOT]
at org.apache.druid.java.util.common.RetryUtils.retry(RetryUtils.java:129) ~[druid-core-25.0.0-SNAPSHOT.jar:25.0.0-SNAPSHOT]
at org.apache.druid.java.util.common.RetryUtils.retry(RetryUtils.java:81) ~[druid-core-25.0.0-SNAPSHOT.jar:25.0.0-SNAPSHOT]
at org.apache.druid.java.util.common.RetryUtils.retry(RetryUtils.java:163) ~[druid-core-25.0.0-SNAPSHOT.jar:25.0.0-SNAPSHOT]
at org.apache.druid.java.util.common.RetryUtils.retry(RetryUtils.java:153) ~[druid-core-25.0.0-SNAPSHOT.jar:25.0.0-SNAPSHOT]
```
```
public TaskState pollTaskIdForCompletion(String taskId) throws Exception
{
return RetryUtils.retry(
() -> {
TaskStatusPlus taskStatusPlus = overlordClient.getTaskStatus(taskId);
TaskState statusCode = taskStatusPlus.getStatusCode();
if (statusCode != null && statusCode.isComplete()) {
return taskStatusPlus.getStatusCode();
}
throw new TaskStillRunningException();
},
(Throwable t) -> t instanceof TaskStillRunningException,
100
);
}
```
let's change this method to not throw an exception but just log unable to get the status of the task.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] abhagraw commented on a diff in pull request #13374: Migrate current integration batch tests to equivalent MSQ tests
Posted by GitBox <gi...@apache.org>.
abhagraw commented on code in PR #13374:
URL: https://github.com/apache/druid/pull/13374#discussion_r1025757408
##########
integration-tests-ex/cases/src/test/resources/multi-stage-query/batch-index/wikipedia_index_msq.json:
##########
@@ -0,0 +1,4 @@
+{
+ "sqlTask" : "REPLACE INTO \"%%DATASOURCE%%\" OVERWRITE ALL\nWITH \"source\" as (SELECT * FROM TABLE(\n EXTERN(\n '{\"type\":\"local\",\"files\":[\"/resources/data/batch_index/json/wikipedia_index_data1.json\",\"/resources/data/batch_index/json/wikipedia_index_data2.json\",\"/resources/data/batch_index/json/wikipedia_index_data3.json\"]}',\n '{\"type\":\"json\"}',\n '[{\"name\":\"timestamp\",\"type\":\"string\"},{\"name\":\"page\",\"type\":\"string\"},{\"name\":\"language\",\"type\":\"string\"},{\"name\":\"user\",\"type\":\"string\"},{\"name\":\"unpatrolled\",\"type\":\"string\"},{\"name\":\"newPage\",\"type\":\"string\"},{\"name\":\"robot\",\"type\":\"string\"},{\"name\":\"anonymous\",\"type\":\"string\"},{\"name\":\"namespace\",\"type\":\"string\"},{\"name\":\"continent\",\"type\":\"string\"},{\"name\":\"country\",\"type\":\"string\"},{\"name\":\"region\",\"type\":\"string\"},{\"name\":\"city\",\"type\":\"string\"},{\"name\":\"added\",\"type\":\"double\"},{\"name\":\"de
leted\",\"type\":\"double\"},{\"name\":\"delta\",\"type\":\"double\"}]'\n )\n))\nSELECT\n TIME_FLOOR(CASE WHEN CAST(\"timestamp\" AS BIGINT) > 0 THEN MILLIS_TO_TIMESTAMP(CAST(\"timestamp\" AS BIGINT)) ELSE TIME_PARSE(\"timestamp\") END, 'PT1S') AS __time,\n \"page\",\n \"language\",\n \"user\",\n \"unpatrolled\",\n \"newPage\",\n \"robot\",\n \"anonymous\",\n \"namespace\",\n \"continent\",\n \"country\",\n \"region\",\n \"city\",\n COUNT(*) AS \"count\",\n SUM(\"added\") AS \"added\",\n SUM(\"deleted\") AS \"deleted\",\n SUM(\"delta\") AS \"delta\",\n APPROX_COUNT_DISTINCT_DS_THETA(\"user\") AS \"thetaSketch\",\n DS_QUANTILES_SKETCH(\"delta\") AS \"quantilesDoublesSketch\",\n APPROX_COUNT_DISTINCT_DS_HLL(\"user\") AS \"HLLSketchBuild\"\nFROM \"source\"\nGROUP BY 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13\nPARTITIONED BY DAY",
Review Comment:
json files does not support pretty multi-line string syntax.
ref - https://stackoverflow.com/a/52557585
Have changed structure, and now MSQ sql ingestion queries are placed in separate sql files which are more readable.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] abhagraw commented on a diff in pull request #13374: Migrate current integration batch tests to equivalent MSQ tests
Posted by GitBox <gi...@apache.org>.
abhagraw commented on code in PR #13374:
URL: https://github.com/apache/druid/pull/13374#discussion_r1026462629
##########
integration-tests-ex/cases/pom.xml:
##########
@@ -346,7 +351,7 @@
</goals>
<configuration>
<!-- our tests are very verbose, let's keep the volume down -->
- <redirectTestOutputToFile>true</redirectTestOutputToFile>
+ <redirectTestOutputToFile>False</redirectTestOutputToFile>
Review Comment:
Updated the comment to explain the change.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] abhagraw commented on a diff in pull request #13374: Migrate current integration batch tests to equivalent MSQ tests
Posted by GitBox <gi...@apache.org>.
abhagraw commented on code in PR #13374:
URL: https://github.com/apache/druid/pull/13374#discussion_r1026456680
##########
distribution/pom.xml:
##########
@@ -225,6 +225,8 @@
<argument>-c</argument>
<argument>org.apache.druid.extensions:druid-multi-stage-query</argument>
<argument>-c</argument>
+ <argument>org.apache.druid.extensions:druid-catalog</argument>
Review Comment:
The docker containers created for the test fail with the following error, if this extension is not added -
```Listening for transport dt_socket at address: 8000
2022-11-15T06:35:58,107 INFO [main] org.hibernate.validator.internal.util.Version - HV000001: Hibernate Validator 5.2.5.Final
Exception in thread "main" org.apache.druid.java.util.common.ISE: Extension [/usr/local/apache-druid-2022.12.0-iap-SNAPSHOT/extensions/druid-catalog] specified in "druid.extensions.loadList" didn't exist!?
at org.apache.druid.guice.ExtensionsLoader.getExtensionFilesToLoad(ExtensionsLoader.java:183)
at org.apache.druid.guice.ExtensionsLoader$ServiceLoadingFromExtensions.addAllFromFileSystem(ExtensionsLoader.java:292)
at org.apache.druid.guice.ExtensionsLoader$ServiceLoadingFromExtensions.<init>(ExtensionsLoader.java:280)
at org.apache.druid.guice.ExtensionsLoader$ServiceLoadingFromExtensions.<init>(ExtensionsLoader.java:268)
at org.apache.druid.guice.ExtensionsLoader.lambda$getFromExtensions$0(ExtensionsLoader.java:139)
at java.util.concurrent.ConcurrentHashMap.computeIfAbsent(ConcurrentHashMap.java:1660)
at org.apache.druid.guice.ExtensionsLoader.getFromExtensions(ExtensionsLoader.java:137)
at org.apache.druid.cli.Main.main(Main.java:98)```
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] cryptoe merged pull request #13374: Migrate current integration batch tests to equivalent MSQ tests
Posted by GitBox <gi...@apache.org>.
cryptoe merged PR #13374:
URL: https://github.com/apache/druid/pull/13374
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] cryptoe commented on a diff in pull request #13374: Migrate current integration batch tests to equivalent MSQ tests
Posted by GitBox <gi...@apache.org>.
cryptoe commented on code in PR #13374:
URL: https://github.com/apache/druid/pull/13374#discussion_r1026263091
##########
integration-tests-ex/cases/src/test/java/org/apache/druid/testsEx/msq/ITSQLBasedBatchIngestion.java:
##########
@@ -0,0 +1,63 @@
+/*
+ * 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.druid.testsEx.msq;
+
+import org.apache.curator.shaded.com.google.common.collect.ImmutableMap;
+import org.apache.druid.testsEx.categories.MultiStageQuery;
+import org.apache.druid.testsEx.config.DruidTestRunner;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.runner.RunWith;
+
+import java.io.File;
+import java.util.Arrays;
+
+@RunWith(DruidTestRunner.class)
+@Category(MultiStageQuery.class)
+public class ITSQLBasedBatchIngestion extends AbstractITSQLBasedBatchIngestion
+{
+ private static final String BATCH_INDEX_TASKS_DIR = "/multi-stage-query/batch-index/";
+
+ @Test
+ public void testSQLBasedBatchIngestion() throws Exception
+ {
+ // Get list of all directories in batch-index folder. Each folder is considered a test case.
+ File[] directories = (new File(getClass().getResource(BATCH_INDEX_TASKS_DIR).toURI())).listFiles(File::isDirectory);
+ int fail_count = 0;
+ LOG.info("Test will be run for sql in the following directories - \n %s", Arrays.toString(directories));
+ for (File dir : directories) {
+ try {
+ runMSQTaskandTestQueries(BATCH_INDEX_TASKS_DIR + dir.getName(), "msqBatchIndex_" + dir.getName(),
Review Comment:
Can we create a parameterized test?
That way we will know which test is exactly flaky
##########
integration-tests-ex/cases/src/test/java/org/apache/druid/testsEx/msq/AbstractITSQLBasedBatchIngestion.java:
##########
@@ -0,0 +1,125 @@
+/*
+ * 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.druid.testsEx.msq;
+
+import com.google.inject.Inject;
+import org.apache.commons.io.IOUtils;
+import org.apache.druid.java.util.common.ISE;
+import org.apache.druid.java.util.common.StringUtils;
+import org.apache.druid.java.util.common.logger.Logger;
+import org.apache.druid.testing.utils.DataLoaderHelper;
+import org.apache.druid.testing.utils.MsqTestQueryHelper;
+import org.apache.druid.testing.utils.TestQueryHelper;
+import org.apache.druid.testsEx.indexer.AbstractITBatchIndexTest;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.URISyntaxException;
+import java.nio.charset.StandardCharsets;
+import java.util.Map;
+
+public class AbstractITSQLBasedBatchIngestion
+{
+ public static final Logger LOG = new Logger(TestQueryHelper.class);
+ @Inject
+ private MsqTestQueryHelper msqHelper;
+
+ @Inject
+ protected TestQueryHelper queryHelper;
+
+ @Inject
+ private DataLoaderHelper dataLoaderHelper;
+
+ protected String getFileWithFormatFromDir(String dir, String format) throws URISyntaxException
+ {
+ File[] file = (new File(getClass().getResource(dir).toURI())).listFiles(pathname -> {
+ String name = pathname.getName();
+ return name.endsWith(format) && pathname.isFile();
+ });
+ return dir + '/' + file[0].getName();
+ }
+
+ protected String getSqlStringFromDir(String dir, String datasource) throws URISyntaxException
+ {
+ String filePath = getFileWithFormatFromDir(dir, ".sql");
+ String sqlString;
+ try {
+ InputStream is = AbstractITBatchIndexTest.class.getResourceAsStream(filePath);
+ sqlString = IOUtils.toString(is, StandardCharsets.UTF_8);
+ }
+ catch (IOException e) {
+ throw new ISE(e, "could not read query file: %s", filePath);
+ }
+
+ sqlString = StringUtils.replace(
+ sqlString,
+ "%%DATASOURCE%%",
+ datasource
+ );
+
+ return sqlString;
+ }
+
+ protected void doTestQuery(String dir, String dataSource) throws URISyntaxException
+ {
+ String queryFilePath = getFileWithFormatFromDir(dir, ".json");
+ try {
+ String queryResponseTemplate;
+ try {
+ InputStream is = AbstractITBatchIndexTest.class.getResourceAsStream(queryFilePath);
+ queryResponseTemplate = IOUtils.toString(is, StandardCharsets.UTF_8);
+ }
+ catch (IOException e) {
+ throw new ISE(e, "could not read query file: %s", queryFilePath);
+ }
+
+ queryResponseTemplate = StringUtils.replace(
+ queryResponseTemplate,
+ "%%DATASOURCE%%",
+ dataSource
+ );
+ queryHelper.testQueriesFromString(queryResponseTemplate);
+
+ }
+ catch (Exception e) {
+ LOG.error(e, "Error while running test query");
+ throw new RuntimeException(e);
+ }
+ }
+
+ protected void runMSQTaskandTestQueries(String relativePath, String datasource, Map<String, Object> msqContext) throws Exception
Review Comment:
lets java doc this method.
##########
integration-tests-ex/cases/src/test/java/org/apache/druid/testsEx/msq/AbstractITSQLBasedBatchIngestion.java:
##########
@@ -0,0 +1,125 @@
+/*
+ * 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.druid.testsEx.msq;
+
+import com.google.inject.Inject;
+import org.apache.commons.io.IOUtils;
+import org.apache.druid.java.util.common.ISE;
+import org.apache.druid.java.util.common.StringUtils;
+import org.apache.druid.java.util.common.logger.Logger;
+import org.apache.druid.testing.utils.DataLoaderHelper;
+import org.apache.druid.testing.utils.MsqTestQueryHelper;
+import org.apache.druid.testing.utils.TestQueryHelper;
+import org.apache.druid.testsEx.indexer.AbstractITBatchIndexTest;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.URISyntaxException;
+import java.nio.charset.StandardCharsets;
+import java.util.Map;
+
+public class AbstractITSQLBasedBatchIngestion
+{
+ public static final Logger LOG = new Logger(TestQueryHelper.class);
+ @Inject
+ private MsqTestQueryHelper msqHelper;
+
+ @Inject
+ protected TestQueryHelper queryHelper;
+
+ @Inject
+ private DataLoaderHelper dataLoaderHelper;
+
+ protected String getFileWithFormatFromDir(String dir, String format) throws URISyntaxException
+ {
+ File[] file = (new File(getClass().getResource(dir).toURI())).listFiles(pathname -> {
+ String name = pathname.getName();
+ return name.endsWith(format) && pathname.isFile();
+ });
+ return dir + '/' + file[0].getName();
+ }
+
+ protected String getSqlStringFromDir(String dir, String datasource) throws URISyntaxException
Review Comment:
We are doing the replacement for the data source. So we should mention that in the signature.
Lets javadoc this method.
##########
integration-tests-ex/cases/src/test/java/org/apache/druid/testsEx/msq/AbstractITSQLBasedBatchIngestion.java:
##########
@@ -0,0 +1,125 @@
+/*
+ * 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.druid.testsEx.msq;
+
+import com.google.inject.Inject;
+import org.apache.commons.io.IOUtils;
+import org.apache.druid.java.util.common.ISE;
+import org.apache.druid.java.util.common.StringUtils;
+import org.apache.druid.java.util.common.logger.Logger;
+import org.apache.druid.testing.utils.DataLoaderHelper;
+import org.apache.druid.testing.utils.MsqTestQueryHelper;
+import org.apache.druid.testing.utils.TestQueryHelper;
+import org.apache.druid.testsEx.indexer.AbstractITBatchIndexTest;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.URISyntaxException;
+import java.nio.charset.StandardCharsets;
+import java.util.Map;
+
+public class AbstractITSQLBasedBatchIngestion
+{
+ public static final Logger LOG = new Logger(TestQueryHelper.class);
+ @Inject
+ private MsqTestQueryHelper msqHelper;
+
+ @Inject
+ protected TestQueryHelper queryHelper;
+
+ @Inject
+ private DataLoaderHelper dataLoaderHelper;
+
+ protected String getFileWithFormatFromDir(String dir, String format) throws URISyntaxException
+ {
+ File[] file = (new File(getClass().getResource(dir).toURI())).listFiles(pathname -> {
+ String name = pathname.getName();
+ return name.endsWith(format) && pathname.isFile();
+ });
+ return dir + '/' + file[0].getName();
+ }
+
+ protected String getSqlStringFromDir(String dir, String datasource) throws URISyntaxException
+ {
+ String filePath = getFileWithFormatFromDir(dir, ".sql");
+ String sqlString;
+ try {
+ InputStream is = AbstractITBatchIndexTest.class.getResourceAsStream(filePath);
+ sqlString = IOUtils.toString(is, StandardCharsets.UTF_8);
+ }
+ catch (IOException e) {
+ throw new ISE(e, "could not read query file: %s", filePath);
+ }
+
+ sqlString = StringUtils.replace(
+ sqlString,
+ "%%DATASOURCE%%",
+ datasource
+ );
+
+ return sqlString;
+ }
+
+ protected void doTestQuery(String dir, String dataSource) throws URISyntaxException
Review Comment:
this as well since it will be used in all tests.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] abhagraw commented on a diff in pull request #13374: Migrate current integration batch tests to equivalent MSQ tests
Posted by GitBox <gi...@apache.org>.
abhagraw commented on code in PR #13374:
URL: https://github.com/apache/druid/pull/13374#discussion_r1027039416
##########
integration-tests-ex/cases/src/test/java/org/apache/druid/testsEx/msq/ITSQLBasedBatchIngestion.java:
##########
@@ -0,0 +1,70 @@
+/*
+ * 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.druid.testsEx.msq;
+
+import junitparams.Parameters;
+import org.apache.commons.io.FilenameUtils;
+import org.apache.curator.shaded.com.google.common.collect.ImmutableMap;
+import org.apache.druid.testsEx.categories.MultiStageQuery;
+import org.apache.druid.testsEx.config.DruidTestRunner;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.runner.RunWith;
+
+import java.util.Arrays;
+import java.util.List;
+
+@RunWith(DruidTestRunner.class)
+@Category(MultiStageQuery.class)
+public class ITSQLBasedBatchIngestion extends AbstractITSQLBasedBatchIngestion
+{
+ private static final String BATCH_INDEX_TASKS_DIR = "/multi-stage-query/batch-index/";
+
+ public static List<List<String>> test_cases()
+ {
+ return Arrays.asList(
+ Arrays.asList("msq_inline.sql", "json_path_index_queries.json"),
+ Arrays.asList("sparse_column_msq.sql", "sparse_column_msq.json"),
+ Arrays.asList("wikipedia_http_inputsource_msq.sql", "wikipedia_http_inputsource_queries.json"),
+ Arrays.asList("wikipedia_index_msq.sql", "wikipedia_index_queries.json"),
+ Arrays.asList("wikipedia_merge_index_task.sql", "wikipedia_index_queries.json"),
+ Arrays.asList("wikipedia_index_task_with_transform.sql", "wikipedia_index_queries_with_transform.json")
+ );
+
+ }
+
+ @Test
+ @Parameters(method = "test_cases")
+ public void testSQLBasedBatchIngestion(String sqlFileName, String queryFileName)
+ {
+ try {
+ runMSQTaskandTestQueries(BATCH_INDEX_TASKS_DIR + sqlFileName,
+ BATCH_INDEX_TASKS_DIR + queryFileName,
+ FilenameUtils.removeExtension(sqlFileName),
+ ImmutableMap.of("finalizeAggregations", false,
+ "maxNumTasks", 10,
Review Comment:
Reduced the maxNumTasks to 5
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] cryptoe commented on a diff in pull request #13374: Migrate current integration batch tests to equivalent MSQ tests
Posted by GitBox <gi...@apache.org>.
cryptoe commented on code in PR #13374:
URL: https://github.com/apache/druid/pull/13374#discussion_r1024776864
##########
integration-tests-ex/cases/src/test/resources/multi-stage-query/batch-index/wikipedia_index_msq.json:
##########
@@ -0,0 +1,4 @@
+{
+ "sqlTask" : "REPLACE INTO \"%%DATASOURCE%%\" OVERWRITE ALL\nWITH \"source\" as (SELECT * FROM TABLE(\n EXTERN(\n '{\"type\":\"local\",\"files\":[\"/resources/data/batch_index/json/wikipedia_index_data1.json\",\"/resources/data/batch_index/json/wikipedia_index_data2.json\",\"/resources/data/batch_index/json/wikipedia_index_data3.json\"]}',\n '{\"type\":\"json\"}',\n '[{\"name\":\"timestamp\",\"type\":\"string\"},{\"name\":\"page\",\"type\":\"string\"},{\"name\":\"language\",\"type\":\"string\"},{\"name\":\"user\",\"type\":\"string\"},{\"name\":\"unpatrolled\",\"type\":\"string\"},{\"name\":\"newPage\",\"type\":\"string\"},{\"name\":\"robot\",\"type\":\"string\"},{\"name\":\"anonymous\",\"type\":\"string\"},{\"name\":\"namespace\",\"type\":\"string\"},{\"name\":\"continent\",\"type\":\"string\"},{\"name\":\"country\",\"type\":\"string\"},{\"name\":\"region\",\"type\":\"string\"},{\"name\":\"city\",\"type\":\"string\"},{\"name\":\"added\",\"type\":\"double\"},{\"name\":\"de
leted\",\"type\":\"double\"},{\"name\":\"delta\",\"type\":\"double\"}]'\n )\n))\nSELECT\n TIME_FLOOR(CASE WHEN CAST(\"timestamp\" AS BIGINT) > 0 THEN MILLIS_TO_TIMESTAMP(CAST(\"timestamp\" AS BIGINT)) ELSE TIME_PARSE(\"timestamp\") END, 'PT1S') AS __time,\n \"page\",\n \"language\",\n \"user\",\n \"unpatrolled\",\n \"newPage\",\n \"robot\",\n \"anonymous\",\n \"namespace\",\n \"continent\",\n \"country\",\n \"region\",\n \"city\",\n COUNT(*) AS \"count\",\n SUM(\"added\") AS \"added\",\n SUM(\"deleted\") AS \"deleted\",\n SUM(\"delta\") AS \"delta\",\n APPROX_COUNT_DISTINCT_DS_THETA(\"user\") AS \"thetaSketch\",\n DS_QUANTILES_SKETCH(\"delta\") AS \"quantilesDoublesSketch\",\n APPROX_COUNT_DISTINCT_DS_HLL(\"user\") AS \"HLLSketchBuild\"\nFROM \"source\"\nGROUP BY 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13\nPARTITIONED BY DAY",
Review Comment:
Is it possible to prettify this long str somehow ?
##########
integration-tests/src/main/java/org/apache/druid/testing/utils/MsqTestQueryHelper.java:
##########
@@ -95,7 +95,7 @@ public String getQueryURL(String schemeAndHost)
*/
public SqlTaskStatus submitMsqTask(String sqlQueryString) throws ExecutionException, InterruptedException
{
- return submitMsqTask(new SqlQuery(sqlQueryString, null, false, false, false, ImmutableMap.of(), null));
+ return submitMsqTask(new SqlQuery(sqlQueryString, null, false, false, false, ImmutableMap.of("finalizeAggregations", false), null));
Review Comment:
Long fix would be change it to a sql query builder.
The short fix would be to have the context param as part of the function argument.
I donot think its correct to have "finalizeAggregations", false as default
##########
integration-tests-ex/cases/src/test/java/org/apache/druid/testsEx/msq/ITMSQForBatchIndex.java:
##########
@@ -0,0 +1,144 @@
+/*
+ * 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.druid.testsEx.msq;
+
+import com.fasterxml.jackson.core.type.TypeReference;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.google.inject.Inject;
+import org.apache.commons.io.IOUtils;
+import org.apache.druid.java.util.common.ISE;
+import org.apache.druid.java.util.common.StringUtils;
+import org.apache.druid.java.util.common.logger.Logger;
+import org.apache.druid.msq.sql.SqlTaskStatus;
+import org.apache.druid.testing.IntegrationTestingConfig;
+import org.apache.druid.testing.clients.CoordinatorResourceTestClient;
+import org.apache.druid.testing.utils.DataLoaderHelper;
+import org.apache.druid.testing.utils.MsqSqlTasksWithTestQueries;
+import org.apache.druid.testing.utils.MsqTestQueryHelper;
+import org.apache.druid.testing.utils.TestQueryHelper;
+import org.apache.druid.testsEx.categories.MultiStageQuery;
+import org.apache.druid.testsEx.config.DruidTestRunner;
+import org.apache.druid.testsEx.indexer.AbstractITBatchIndexTest;
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.runner.RunWith;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
+import java.util.Arrays;
+
+@RunWith(DruidTestRunner.class)
+@Category(MultiStageQuery.class)
+public class ITMSQForBatchIndex
Review Comment:
NIt: ITSQLBasedBatchIngestion
##########
integration-tests-ex/cases/src/test/resources/multi-stage-query/batch-index/wikipedia_index_msq.json:
##########
@@ -0,0 +1,4 @@
+{
+ "sqlTask" : "REPLACE INTO \"%%DATASOURCE%%\" OVERWRITE ALL\nWITH \"source\" as (SELECT * FROM TABLE(\n EXTERN(\n '{\"type\":\"local\",\"files\":[\"/resources/data/batch_index/json/wikipedia_index_data1.json\",\"/resources/data/batch_index/json/wikipedia_index_data2.json\",\"/resources/data/batch_index/json/wikipedia_index_data3.json\"]}',\n '{\"type\":\"json\"}',\n '[{\"name\":\"timestamp\",\"type\":\"string\"},{\"name\":\"page\",\"type\":\"string\"},{\"name\":\"language\",\"type\":\"string\"},{\"name\":\"user\",\"type\":\"string\"},{\"name\":\"unpatrolled\",\"type\":\"string\"},{\"name\":\"newPage\",\"type\":\"string\"},{\"name\":\"robot\",\"type\":\"string\"},{\"name\":\"anonymous\",\"type\":\"string\"},{\"name\":\"namespace\",\"type\":\"string\"},{\"name\":\"continent\",\"type\":\"string\"},{\"name\":\"country\",\"type\":\"string\"},{\"name\":\"region\",\"type\":\"string\"},{\"name\":\"city\",\"type\":\"string\"},{\"name\":\"added\",\"type\":\"double\"},{\"name\":\"de
leted\",\"type\":\"double\"},{\"name\":\"delta\",\"type\":\"double\"}]'\n )\n))\nSELECT\n TIME_FLOOR(CASE WHEN CAST(\"timestamp\" AS BIGINT) > 0 THEN MILLIS_TO_TIMESTAMP(CAST(\"timestamp\" AS BIGINT)) ELSE TIME_PARSE(\"timestamp\") END, 'PT1S') AS __time,\n \"page\",\n \"language\",\n \"user\",\n \"unpatrolled\",\n \"newPage\",\n \"robot\",\n \"anonymous\",\n \"namespace\",\n \"continent\",\n \"country\",\n \"region\",\n \"city\",\n COUNT(*) AS \"count\",\n SUM(\"added\") AS \"added\",\n SUM(\"deleted\") AS \"deleted\",\n SUM(\"delta\") AS \"delta\",\n APPROX_COUNT_DISTINCT_DS_THETA(\"user\") AS \"thetaSketch\",\n DS_QUANTILES_SKETCH(\"delta\") AS \"quantilesDoublesSketch\",\n APPROX_COUNT_DISTINCT_DS_HLL(\"user\") AS \"HLLSketchBuild\"\nFROM \"source\"\nGROUP BY 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13\nPARTITIONED BY DAY",
+ "testQueriesFilePath" : "/multi-stage-query/batch-index/test-queries/wikipedia_index_msq_queries.json"
Review Comment:
So if every test case has the same q's then it makes sense to create a new file else I would add the native q's here directly.
##########
integration-tests-ex/cases/src/test/java/org/apache/druid/testsEx/msq/ITMSQForBatchIndex.java:
##########
@@ -0,0 +1,144 @@
+/*
+ * 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.druid.testsEx.msq;
+
+import com.fasterxml.jackson.core.type.TypeReference;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.google.inject.Inject;
+import org.apache.commons.io.IOUtils;
+import org.apache.druid.java.util.common.ISE;
+import org.apache.druid.java.util.common.StringUtils;
+import org.apache.druid.java.util.common.logger.Logger;
+import org.apache.druid.msq.sql.SqlTaskStatus;
+import org.apache.druid.testing.IntegrationTestingConfig;
+import org.apache.druid.testing.clients.CoordinatorResourceTestClient;
+import org.apache.druid.testing.utils.DataLoaderHelper;
+import org.apache.druid.testing.utils.MsqSqlTasksWithTestQueries;
+import org.apache.druid.testing.utils.MsqTestQueryHelper;
+import org.apache.druid.testing.utils.TestQueryHelper;
+import org.apache.druid.testsEx.categories.MultiStageQuery;
+import org.apache.druid.testsEx.config.DruidTestRunner;
+import org.apache.druid.testsEx.indexer.AbstractITBatchIndexTest;
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.runner.RunWith;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
+import java.util.Arrays;
+
+@RunWith(DruidTestRunner.class)
+@Category(MultiStageQuery.class)
+public class ITMSQForBatchIndex
+{
+ public static final Logger LOG = new Logger(TestQueryHelper.class);
+ @Inject
+ private MsqTestQueryHelper msqHelper;
+
+ @Inject
+ protected TestQueryHelper queryHelper;
+
+ @Inject
+ private IntegrationTestingConfig config;
+
+ @Inject
+ private ObjectMapper jsonMapper;
+
+ @Inject
+ private DataLoaderHelper dataLoaderHelper;
+
+ @Inject
+ private CoordinatorResourceTestClient coordinatorClient;
+
+ private static final String BATCH_INDEX_TASKS_DIR = "/multi-stage-query/batch-index/";
+
+ protected void doTestQuery(String dataSource, String queryFilePath)
+ {
+ try {
+ String queryResponseTemplate;
+ try {
+ InputStream is = AbstractITBatchIndexTest.class.getResourceAsStream(queryFilePath);
+ queryResponseTemplate = IOUtils.toString(is, StandardCharsets.UTF_8);
+ }
+ catch (IOException e) {
+ throw new ISE(e, "could not read query file: %s", queryFilePath);
+ }
+
+ queryResponseTemplate = StringUtils.replace(
+ queryResponseTemplate,
+ "%%DATASOURCE%%",
+ dataSource
+ );
+ queryHelper.testQueriesFromString(queryResponseTemplate);
+
+ }
+ catch (Exception e) {
+ LOG.error(e, "Error while testing");
+ throw new RuntimeException(e);
+ }
+ }
+
+ @Test
+ public void testMsqIngestionForBatchIndexTasks() throws Exception
+ {
+ File[] files = (new File(getClass().getResource(BATCH_INDEX_TASKS_DIR).toURI())).listFiles();
+ LOG.info(Arrays.toString(files));
+ String datasource = "dst";
+ for (int i = 1; i < files.length; i++) {
+ LOG.info("Starting MSQ test for [%s]", files[i]);
+ MsqSqlTasksWithTestQueries batchSqlTasksAndQueries =
+ jsonMapper.readValue(
+ TestQueryHelper.class.getResourceAsStream(BATCH_INDEX_TASKS_DIR + files[i].getName()),
+ new TypeReference<MsqSqlTasksWithTestQueries>()
+ {
+ }
+ );
+ // Clear up the datasource from the previous runs
+ coordinatorClient.unloadSegmentsForDataSource(datasource + files[i].getName());
+
+ String sqlTask = StringUtils.replace(
+ batchSqlTasksAndQueries.getSqlTask(),
+ "%%DATASOURCE%%",
+ datasource + files[i].getName()
+ );
+
+ LOG.info("SqlTask - \n %s", sqlTask);
+
+ // Submit the tasks and wait for the datasource to get loaded
+ SqlTaskStatus sqlTaskStatus = msqHelper.submitMsqTask(sqlTask);
+
+ LOG.info("Sql Task submitted with task Id - %s", sqlTaskStatus.getTaskId());
+
+ if (sqlTaskStatus.getState().isFailure()) {
+ Assert.fail(StringUtils.format(
+ "Unable to start the task successfully.\nPossible exception: %s",
+ sqlTaskStatus.getError()
+ ));
+ }
+ msqHelper.pollTaskIdForCompletion(sqlTaskStatus.getTaskId());
+ dataLoaderHelper.waitUntilDatasourceIsReady(datasource + files[i].getName());
Review Comment:
I think this should be pushed inside msqHelper or a base test class. It would most likely get copied in every test case.
##########
integration-tests/src/main/java/org/apache/druid/testing/utils/MsqSqlTasksWithTestQueries.java:
##########
@@ -0,0 +1,52 @@
+/*
+ * 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.druid.testing.utils;
+
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonProperty;
+
+public class MsqSqlTasksWithTestQueries
+{
+ private final String sqlTask;
+ private final String testQueriesFilePath;
+
+ @JsonCreator
+ public MsqSqlTasksWithTestQueries(
Review Comment:
I think this class is not needed.
checkout : ITMultiStageQuery
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] abhagraw commented on a diff in pull request #13374: Migrate current integration batch tests to equivalent MSQ tests
Posted by GitBox <gi...@apache.org>.
abhagraw commented on code in PR #13374:
URL: https://github.com/apache/druid/pull/13374#discussion_r1026453689
##########
integration-tests-ex/cases/src/test/java/org/apache/druid/testsEx/msq/ITSQLBasedBatchIngestion.java:
##########
@@ -0,0 +1,63 @@
+/*
+ * 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.druid.testsEx.msq;
+
+import org.apache.curator.shaded.com.google.common.collect.ImmutableMap;
+import org.apache.druid.testsEx.categories.MultiStageQuery;
+import org.apache.druid.testsEx.config.DruidTestRunner;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.runner.RunWith;
+
+import java.io.File;
+import java.util.Arrays;
+
+@RunWith(DruidTestRunner.class)
+@Category(MultiStageQuery.class)
+public class ITSQLBasedBatchIngestion extends AbstractITSQLBasedBatchIngestion
+{
+ private static final String BATCH_INDEX_TASKS_DIR = "/multi-stage-query/batch-index/";
+
+ @Test
+ public void testSQLBasedBatchIngestion() throws Exception
+ {
+ // Get list of all directories in batch-index folder. Each folder is considered a test case.
+ File[] directories = (new File(getClass().getResource(BATCH_INDEX_TASKS_DIR).toURI())).listFiles(File::isDirectory);
+ int fail_count = 0;
+ LOG.info("Test will be run for sql in the following directories - \n %s", Arrays.toString(directories));
+ for (File dir : directories) {
+ try {
+ runMSQTaskandTestQueries(BATCH_INDEX_TASKS_DIR + dir.getName(), "msqBatchIndex_" + dir.getName(),
Review Comment:
Have parameterized the tests now. All the SQL ingestion task and test queries have been moved in the same folder.
A testcase would a be a pair of SQL ingestion task filename and test queries filename.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] abhagraw commented on a diff in pull request #13374: Migrate current integration batch tests to equivalent MSQ tests
Posted by GitBox <gi...@apache.org>.
abhagraw commented on code in PR #13374:
URL: https://github.com/apache/druid/pull/13374#discussion_r1026456680
##########
distribution/pom.xml:
##########
@@ -225,6 +225,8 @@
<argument>-c</argument>
<argument>org.apache.druid.extensions:druid-multi-stage-query</argument>
<argument>-c</argument>
+ <argument>org.apache.druid.extensions:druid-catalog</argument>
Review Comment:
The docker containers created for the test fail with the following error, if this extension is not added -
`Listening for transport dt_socket at address: 8000
2022-11-15T06:35:58,107 INFO [main] org.hibernate.validator.internal.util.Version - HV000001: Hibernate Validator 5.2.5.Final
Exception in thread "main" org.apache.druid.java.util.common.ISE: Extension [/usr/local/apache-druid-2022.12.0-iap-SNAPSHOT/extensions/druid-catalog] specified in "druid.extensions.loadList" didn't exist!?
at org.apache.druid.guice.ExtensionsLoader.getExtensionFilesToLoad(ExtensionsLoader.java:183)
at org.apache.druid.guice.ExtensionsLoader$ServiceLoadingFromExtensions.addAllFromFileSystem(ExtensionsLoader.java:292)
at org.apache.druid.guice.ExtensionsLoader$ServiceLoadingFromExtensions.<init>(ExtensionsLoader.java:280)
at org.apache.druid.guice.ExtensionsLoader$ServiceLoadingFromExtensions.<init>(ExtensionsLoader.java:268)
at org.apache.druid.guice.ExtensionsLoader.lambda$getFromExtensions$0(ExtensionsLoader.java:139)
at java.util.concurrent.ConcurrentHashMap.computeIfAbsent(ConcurrentHashMap.java:1660)
at org.apache.druid.guice.ExtensionsLoader.getFromExtensions(ExtensionsLoader.java:137)
at org.apache.druid.cli.Main.main(Main.java:98)`
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] LakshSingla commented on a diff in pull request #13374: Migrate current integration batch tests to equivalent MSQ tests
Posted by GitBox <gi...@apache.org>.
LakshSingla commented on code in PR #13374:
URL: https://github.com/apache/druid/pull/13374#discussion_r1026277283
##########
integration-tests-ex/cases/pom.xml:
##########
@@ -346,7 +351,7 @@
</goals>
<configuration>
<!-- our tests are very verbose, let's keep the volume down -->
- <redirectTestOutputToFile>true</redirectTestOutputToFile>
+ <redirectTestOutputToFile>False</redirectTestOutputToFile>
Review Comment:
Why is this change required? Also, if we go ahead with the change, then the comment above this should be updated to explain why it is set to false
```suggestion
<redirectTestOutputToFile>false</redirectTestOutputToFile>
```
##########
integration-tests-ex/cases/src/test/java/org/apache/druid/testsEx/msq/AbstractITSQLBasedBatchIngestion.java:
##########
@@ -0,0 +1,125 @@
+/*
+ * 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.druid.testsEx.msq;
+
+import com.google.inject.Inject;
+import org.apache.commons.io.IOUtils;
+import org.apache.druid.java.util.common.ISE;
+import org.apache.druid.java.util.common.StringUtils;
+import org.apache.druid.java.util.common.logger.Logger;
+import org.apache.druid.testing.utils.DataLoaderHelper;
+import org.apache.druid.testing.utils.MsqTestQueryHelper;
+import org.apache.druid.testing.utils.TestQueryHelper;
+import org.apache.druid.testsEx.indexer.AbstractITBatchIndexTest;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.URISyntaxException;
+import java.nio.charset.StandardCharsets;
+import java.util.Map;
+
+public class AbstractITSQLBasedBatchIngestion
+{
+ public static final Logger LOG = new Logger(TestQueryHelper.class);
+ @Inject
+ private MsqTestQueryHelper msqHelper;
+
+ @Inject
+ protected TestQueryHelper queryHelper;
+
+ @Inject
+ private DataLoaderHelper dataLoaderHelper;
+
+ protected String getFileWithFormatFromDir(String dir, String format) throws URISyntaxException
Review Comment:
What's the advantage of the method if it is returning just the first file ending with `format`. Should the user pass the filename himself?
##########
distribution/pom.xml:
##########
@@ -225,6 +225,8 @@
<argument>-c</argument>
<argument>org.apache.druid.extensions:druid-multi-stage-query</argument>
<argument>-c</argument>
+ <argument>org.apache.druid.extensions:druid-catalog</argument>
Review Comment:
Is this change related to this PR? If not, then should this be raised as a separate PR?
##########
integration-tests-ex/cases/src/test/java/org/apache/druid/testsEx/msq/ITSQLBasedBatchIngestion.java:
##########
@@ -0,0 +1,63 @@
+/*
+ * 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.druid.testsEx.msq;
+
+import org.apache.curator.shaded.com.google.common.collect.ImmutableMap;
+import org.apache.druid.testsEx.categories.MultiStageQuery;
+import org.apache.druid.testsEx.config.DruidTestRunner;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.runner.RunWith;
+
+import java.io.File;
+import java.util.Arrays;
+
+@RunWith(DruidTestRunner.class)
+@Category(MultiStageQuery.class)
+public class ITSQLBasedBatchIngestion extends AbstractITSQLBasedBatchIngestion
+{
+ private static final String BATCH_INDEX_TASKS_DIR = "/multi-stage-query/batch-index/";
+
+ @Test
+ public void testSQLBasedBatchIngestion() throws Exception
+ {
+ // Get list of all directories in batch-index folder. Each folder is considered a test case.
+ File[] directories = (new File(getClass().getResource(BATCH_INDEX_TASKS_DIR).toURI())).listFiles(File::isDirectory);
+ int fail_count = 0;
+ LOG.info("Test will be run for sql in the following directories - \n %s", Arrays.toString(directories));
+ for (File dir : directories) {
+ try {
+ runMSQTaskandTestQueries(BATCH_INDEX_TASKS_DIR + dir.getName(), "msqBatchIndex_" + dir.getName(),
Review Comment:
I agree with @cryptoe. Either we should pass the filename individually in each test to a standard helper method or create parameterized tests. Extracting filenames from the dir in a test makes it harder to debug failing tests, and also are difficult to pinpoint why the tests are failing clearly.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org