Migrate current integration batch tests to equivalent MSQ tests#13374
Merged
cryptoe merged 10 commits intoapache:masterfrom Nov 21, 2022
Merged
Migrate current integration batch tests to equivalent MSQ tests#13374cryptoe merged 10 commits intoapache:masterfrom
cryptoe merged 10 commits intoapache:masterfrom
Conversation
added 3 commits
November 16, 2022 17:57
kfaraz
added
Area - Testing
Area - MSQ
cryptoe
reviewed
Nov 17, 2022
cryptoe
reviewed
Nov 18, 2022
LakshSingla
reviewed
Nov 18, 2022
| <argument>-c</argument> | ||
| <argument>org.apache.druid.extensions:druid-multi-stage-query</argument> | ||
| <argument>-c</argument> | ||
| <argument>org.apache.druid.extensions:druid-catalog</argument> |
Contributor
There was a problem hiding this comment.
Is this change related to this PR? If not, then should this be raised as a separate PR?
Contributor
Author
There was a problem hiding this comment.
The docker containers created for the test fail with the following error, if this extension is not added -
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)```
cryptoe
reviewed
Nov 19, 2022
Contributor
cryptoe
left a comment
cryptoe
left a comment
There was a problem hiding this comment.
Some comments.
LGTM post they are addressed and CICD is 🟢
Contributor
|
Another nit we should fix in this PR : 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. |
Contributor
Author
Used quite retry - so that not all retries throw the exception + stacktrace. |
cryptoe
pushed a commit
to cryptoe/druid
that referenced
this pull request
Nov 28, 2022
…he#13374) * Migrate current integration batch tests to equivalent MSQ tests using new IT framework * Fix build issues * Trigger Build * Adding more tests and addressing comments * fixBuildIssues * fix dependency issues * Parameterized the test and addressed comments * Addressing comments * fixing checkstyle errors * Adressing comments
cryptoe
added a commit
that referenced
this pull request
Jan 11, 2023
* Initial commit. * Fixing error message in retry exceeded exception * Cleaning up some code * Adding some test cases. * Adding java docs. * Finishing up state test cases. * Adding some more java docs and fixing spot bugs, intellij inspections * Fixing intellij inspections and added tests * Documenting error codes * Migrate current integration batch tests to equivalent MSQ tests (#13374) * Migrate current integration batch tests to equivalent MSQ tests using new IT framework * Fix build issues * Trigger Build * Adding more tests and addressing comments * fixBuildIssues * fix dependency issues * Parameterized the test and addressed comments * Addressing comments * fixing checkstyle errors * Adressing comments * Adding ITTest which kills the worker abruptly * Review comments phase one * Adding doc changes * Adjusting for single threaded execution. * Adding Sequential Merge PR state handling * Merge things * Fixing checkstyle. * Adding new context param for fault tolerance. Adding stale task handling in sketchFetcher. Adding UT's. * Merge things * Merge things * Adding parameterized tests Created separate module for faultToleranceTests * Adding missed files * Review comments and fixing tests. * Documentation things. * Fixing IT * Controller impl fix. * Fixing racy WorkerSketchFetcherTest.java exception handling. Co-authored-by: abhagraw <99210446+abhagraw@users.noreply.github.com> Co-authored-by: Karan Kumar <cryptoe@karans-mbp.lan>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.