Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Some queries failing with code 500: all shards failed | Failed to compile inline script #273

Open
Yury-Fridlyand opened this issue Nov 9, 2021 · 3 comments
Labels
enhancement New feature or request tdvt Tableau test framework

Comments

@Yury-Fridlyand
Copy link
Collaborator

Describe the bug
I started TDVT test suite using JDBC driver 5 times and got 5 different results:

test-1 315 passed
test-2 312 passed
test-3 311
test-4 321
test-5 251

To Reproduce
Steps to reproduce the behavior:

  1. Install TDVT, see links: one, two, three.
  2. Unpack connector to C:\Users\%username%\Documents\My Tableau Repository\Connectors
  3. Launch an instance of OpenSearch
  4. Load test data into it using files in test-tables.zip
  5. Create tds files as it described in links above or use attached ones: tds.zip. Note, they works only with OS running on http://localhost:9200 without authentication
  6. Run tests: python -m tdvt.tdvt run OpenSearch few times

Expected result
Tests would produce the same result all times

Actual result
214 tests failed at the 5th run with the following error:

Shard[0]: GeneralScriptException[Failed to compile inline script [rO0ABXNyADRvcmcub3B ... <omitted>] using lang [opensearch_query_expression]]; nested: CircuitBreakingException[[script] Too many dynamic script compilations within, max: [75/5m]; please use indexed, or scripts with parameters instead; this limit can be changed by the [script.context.aggs.max_compilations_rate] setting];\n\nFor more details, please send request for Json format to see the raw response from OpenSearch engine.

Additional context
The connector forces driver to produce log at %temp%\TableauTemp\jdbc-driver-log.txt

See logs attached:
test-5.zip
test-1.zip
test-2.zip
test-3.zip
test-4.zip
tests-3-4-5-result-compare.csv
test-5-jdbc-driver-log.txt
test-5-part-of-server-log.txt

@Yury-Fridlyand Yury-Fridlyand added Beta bug Something isn't working untriaged labels Nov 9, 2021
@chloe-zh
Copy link
Contributor

Many of the expressions are translated to DSL before executing, this results in a lot of compiling work, so when many requests hit the engine in parallel, the engine might run out of the resources and give out compiling error. We should optimize the expression calculation to fix this issue, but short termly it is recommended to scale your instance up or out. Optionally, we also recommend you change the queries to our preferred semantic, for example, use implicit/explicit cast from string literal to timestamp (e.g. cast('2021-11-09 00:00:00' as timestamp) or directly '2021-11-09 00:00:00') instead of using functions like timestamp('2021-11-09 00:00:00') to do the data converting.

@chloe-zh chloe-zh added enhancement New feature or request and removed bug Something isn't working Beta untriaged labels Nov 13, 2021
@joshuali925 joshuali925 added the tdvt Tableau test framework label Nov 24, 2021
@matthewryanwells
Copy link
Contributor

I have conducted some research into the performance of the functions you mentioned and data type inputs for those functions to see how quickly they run to see if that is an issue that could cause the test failures. The results are in operations per second meaning that the higher the number is the faster it runs, below I have laid out my findings:

Operation Data Type 2.4 upstream/main
CAST AS TIMESTAMP STRING 834.418 382.739
CAST AS TIMESTAMP TIMESTAMP 3538.903 2129.168
CAST AS TIMESTAMP DATE N/A 1165.644
CAST AS TIMESTAMP TIME N/A 1117.844
TIMESTAMP() STRING 648.069 282.688
TIMESTAMP() TIMESTAMP 2872.664 5274.195
TIMESTAMP() DATE 2531.714 719.601
TIMESTAMP() TIME N/A 693.222

The overall efficiency of almost all functions have gone down in the newer version with the exception of giving TIMESTAMP() a TIMESTAMP but that doesn't matter as much compared to the difference in performance from TIMESTAMP() to CAST AS TIMESTAMP. Overall it appears that the average TIMESTAMP() function is roughly 60-70% of the speed of casting it to timestamp with the outlier of casting a timestamp to a timestamp being exceptionally quick.

So although it is faster most of the time to case as timestamp it doesn't appear to be a big enough of a performance hit to be causing test to fail

@matthewryanwells
Copy link
Contributor

Here is the graph for memory usage on the current opensearchsql version:
image

Each function is ran 1000 times to get consistent results and the result is in Bytes per operation, because of this you can divide the result by 1000 to get the amount of data per single operation.

Here is the results of the same function compared to the 2.4 engine:
image

From a precursory glance at both graphs we can see that almost every operation has just about doubled in memory allocation except for inputting a timestamp datatype in the timestamp() function. Looking at the difference in memory usage from using timestamp() compared to casting we can see the casting uses ~30% less memory on average, which I don't believe would be enough to cause tests to fail.

MaxKsyunz pushed a commit that referenced this issue Jul 11, 2023
* Support user-defined and incomplete date formats (#273)

* Check custom formats for characters

Signed-off-by: Guian Gumpac <[email protected]>

* Removed duplicated code

Signed-off-by: Guian Gumpac <[email protected]>

* Reworked checking for exprcoretype

Signed-off-by: Guian Gumpac <[email protected]>

* Changed check for time

Signed-off-by: Guian Gumpac <[email protected]>

* Rework processing custom and incomplete formats and add tests.

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

* Values of incomplete and incorrect formats to be returned as `TIMESTAMP` instead of `STRING`.

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

* Complete fix and update tests.

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

* More fixes for god of fixes.

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

---------

Signed-off-by: Guian Gumpac <[email protected]>
Signed-off-by: Yury-Fridlyand <[email protected]>
Co-authored-by: Yury-Fridlyand <[email protected]>

* Refactoring.

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

---------

Signed-off-by: Guian Gumpac <[email protected]>
Signed-off-by: Yury-Fridlyand <[email protected]>
Co-authored-by: Guian Gumpac <[email protected]>
opensearch-trigger-bot bot pushed a commit that referenced this issue Jul 11, 2023
* Support user-defined and incomplete date formats (#273)

* Check custom formats for characters

Signed-off-by: Guian Gumpac <[email protected]>

* Removed duplicated code

Signed-off-by: Guian Gumpac <[email protected]>

* Reworked checking for exprcoretype

Signed-off-by: Guian Gumpac <[email protected]>

* Changed check for time

Signed-off-by: Guian Gumpac <[email protected]>

* Rework processing custom and incomplete formats and add tests.

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

* Values of incomplete and incorrect formats to be returned as `TIMESTAMP` instead of `STRING`.

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

* Complete fix and update tests.

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

* More fixes for god of fixes.

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

---------

Signed-off-by: Guian Gumpac <[email protected]>
Signed-off-by: Yury-Fridlyand <[email protected]>
Co-authored-by: Yury-Fridlyand <[email protected]>

* Refactoring.

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

---------

Signed-off-by: Guian Gumpac <[email protected]>
Signed-off-by: Yury-Fridlyand <[email protected]>
Co-authored-by: Guian Gumpac <[email protected]>
(cherry picked from commit a60b222)
Yury-Fridlyand added a commit that referenced this issue Jul 11, 2023
* Support user-defined and incomplete date formats (#273)

* Check custom formats for characters

Signed-off-by: Guian Gumpac <[email protected]>

* Removed duplicated code

Signed-off-by: Guian Gumpac <[email protected]>

* Reworked checking for exprcoretype

Signed-off-by: Guian Gumpac <[email protected]>

* Changed check for time

Signed-off-by: Guian Gumpac <[email protected]>

* Rework processing custom and incomplete formats and add tests.

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

* Values of incomplete and incorrect formats to be returned as `TIMESTAMP` instead of `STRING`.

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

* Complete fix and update tests.

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

* More fixes for god of fixes.

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

---------

Signed-off-by: Guian Gumpac <[email protected]>
Signed-off-by: Yury-Fridlyand <[email protected]>
Co-authored-by: Yury-Fridlyand <[email protected]>

* Refactoring.

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

---------

Signed-off-by: Guian Gumpac <[email protected]>
Signed-off-by: Yury-Fridlyand <[email protected]>
Co-authored-by: Guian Gumpac <[email protected]>
(cherry picked from commit a60b222)

Co-authored-by: Yury-Fridlyand <[email protected]>
opensearch-trigger-bot bot pushed a commit that referenced this issue Jul 11, 2023
* Support user-defined and incomplete date formats (#273)

* Check custom formats for characters

Signed-off-by: Guian Gumpac <[email protected]>

* Removed duplicated code

Signed-off-by: Guian Gumpac <[email protected]>

* Reworked checking for exprcoretype

Signed-off-by: Guian Gumpac <[email protected]>

* Changed check for time

Signed-off-by: Guian Gumpac <[email protected]>

* Rework processing custom and incomplete formats and add tests.

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

* Values of incomplete and incorrect formats to be returned as `TIMESTAMP` instead of `STRING`.

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

* Complete fix and update tests.

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

* More fixes for god of fixes.

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

---------

Signed-off-by: Guian Gumpac <[email protected]>
Signed-off-by: Yury-Fridlyand <[email protected]>
Co-authored-by: Yury-Fridlyand <[email protected]>

* Refactoring.

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

---------

Signed-off-by: Guian Gumpac <[email protected]>
Signed-off-by: Yury-Fridlyand <[email protected]>
Co-authored-by: Guian Gumpac <[email protected]>
(cherry picked from commit a60b222)
forestmvey pushed a commit that referenced this issue Jul 11, 2023
* Support user-defined and incomplete date formats (#273)

* Check custom formats for characters

Signed-off-by: Guian Gumpac <[email protected]>

* Removed duplicated code

Signed-off-by: Guian Gumpac <[email protected]>

* Reworked checking for exprcoretype

Signed-off-by: Guian Gumpac <[email protected]>

* Changed check for time

Signed-off-by: Guian Gumpac <[email protected]>

* Rework processing custom and incomplete formats and add tests.

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

* Values of incomplete and incorrect formats to be returned as `TIMESTAMP` instead of `STRING`.

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

* Complete fix and update tests.

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

* More fixes for god of fixes.

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

---------

Signed-off-by: Guian Gumpac <[email protected]>
Signed-off-by: Yury-Fridlyand <[email protected]>
Co-authored-by: Yury-Fridlyand <[email protected]>

* Refactoring.

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

---------

Signed-off-by: Guian Gumpac <[email protected]>
Signed-off-by: Yury-Fridlyand <[email protected]>
Co-authored-by: Guian Gumpac <[email protected]>
(cherry picked from commit a60b222)

Co-authored-by: Yury-Fridlyand <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request tdvt Tableau test framework
Projects
None yet
Development

No branches or pull requests

4 participants