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

[YSQL] Failed Test: TestPgRegressJson.testPgRegressJson #15682

Closed
rjalan-yb opened this issue Jan 13, 2023 · 7 comments
Closed

[YSQL] Failed Test: TestPgRegressJson.testPgRegressJson #15682

rjalan-yb opened this issue Jan 13, 2023 · 7 comments
Assignees
Labels
area/ysql Yugabyte SQL (YSQL) kind/bug This issue is a bug kind/failing-test Tests and testing infra priority/medium Medium priority issue

Comments

@rjalan-yb
Copy link
Contributor

rjalan-yb commented Jan 13, 2023

Jira Link: DB-5047

Description

The test case: TestPgRegressJson.testPgRegressJson has failed on alma8-clang15-asan since beginning. Never passed and failed with same json and jsonb error:

pg_regress|pid78021|stdout test yb_pg_create_type            ... ok
pg_regress|pid78021|stdout test yb_pg_create_table           ... ok
pg_regress|pid78021|stdout test yb_pg_copy                   ... ok
pg_regress|pid78021|stdout test yb_pg_json                   ... FAILED
pg_regress|pid78021|stdout test yb_pg_jsonb                  ... FAILED
pg_regress|pid78021|stdout test yb_pg_jsonpath               ... ok
pg_regress|pid78021|stdout test yb_pg_jsonpath_encoding      ... ok
pg_regress|pid78021|stdout test yb_pg_jsonb_jsonpath         ... ok
pg_regress|pid78021|stderr -----------------------------------------------------
pg_regress|pid78021|stderr Suppressions used:
pg_regress|pid78021|stderr   count      bytes template
pg_regress|pid78021|stderr       3        505 pg_strdup
pg_regress|pid78021|stderr -----------------------------------------------------
pg_regress|pid78021|stderr 
pg_regress|pid78021|stdout 
pg_regress|pid78021|stdout ======================
pg_regress|pid78021|stdout  2 of 9 tests failed. 
pg_regress|pid78021|stdout ======================

For Json: yb_pg_json.sql#89

SELECT row_to_json(q)
FROM (SELECT $$a$$ || x AS b,
         y AS c,
--- 211,278 ----
ERROR:  stack depth limit exceeded
HINT:  Increase the configuration parameter "max_stack_depth" (currently 100kB), after ensuring the platform's stack depth limit is adequate.
RESET max_stack_depth;

For JSONB all tsheadline case get errored out with this: yb_pg_jsonb.sql#1248

4838  -- corner cases for ts_headline with jsonb                                                                                                                                                                                                                                                                                                             2051  -- corner cases for ts_headline with jsonb
4839  select ts_headline('null'::jsonb, tsquery('aaa & bbb'));                                                                                                                                                                                                                                                                                               2052  select ts_headline('null'::jsonb, tsquery('aaa & bbb'));
4840   ts_headline                                                                                                                                                                                                                                                                                                                                        |  2053  ERROR:  current transaction is aborted, commands ignored until end of transaction block
4841  -------------    

Failing 98/100 times on alma8-clang15-asan: https://detective-gcp.dev.yugabyte.com/stability/test?analyze_trends=false&branch=master&build_type=all&class=org.yb.pgsql.TestPgRegressJson&fail_tag=all&name=testPgRegressJson&num_commits=500&num_fails=100&platform=all

Passing on 2.17.0 on alma8-clang13-asan:
https://detective-gcp.dev.yugabyte.com/stability/test?analyze_trends=true&branch=2.17.0&build_type=all&class=org.yb.pgsql.TestPgRegressJson&fail_tag=all&name=testPgRegressJson&num_commits=500&platform=all

yb_pg_jsonb.sql was modified in Commit: 509b95010c

@rjalan-yb rjalan-yb added area/ysql Yugabyte SQL (YSQL) status/awaiting-triage Issue awaiting triage labels Jan 13, 2023
@yugabyte-ci yugabyte-ci added kind/bug This issue is a bug priority/medium Medium priority issue labels Jan 13, 2023
@rjalan-yb rjalan-yb added the kind/failing-test Tests and testing infra label Jan 13, 2023
@yugabyte-ci yugabyte-ci removed the status/awaiting-triage Issue awaiting triage label Jan 22, 2023
@bmatican bmatican assigned jasonyb and unassigned rjalan-yb Mar 13, 2023
@jasonyb
Copy link
Contributor

jasonyb commented Jul 3, 2023

Commit 8c26092 fixes similar issue, but a deeper fix should be done, such as modifying ASAN's limit.

Internal link: https://yugabyte.slack.com/archives/C4NAYRFS8/p1677706886196309?thread_ts=1677085105.706619&cid=C4NAYRFS8

@sushantrmishra
Copy link

SELECT array_to_json(array_agg(x),false) from generate_series(5,10) x;
! array_to_json
! ----------------
! [5,6,7,8,9,10]
! (1 row)
!
SELECT array_to_json('{{1,5},{99,100}}'::int[]);
! array_to_json
! ------------------
! [[1,5],[99,100]]
! (1 row)
!
-- row_to_json
SELECT row_to_json(row(1,'foo'));
! row_to_json
! ---------------------
! {"f1":1,"f2":"foo"}
! (1 row)
!
SELECT row_to_json(q)
FROM (SELECT $$a$$ || x AS b,
y AS c,
--- 211,278 ----
ERROR: stack depth limit exceeded
HINT: Increase the configuration parameter "max_stack_depth" (currently 100kB), after ensuring the platform's stack depth limit is adequate.
RESET max_stack_depth;

  • ERROR: stack depth limit exceeded
  • HINT: Increase the configuration parameter "max_stack_depth" (currently 100kB), after ensuring the platform's stack depth limit is adequate.
    -- Miscellaneous stuff.
    SELECT 'true'::json; -- OK
    ! ERROR: stack depth limit exceeded
    ! HINT: Increase the configuration parameter "max_stack_depth" (currently 100kB), after ensuring the platform's stack depth limit is adequate.
    SELECT 'false'::json; -- OK
    ! ERROR: stack depth limit exceeded
    ! HINT: Increase the configuration parameter "max_stack_depth" (currently 100kB), after ensuring the platform's stack depth limit is adequate.
    SELECT 'null'::json; -- OK
    ! ERROR: stack depth limit exceeded
    ! HINT: Increase the configuration parameter "max_stack_depth" (currently 100kB), after ensuring the platform's stack depth limit is adequate.
    SELECT ' true '::json; -- OK, even with extra whitespace
    ! ERROR: stack depth limit exceeded
    ! HINT: Increase the configuration parameter "max_stack_depth" (currently 100kB), after ensuring the platform's stack depth limit is adequate.
    SELECT 'true false'::json; -- ERROR, too many values
    ! ERROR: stack depth limit exceeded
    ! HINT: Increase the configuration parameter "max_stack_depth" (currently 100kB), after ensuring the platform's stack depth limit is adequate.
    SELECT 'true, false'::json; -- ERROR, too many values
    ! ERROR: stack depth limit exceeded
    ! HINT: Increase the configuration parameter "max_stack_depth" (currently 100kB), after ensuring the platform's stack depth limit is adequate.
    SELECT 'truf'::json; -- ERROR, not a keyword
    ! ERROR: stack depth limit exceeded
    ! HINT: Increase the configuration parameter "max_stack_depth" (currently 100kB), after ensuring the platform's stack depth limit is adequate.
    SELECT 'trues'::json; -- ERROR, not a keyword
    ! ERROR: stack depth limit exceeded
    ! HINT: Increase the configuration parameter "max_stack_depth" (currently 100kB), after ensuring the platform's stack depth limit is adequate.
    SELECT ''::json; -- ERROR, no value
    ! ERROR: stack depth limit exceeded
    ! HINT: Increase the configuration parameter "max_stack_depth" (currently 100kB), after ensuring the platform's stack depth limit is adequate.
    SELECT ' '::json; -- ERROR, no value
    ! ERROR: stack depth limit exceeded
    ! HINT: Increase the configuration parameter "max_stack_depth" (currently 100kB), after ensuring the platform's stack depth limit is adequate.
    --constructors
    -- array_to_json
    SELECT array_to_json(array(select 1 as a));
    ! ERROR: stack depth limit exceeded
    ! HINT: Increase the configuration parameter "max_stack_depth" (currently 100kB), after ensuring the platform's stack depth limit is adequate.
    SELECT array_to_json(array_agg(q),false) from (select x as b, x * 2 as c from generate_series(1,3) x) q;
    ! ERROR: stack depth limit exceeded
    ! HINT: Increase the configuration parameter "max_stack_depth" (currently 100kB), after ensuring the platform's stack depth limit is adequate.
    SELECT array_to_json(array_agg(q),true) from (select x as b, x * 2 as c from generate_series(1,3) x) q;
    ! ERR
    ...[truncated 3298779 chars]...

@yugabyte-ci yugabyte-ci added priority/high High Priority and removed priority/medium Medium priority issue labels Nov 8, 2023
@yugabyte-ci yugabyte-ci added priority/medium Medium priority issue and removed priority/high High Priority labels Nov 16, 2023
@OlegLoginov
Copy link
Contributor

RCA:

SET max_stack_depth = '100kB';

SELECT repeat('[', 10000)::json;                                             
ERROR:  stack depth limit exceeded                                           
HINT:  Increase the configuration parameter "max_stack_depth" (currently 100k

show max_stack_depth;                                                        
ERROR:  stack depth limit exceeded                                           
HINT:  Increase the configuration parameter "max_stack_depth" (currently 100k

Here we have expected error stack depth limit exceeded after recursive statement repeat.
In usual (debug/release) mode the next statement starts form a clean stack.
But in ASAN the stack is NOT cleaned. So, ANY next statement has the same stack state as after the previous repeat command. The debug log - see stack_depth value:

show max_stack_depth;
 max_stack_depth
-----------------
 512kB
(1 row)

SELECT repeat('[', 10000)::json;
ERROR:  stack depth limit exceeded
HINT:  stack_depth = 524352 max_stack_depth_bytes = 524288

show max_stack_depth;
ERROR:  stack depth limit exceeded
HINT:  stack_depth = 544000 max_stack_depth_bytes = 524288 

CONCLUSION: In ASAN any statement after recursive command (like repeat above) should generate stack depth limit exceeded error after ANY statement (even after the simplest show statement).

@OlegLoginov
Copy link
Contributor

Possible FIX: let's move all recurrent statements to the end of the tests.
It does not affect testing in debug & release, but hides the "only growing stack" issue in ASAN.

@OlegLoginov
Copy link
Contributor

OlegLoginov commented Jun 6, 2024

It potentially affects the following PG tests:
ybd asan --java-test org.yb.pgsql.TestPgRegressJson#testPgRegressJson

src/postgres/src/test/regress/sql/yb_pg_json.sql   
src/postgres/src/test/regress/sql/yb_pg_jsonb.sql  

ybd asan --java-test org.yb.pgsql.TestPgRegressPgMisc#testPgRegressPgMisc

src/postgres/src/test/regress/sql/yb_pg_errors.sql 

Not used:

src/postgres/src/test/regress/sql/json.sql         
src/postgres/src/test/regress/sql/jsonb.sql        

@OlegLoginov
Copy link
Contributor

OlegLoginov commented Jun 6, 2024

For the test
ybd asan --java-test org.yb.pgsql.TestPgRegressPgMisc#testPgRegressPgMisc
the error is:

-- Check that stack depth detection mechanism works and                         -- Check that stack depth detection mechanism works and
-- max_stack_depth is not set too high                                          -- max_stack_depth is not set too high
create function infinite_recurse() returns int as                               create function infinite_recurse() returns int as
'select infinite_recurse()' language sql;                                       'select infinite_recurse()' language sql;
\set VERBOSITY terse                                                            \set VERBOSITY terse
select infinite_recurse();                                                      select infinite_recurse();
ERROR:  stack depth limit exceeded                                            | server closed the connection unexpectedly
                                                                              >         This probably means the server terminated abnormally
                                                                              >         before or while processing the request.
                                                                              > connection to server was lost

It should be investigated & fixed in another GHI: #16188

@OlegLoginov
Copy link
Contributor

Alternative fix (D35672) was abandoned.

d-uspenskiy added a commit that referenced this issue Jun 26, 2024
Summary:
The `stack_is_too_deep` Postgres function is used to limits stack depth based on total stack size.
The stack size is calculated by using  local variables address offset.
This method works well in case of regular call stack (i.e. when all stack frames are allocated in stack).
But for the `detect_stack_use_after_return` ASAN uses fake stack.
In case of using it stack frames are allocated in the heap.
As a result it is not possible to estimate stack depth base on local variables address offset and the `stack_is_too_deep` returns almost unpredictable results.

To make `stack_is_too_deep `return predictable result in case of ASAN it is reasonable to return `false` all the time.

**Note**:
YSQL has some unit tests which checks that Postgres can detect too deep recursion.
These tests changes the `max_stack_depth` GUC variable to lower value.
And later restore the original value with the `RESET max_stack_depth` statement.
To make these tests works under the ASAN the function returns true in case the `max_stack_depth`
GUC contains non default value and number of call stack frames is huge enough.
The check of call stack frames is required to avoid undesired failure on attempt to restore
original value for the `max_stack_depth` GUC with the `RESET max_stack_depth` statement.
Jira: DB-5047

Test Plan:
Jenkins

```
./yb_build.sh asan --java-test org.yb.pgsql.TestPgRegressJson#testPgRegressJson
```

Reviewers: loginov, jason

Reviewed By: loginov, jason

Subscribers: yql

Tags: #jenkins-ready

Differential Revision: https://phorge.dev.yugabyte.com/D35979
jasonyb pushed a commit that referenced this issue Jun 27, 2024
Summary:
 411a32e [DB-11813] Rename ysql_conn_mgr_idle_or_pending_clients metric name
 c2e13ef [#15682] YSQL: Fix stack_is_too_deep function in ASAN
 ef31455 [PLAT-14188] Fixing upgrade disk availability check
 db6b1b7 [#23004] YCQL: Fix tserver crash due to NULL pointer dereference
 0ada80a [PLAT-14433] Use correct kubeconfig for edit provider validation
 eccbc10 [PLAT-14414] Enable Kubernetes provider validation by default
 199f679 [PLAT-14324]: Move all node agent based flags from BETA to INTERNAL in Provider Conf keys file
 86a865d [PLAT-14443] YBA Installer wait for ready time configurable.
 ac184a8 [#22882] YSQL: Fix deadlock in DDL atomicity
 a4218fb [Docs] Sort feature to tables (Where fulfills the criteria) (#22836)
 2f267ca [#22996] xCluster: Add SOURCE_UNREACHABLE and SYSTEM_ERROR enums

Skipped due to conflict:
dee7691 [#21534] docdb: Set owner correctly for cloned databases
34632ba [PLAT-14495] Set up the node_exporter for ynp
7c99ff9 [#22876][#22773] CDCSDK: Add new yb-admin command to remove user table from CDCSDK stream
4e9a81c [#22876][#22835][#22773] CDCSDK: Remove non-eligible tables for CDC from existing CDCSDK stream
f2e574e [#23013] xClusterDDLRepl: Allow table_ids for GetXClusterStreams

Test Plan: Jenkins: rebase: pg15-cherrypicks

Reviewers: tfoucher, sanketh, jenkins-bot

Subscribers: yql

Differential Revision: https://phorge.dev.yugabyte.com/D36184
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ysql Yugabyte SQL (YSQL) kind/bug This issue is a bug kind/failing-test Tests and testing infra priority/medium Medium priority issue
Projects
None yet
Development

No branches or pull requests

6 participants