Made changes to build.gradle for doctest and integ-test to make stop Prometheus #270

MitchellGale · 2023-05-29T22:46:51Z

Description

This stops any prometheus server before doctests and integ tests are run. Prometheus is stopped after a successful execution of the program, but when a failure occurs, it does not clean up prometheus.

Steps to reproduce error that is fixed.

Run an OpenSearch instance
Run build in OpenSearch SQL
Observe that OpenSearch SQL crashes due to step 1
Stop OpenSearch instance started in step 1
Re-run build for OpenSearch SQL
Observe that there is error about Prometheus already running.

Same steps can be taken working from this PR branch and it will not show the error in step 6.

Issues Resolved

1083

Check List

New functionality includes testing.
- All tests pass, including unit test, integration test and doctest
New functionality has been documented.
- New functionality has javadoc added
- New functionality has user manual doc added
Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

codecov · 2023-05-29T23:02:00Z

Codecov Report

Merging #270 (4bc72c6) into Integ/Fix-StopPrometheus (4522422) will increase coverage by 0.14%.
The diff coverage is 100.00%.

❗ Current head 4bc72c6 differs from pull request most recent head 630a715. Consider uploading reports for the commit 630a715 to get more accurate results

@@                      Coverage Diff                       @@
##             Integ/Fix-StopPrometheus     #270      +/-   ##
==============================================================
+ Coverage                       97.16%   97.30%   +0.14%     
- Complexity                       4116     4326     +210     
==============================================================
  Files                             371      385      +14     
  Lines                           10365    10807     +442     
  Branches                          704      762      +58     
==============================================================
+ Hits                            10071    10516     +445     
+ Misses                            287      284       -3     
  Partials                            7        7

Flag	Coverage Δ
sql-engine	`97.30% <100.00%> (+0.14%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...ch/sql/planner/optimizer/LogicalPlanOptimizer.java	`100.00% <ø> (ø)`
...pensearch/sql/planner/physical/FilterOperator.java	`100.00% <ø> (ø)`
...pensearch/sql/planner/physical/NestedOperator.java	`100.00% <ø> (ø)`
...java/org/opensearch/sql/storage/StorageEngine.java	`100.00% <ø> (ø)`
...rc/main/java/org/opensearch/sql/storage/Table.java	`100.00% <ø> (ø)`
...ch/sql/opensearch/response/OpenSearchResponse.java	`100.00% <ø> (ø)`
...ql/opensearch/storage/OpenSearchStorageEngine.java	`100.00% <ø> (ø)`
...ge/script/aggregation/AggregationQueryBuilder.java	`100.00% <ø> (ø)`
...ain/java/org/opensearch/sql/analysis/Analyzer.java	`100.00% <100.00%> (ø)`
...rg/opensearch/sql/exception/NoCursorException.java	`100.00% <100.00%> (ø)`
... and 61 more

... and 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

MaxKsyunz · 2023-05-31T19:31:13Z

doctest/build.gradle

+            logger.quiet "No Prometheus server running!"
+        } else {
+            def pid = pidFile.text
+            def process = "kill $pid".execute()


Will this work on all platforms, namely Windows?

It should, it's using the same logic to stop prometheus that already existed.

Ah, I see it's a copy of the stopPrometheus task, right?

And you can't call it from here because that creates a circular dependency?

MaxKsyunz · 2023-05-31T19:34:38Z

doctest/build.gradle

+                file("$projectDir/bin/prometheus").deleteDir()
+                file("$projectDir/bin/prometheus.tar.gz").delete()


Does this cause Prometheus to be downloaded again immediately after?

If it exists and would have been deleted had the previous run been successful, it will delete it then download it again after. Do you suggest if it exists we just go with the previously running version? I worried that if there were changes made to it, if it was not deleted it could create issues in testing.

Yury-Fridlyand · 2023-05-31T19:46:16Z

doctest/build.gradle

+                file("$projectDir/bin/prometheus.tar.gz").delete()
+            }
+        }
+
        download.run {
            src getPrometheusBinaryLocation()
            dest new File("$projectDir/bin", 'prometheus.tar.gz')


Suggested change

dest new File("$projectDir/bin", 'prometheus.tar.gz')

dest new File("$projectDir/bin", 'prometheus.tar.gz')

overwrite false

What will this change do?

It should prevent redownloading

I see, because the .tar.gz should already exist? I assume I should delete line 49 in that case?

Right. I think we shouldn't delete and shouldn't re-download it.

If it does not re-download Prometheus, how would a developer get a newer version of it?

The version is hardcoded. CI will always download the file, because it starts with a clean VM.

MaxKsyunz · 2023-05-31T19:47:41Z

doctest/build.gradle

+            def process = "kill $pid".execute()
+
+            try {
+                process.waitFor()


Do we not want to check result?

Normally, this is crucial when spawning other processes. Is "we tried, I hope it worked" sufficient in this case?

What should happen if it was not successful? I think now it'll produce an error.

MaxKsyunz · 2023-05-31T22:41:00Z

doctest/build.gradle

-if(getOSFamilyType() != "windows") {
-    stopPrometheus.mustRunAfter startPrometheus
-    startOpenSearch.dependsOn startPrometheus
-    stopOpenSearch.finalizedBy stopPrometheus
-}


Isn't this still necessary?

startOpenSearch needs startPrometheus because there are some integration tests that use both.
stopPrometheus needs to be run after stopOpenSearch.

Yury-Fridlyand · 2023-06-12T20:00:25Z

Can you rebase integ branch to clean up commit history?

Signed-off-by: Derek Ho <[email protected]> Signed-off-by: Mitchell Gale <[email protected]>

* Changed gradle version and removed values iterator Signed-off-by: Guian Gumpac <[email protected]> * Update a test to match new indexResponse.aliases() type. Signed-off-by: MaxKsyunz <[email protected]> * Ran ./gradlew wrapper Signed-off-by: Guian Gumpac <[email protected]> --------- Signed-off-by: Guian Gumpac <[email protected]> Signed-off-by: MaxKsyunz <[email protected]> Co-authored-by: MaxKsyunz <[email protected]> Signed-off-by: Mitchell Gale <[email protected]>

* Keep up with refactoring in OpenSearch. Signed-off-by: MaxKsyunz <[email protected]> * Updating code formatting. Signed-off-by: MaxKsyunz <[email protected]> --------- Signed-off-by: MaxKsyunz <[email protected]> Signed-off-by: Mitchell Gale <[email protected]>

* feat: PPL parser for ccs Signed-off-by: Sean Kao <[email protected]> * feat: disable describe remote cluster index in PPL Allowing the syntax will lead to misunderstanding in the query result, because we will do a local cluster query for index mapping, even for remote indices. This is due to the restriction that OpenSearch doesn't support remote cluster index mapping query at the moment. Signed-off-by: Sean Kao <[email protected]> * feat: Query system index without cluster name We require system index query to happen at the local cluster. Currently, OpenSearch does not support cross cluster system index query. Thus, mapping of a remote index is unavailable. Therefore, we require the local cluster to have the system index of the remote cluster index. The full "cluster:index" name is still used to query OpenSearch for datarows, as CCS is natively supported. Signed-off-by: Sean Kao <[email protected]> * fix: index name parsing for datasources To identify datasources in the index qualified names, they need to be parsed into parts (separated only by dots). clusterQualifiedName can't contain custom datasources, hence the distinction. Signed-off-by: Sean Kao <[email protected]> * multi clusters setup for integration test Signed-off-by: Sean Kao <[email protected]> * Add IT test case Signed-off-by: Sean Kao <[email protected]> * Document ccs for ppl Signed-off-by: Sean Kao <[email protected]> * documentation update Signed-off-by: Sean Kao <[email protected]> * feat: allow describe remote cluster index in PPL Signed-off-by: Sean Kao <[email protected]> * feat: allow "*:index" to match all remote clusters Signed-off-by: Sean Kao <[email protected]> * use local index names for field mappings request Signed-off-by: Sean Kao <[email protected]> * allow ':' in index identifier Signed-off-by: Sean Kao <[email protected]> * docs update Signed-off-by: Sean Kao <[email protected]> * limit cluster prefix to table names only Signed-off-by: Sean Kao <[email protected]> * move multicluster capability to sql rest test case Signed-off-by: Sean Kao <[email protected]> * add IT for failure case Signed-off-by: Sean Kao <[email protected]> * remove logger info for connection in IT test case Signed-off-by: Sean Kao <[email protected]> --------- Signed-off-by: Sean Kao <[email protected]> Signed-off-by: Mitchell Gale <[email protected]>

…h-project#1598) * Bump org.json version for CVE * Fix assertion by json array similar method * Fix more assertions which failed on query path return BigDecimal * Fix legacy expr value factory --------- Signed-off-by: Chen Dai <[email protected]> Signed-off-by: Mitchell Gale <[email protected]>

Signed-off-by: Yury-Fridlyand <[email protected]> Signed-off-by: Mitchell Gale <[email protected]>

…ject#1610) Signed-off-by: Sean Kao <[email protected]> Signed-off-by: Mitchell Gale <[email protected]>

Signed-off-by: Dhrubo Saha <[email protected]> Signed-off-by: Mitchell Gale <[email protected]>

* Add hash join benchmark doc Signed-off-by: Chen Dai <[email protected]> * Fix graph link Signed-off-by: Chen Dai <[email protected]> * Reformat with new lines Signed-off-by: Chen Dai <[email protected]> * Rename doc title Signed-off-by: Chen Dai <[email protected]> * Add note for OOM Signed-off-by: Chen Dai <[email protected]> --------- Signed-off-by: Chen Dai <[email protected]> Signed-off-by: Mitchell Gale <[email protected]>

Signed-off-by: Derek Ho <[email protected]>

…#1620) * Initial commit for adding nested documentation and select clause specific documentation. Signed-off-by: forestmvey <[email protected]> * Adding priorities, additional examples, cleaning up diagrams based on comments and adding table of contents. Signed-off-by: forestmvey <[email protected]> * Fixing using complete class names for state diagrams, added clarification for examples, and used a better example to describe response flattening and cross join. Signed-off-by: forestmvey <[email protected]> * Removing release schedule, removing unnecessary portions to DSL for query examples, minor clarifications on descriptions. Signed-off-by: forestmvey <[email protected]> * Cleaning up Class diagram. Signed-off-by: forestmvey <[email protected]> --------- Signed-off-by: forestmvey <[email protected]> Signed-off-by: Mitchell Gale <[email protected]>

…roject#1645) Signed-off-by: MaxKsyunz <[email protected]> Signed-off-by: Mitchell Gale <[email protected]>

…oject#1662) Signed-off-by: Mitchell Gale <[email protected]>

…prometheus before start. Signed-off-by: Mitchell Gale <[email protected]>

Signed-off-by: Mitchell Gale <[email protected]>

…Prometheus. Signed-off-by: Mitchell Gale <[email protected]>

…o run stopPrometheus before startPrometheus runs. Signed-off-by: Mitchell Gale <[email protected]>

Signed-off-by: Peng Huo <[email protected]> Signed-off-by: Mitchell Gale <[email protected]>

…rch-project#1640) * Adding documentation for nested function use in WHERE clause. Signed-off-by: forestmvey <[email protected]> * Revising sections based on PR feedback. Signed-off-by: forestmvey <[email protected]> * Adding link for flattening details and filling in section with normalization strategy. Signed-off-by: forestmvey <[email protected]> --------- Signed-off-by: forestmvey <[email protected]> Signed-off-by: Mitchell Gale <[email protected]>

…on (opensearch-project#1657) * Adding support for nested function used in predicate expression. Signed-off-by: forestmvey <[email protected]> * Add support for LIKE with nested query in predicate expression. Signed-off-by: forestmvey <[email protected]> --------- Signed-off-by: forestmvey <[email protected]> Signed-off-by: Mitchell Gale <[email protected]>

* Create new anonymizer for new engine (#266) * Created anonymizer listener for anonymizing SQL queries through the new engine Signed-off-by: Matthew Wells <[email protected]> * Update for review comments Signed-off-by: Andrew Carbonetto <[email protected]> * added missing file header, change public variable to private Signed-off-by: Matthew Wells <[email protected]> --------- Signed-off-by: Andrew Carbonetto <[email protected]> Signed-off-by: Matthew Wells <[email protected]> Co-authored-by: Andrew Carbonetto <[email protected]> Signed-off-by: Mitchell Gale <[email protected]>

Signed-off-by: Andrew Carbonetto <[email protected]> Signed-off-by: Mitchell Gale <[email protected]>

* Support Alternate Datetime Formats (#268) * Add OpenSearchDateType as a datatype for matching with Date/Time OpenSearch types Signed-off-by: Andrew Carbonetto <[email protected]> --------- Signed-off-by: Andrew Carbonetto <[email protected]> Signed-off-by: GabeFernandez310 <[email protected]> Signed-off-by: Guian Gumpac <[email protected]> Signed-off-by: MaxKsyunz <[email protected]> Co-authored-by: Andrew Carbonetto <[email protected]> Co-authored-by: GabeFernandez310 <[email protected]> Co-authored-by: MaxKsyunz <[email protected]> Signed-off-by: Mitchell Gale <[email protected]>

opensearch-project#1666) v2 SQL engine can now paginate simple queries. Pagination is initiated by setting fetch_size property in the request JSON. Pagination is implemented using the OpenSearch Scroll API. Please see pagination-v2.md for implementation details. --------- Signed-off-by: MaxKsyunz <[email protected]> Signed-off-by: Yury-Fridlyand <[email protected]> Signed-off-by: Max Ksyunz <[email protected]> Co-authored-by: Yury-Fridlyand <[email protected]> Co-authored-by: GabeFernandez310 <[email protected]> Co-authored-by: Andrew Carbonetto <[email protected]> Signed-off-by: Mitchell Gale <[email protected]>

* Allow backtick around fields in sort cmd Signed-off-by: Joshua Li <[email protected]> * Add test Signed-off-by: Joshua Li <[email protected]> * Update test setup Signed-off-by: Joshua Li <[email protected]> * revert ignoring sql-cli repo if exists Signed-off-by: Joshua Li <[email protected]> --------- Signed-off-by: Joshua Li <[email protected]> Signed-off-by: Mitchell Gale <[email protected]>

Signed-off-by: Mitchell Gale <[email protected]>

…on (opensearch-project#1657) * Adding support for nested function used in predicate expression. Signed-off-by: forestmvey <[email protected]> * Add support for LIKE with nested query in predicate expression. Signed-off-by: forestmvey <[email protected]> --------- Signed-off-by: forestmvey <[email protected]>

opensearch-project#1666) v2 SQL engine can now paginate simple queries. Pagination is initiated by setting fetch_size property in the request JSON. Pagination is implemented using the OpenSearch Scroll API. Please see pagination-v2.md for implementation details. --------- Signed-off-by: MaxKsyunz <[email protected]> Signed-off-by: Yury-Fridlyand <[email protected]> Signed-off-by: Max Ksyunz <[email protected]> Co-authored-by: Yury-Fridlyand <[email protected]> Co-authored-by: GabeFernandez310 <[email protected]> Co-authored-by: Andrew Carbonetto <[email protected]> Signed-off-by: Mitchell Gale <[email protected]>

MitchellGale requested review from MaxKsyunz, Yury-Fridlyand, forestmvey, acarbonetto and GumpacG as code owners May 29, 2023 22:46

Yury-Fridlyand changed the title ~~Made changes to build.gradle for doctest and integ-test to make stop …~~ Made changes to build.gradle for doctest and integ-test to make stop Prometheus May 30, 2023

Yury-Fridlyand approved these changes May 30, 2023

View reviewed changes

forestmvey approved these changes May 30, 2023

View reviewed changes

GumpacG approved these changes May 30, 2023

View reviewed changes

MitchellGale requested a review from matthewryanwells May 30, 2023 17:40

matthewryanwells approved these changes May 30, 2023

View reviewed changes

matthewryanwells approved these changes May 31, 2023

View reviewed changes

MitchellGale self-assigned this May 31, 2023

MaxKsyunz reviewed May 31, 2023

View reviewed changes

Yury-Fridlyand reviewed May 31, 2023

View reviewed changes

MaxKsyunz reviewed May 31, 2023

View reviewed changes

MitchellGale changed the base branch from Integ-fixStopProm to Integ/Fix-StopPrometheus June 12, 2023 18:42

MitchellGale force-pushed the dev-fixStopProm branch from 5ee27d0 to 4af2601 Compare June 12, 2023 18:49

derek-ho and others added 9 commits June 12, 2023 15:59

update codeowners and maintainers (opensearch-project#1579)

12a4c45

Signed-off-by: Derek Ho <[email protected]> Signed-off-by: Mitchell Gale <[email protected]>

Update maintainers list.

d0d2d9f

Signed-off-by: Yury-Fridlyand <[email protected]> Signed-off-by: Mitchell Gale <[email protected]>

Update CODEOWNERS list as proposed.

7eb6676

Signed-off-by: Yury-Fridlyand <[email protected]> Signed-off-by: Mitchell Gale <[email protected]>

Update usage example doc for PPL cross-cluster search (opensearch-pro…

b432756

…ject#1610) Signed-off-by: Sean Kao <[email protected]> Signed-off-by: Mitchell Gale <[email protected]>

adding reflections as a dependency (opensearch-project#1559)

42ee0e1

Signed-off-by: Dhrubo Saha <[email protected]> Signed-off-by: Mitchell Gale <[email protected]>

dai-chen and others added 22 commits June 12, 2023 15:59

add ani to codeowners and maintainers (opensearch-project#1614)

3bc00c9

Signed-off-by: Derek Ho <[email protected]>

Updating imports to reflect changes in opensearch core. (opensearch-p…

534d7d7

…roject#1645) Signed-off-by: MaxKsyunz <[email protected]> Signed-off-by: Mitchell Gale <[email protected]>

Fix of protocol rst for change in error for PPL query. (opensearch-pr…

f45796e

…oject#1662) Signed-off-by: Mitchell Gale <[email protected]>

Made changes to build.gradle for doctest and integ-test to make stop …

bc6b748

…prometheus before start. Signed-off-by: Mitchell Gale <[email protected]>

Undoes changes in integ-test gradle

91042cb

Signed-off-by: Mitchell Gale <[email protected]>

Undoes changes in integ-test gradle

550f212

Signed-off-by: Mitchell Gale <[email protected]>

Made stopPrometheus a method

6e308a4

Signed-off-by: Mitchell Gale <[email protected]>

Added if condition if windows to have startOpenSearch depend on start…

4d5226a

…Prometheus. Signed-off-by: Mitchell Gale <[email protected]>

Made changes to dependance trees for doctests and integration tests t…

94d75ce

…o run stopPrometheus before startPrometheus runs. Signed-off-by: Mitchell Gale <[email protected]>

Bump BWC test version to 2.8 (opensearch-project#1668)

f433a39

Signed-off-by: Peng Huo <[email protected]> Signed-off-by: Mitchell Gale <[email protected]>

Release Notes for 2.8.0 (opensearch-project#1676)

401cd9b

Signed-off-by: Andrew Carbonetto <[email protected]> Signed-off-by: Mitchell Gale <[email protected]>

Moved auth interceptors to common module (opensearch-project#1703)

7613adb

Signed-off-by: Mitchell Gale <[email protected]>

MitchellGale force-pushed the dev-fixStopProm branch from 4bc72c6 to 630a715 Compare June 12, 2023 23:14

MitchellGale closed this Jun 15, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Made changes to build.gradle for doctest and integ-test to make stop Prometheus #270

Made changes to build.gradle for doctest and integ-test to make stop Prometheus #270

MitchellGale commented May 29, 2023

codecov bot commented May 29, 2023 •

edited

Loading

MaxKsyunz May 31, 2023 •

edited

Loading

MitchellGale May 31, 2023

MaxKsyunz May 31, 2023 •

edited

Loading

MaxKsyunz May 31, 2023

MitchellGale May 31, 2023

Yury-Fridlyand May 31, 2023

MitchellGale May 31, 2023

Yury-Fridlyand May 31, 2023

MitchellGale May 31, 2023

Yury-Fridlyand May 31, 2023

MaxKsyunz May 31, 2023

Yury-Fridlyand May 31, 2023

MaxKsyunz May 31, 2023

MitchellGale May 31, 2023

MaxKsyunz May 31, 2023

Yury-Fridlyand commented Jun 12, 2023

		file("$projectDir/bin/prometheus").deleteDir()
		file("$projectDir/bin/prometheus.tar.gz").delete()

	dest new File("$projectDir/bin", 'prometheus.tar.gz')
	dest new File("$projectDir/bin", 'prometheus.tar.gz')
	overwrite false

Made changes to build.gradle for doctest and integ-test to make stop Prometheus #270

Made changes to build.gradle for doctest and integ-test to make stop Prometheus #270

Conversation

MitchellGale commented May 29, 2023

Description

Issues Resolved

Check List

codecov bot commented May 29, 2023 • edited Loading

Codecov Report

MaxKsyunz May 31, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MaxKsyunz May 31, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Yury-Fridlyand commented Jun 12, 2023

codecov bot commented May 29, 2023 •

edited

Loading

MaxKsyunz May 31, 2023 •

edited

Loading

MaxKsyunz May 31, 2023 •

edited

Loading