Skip to content

Commit

Permalink
[yugabyte#11662] [YSQL] Fix asan issue for TestPgIsolationRegress
Browse files Browse the repository at this point in the history
Summary:
The asan issue occurs because of a memory leak in the isolation regress test
suite's spec parser if we have a comment block in setup with some SQL reserved
words. The error is as below -

```
+ =================================================================
+ ==3009==ERROR: LeakSanitizer: detected memory leaks
+
+ Direct leak of 2048 byte(s) in 1 object(s) allocated from:
+     #0 0x7f7df2412cd9 in realloc /opt/yb-build/llvm/yb-llvm-v12.0.1-yb-1-1633099823-bdb147e6-centos7-x86_64-build/src/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
+     #1 0x40eed7 in pg_realloc /nfusr/dev-server/pjain/code/yugabyte-db/src/postgres/src/common/../../../../../src/postgres/src/common/fe_memutils.c:72:8
+     #2 0x405e31 in addlitchar /nfusr/dev-server/pjain/code/yugabyte-db/src/postgres/src/test/isolation/specscanner.l:116:12
+     #3 0x40542f in spec_yylex /nfusr/dev-server/pjain/code/yugabyte-db/src/postgres/src/test/isolation/specscanner.l
+     #4 0x402ae0 in spec_yyparse /nfusr/dev-server/pjain/code/yugabyte-db/src/postgres/src/test/isolation/specparse.c:1190:16
+     #5 0x409349 in main /nfusr/dev-server/pjain/code/yugabyte-db/src/postgres/src/test/isolation/../../../../../../src/postgres/src/test/isolation/isolationtester.c:112:2
+     #6 0x7f7df12a3554 in __libc_start_main (/lib64/libc.so.6+0x22554)
+
+ Objects leaked above:
+ 0x61d000000080 (2048 bytes)
+
+ SUMMARY: AddressSanitizer: 2048 byte(s) leaked in 1 allocation(s).

======================================================================

```
I found which part of the spec caused the issue by just performing a binary
search and running different parts of the spec instead of reading the code
(since that would take more time and effort). And it is evident that the comment
block with reserved words was the issue. The fix is to just move those blocks
outside.

I have also increased the timeout since the test fails occsionaly with a timeout
error. This happened after I added more test schedules to TestPgIsolationRegress
as part of D15383.

Test Plan: ./yb_build.sh --java-test org.yb.pgsql.TestPgIsolationRegress

Reviewers: mtakahara

Reviewed By: mtakahara

Subscribers: yql

Differential Revision: https://phabricator.dev.yugabyte.com/D15886
  • Loading branch information
pkj415 committed Mar 10, 2022
1 parent 66cec7d commit 28902bf
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,11 @@ protected Map<String, String> getTServerFlags() {
return flagMap;
}

@Override
public int getTestMethodTimeoutSec() {
return 1600;
}

private void runIsolationRegressTest() throws Exception {
runPgRegressTest(
PgRegressBuilder.PG_ISOLATION_REGRESS_DIR /* inputDir */, "yb_pg_isolation_schedule",
Expand Down
47 changes: 20 additions & 27 deletions src/postgres/src/test/isolation/specs/yb_pg_eval-plan-qual.spec
Original file line number Diff line number Diff line change
Expand Up @@ -4,33 +4,31 @@
# re-execute UPDATE and DELETE operations against rows that were updated
# by some concurrent transaction.

# TODO: Currently ALTER TABLE on YB results in test failure because an online schema change
# marks the other sessions's txns as expired. To fix it, we will have to make changes to
# isolationtester.c and start other sessions only after setup is done. The following lines were
# removed from setup of tests related to ALTER TABLE:
#
# CREATE TABLE accounts_ext (accountid text PRIMARY KEY, balance numeric not null, other text);
# INSERT INTO accounts_ext VALUES ('checking', 600, 'other'), ('savings', 700, null);
# ALTER TABLE accounts_ext ADD COLUMN newcol int DEFAULT 42;
# ALTER TABLE accounts_ext ADD COLUMN newcol2 text DEFAULT NULL;

# TODO: YSQL doesn't support table inheritance yet. Below is the setup of tests with table
# inheritance:
# CREATE TABLE p (a int, b int, c int);
# CREATE TABLE c1 () INHERITS (p);
# CREATE TABLE c2 () INHERITS (p);
# CREATE TABLE c3 () INHERITS (p);
# INSERT INTO c1 SELECT 0, a / 3, a % 3 FROM generate_series(0, 9) a;
# INSERT INTO c2 SELECT 1, a / 3, a % 3 FROM generate_series(0, 9) a;
# INSERT INTO c3 SELECT 2, a / 3, a % 3 FROM generate_series(0, 9) a;

setup
{
CREATE TABLE accounts (accountid text PRIMARY KEY, balance numeric not null);
INSERT INTO accounts VALUES ('checking', 600), ('savings', 600);

/*
* TODO: Currently alter on YB results in test failure because an online schema change
* marks the other sessions's txns as expired. To fix it, we will have to make changes to
* isolationtester.c and start other sessions only after setup is done.
*
* CREATE TABLE accounts_ext (accountid text PRIMARY KEY, balance numeric not null, other text);
* INSERT INTO accounts_ext VALUES ('checking', 600, 'other'), ('savings', 700, null);
* ALTER TABLE accounts_ext ADD COLUMN newcol int DEFAULT 42;
* ALTER TABLE accounts_ext ADD COLUMN newcol2 text DEFAULT NULL;
*/

/*
* Commenting out tests with table inheritance since we don't support it yet.
* CREATE TABLE p (a int, b int, c int);
* CREATE TABLE c1 () INHERITS (p);
* CREATE TABLE c2 () INHERITS (p);
* CREATE TABLE c3 () INHERITS (p);
* INSERT INTO c1 SELECT 0, a / 3, a % 3 FROM generate_series(0, 9) a;
* INSERT INTO c2 SELECT 1, a / 3, a % 3 FROM generate_series(0, 9) a;
* INSERT INTO c3 SELECT 2, a / 3, a % 3 FROM generate_series(0, 9) a;
*/

CREATE TABLE table_a (id integer, value text);
CREATE TABLE table_b (id integer, value text);
INSERT INTO table_a VALUES (1, 'tableAValue');
Expand All @@ -44,11 +42,6 @@ teardown
{
DROP TABLE accounts;
DROP TABLE table_a, table_b, jointest;

/*
* DROP TABLE accounts_ext;
* DROP TABLE p CASCADE;
*/
}

session "s1"
Expand Down

0 comments on commit 28902bf

Please sign in to comment.