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

Memory leak in ao_truncate_replay #11202

Closed
hidva opened this issue Nov 23, 2020 · 2 comments · Fixed by #11210
Closed

Memory leak in ao_truncate_replay #11202

hidva opened this issue Nov 23, 2020 · 2 comments · Fixed by #11210

Comments

@hidva
Copy link
Contributor

hidva commented Nov 23, 2020

Greenplum version or build

PostgreSQL 12beta2 (Greenplum Database 7.0.0-alpha.0+dev.14088.g9e45a68ef1 build dev) on x86_64-pc-linux-gnu, compiled by gcc (GCC) 6.5.1 20190307 (Alibaba 6.5.1-1 2.17), 64-bit compiled on Nov 23 2020 20:01:14 (with assert checking)

Step to reproduce the behavior

DATADIRS=~/dbdata NUM_PRIMARY_MIRROR_PAIRS=1 make cluster
createdb aocsoom

# create a table table0 with 1024 columns
psql -d aocsoom -f ddl.sql

# insert table0 created as above. The insert.sql looks like:
# 1: begin;
# 2: begin;
# 1: insert into table0 values(...);
# 2: insert into table0 values(...);
# 1: commit;
# 1q:
# 2: commit;
# 2q:
# After this, table0 will have 127 segments.
python3 sql_isolation_testcase.py --dbname=aocsoom < ~/tmp/aocsoom/insert.sql > ~/tmp/aocsoom/insert.log 2>&1

# do vacuum table0, and now mirror will keep calling ao_truncate_replay()
nohup pgbench -f vacuum.sql -c 1 -l -n -T 3600000 -d aocsoom 1>pgben.log 2>&1 &

ddl.sql.txt
insert.sql.txt

Root cause

It seems we should do pfree(dbPath) in ao_truncate_replay().

static void
ao_truncate_replay(XLogReaderState *record)
{
        char       *dbPath;
        char            path[MAXPGPATH];
        File            file;

        xl_ao_truncate *xlrec = (xl_ao_truncate*) XLogRecGetData(record);

        dbPath = GetDatabasePath(xlrec->target.node.dbNode,
                                                         xlrec->target.node.spcNode);

        // It seems we should do `pfree(dbPath)` here.
        FileClose(file);
}

There is no invocation of pfree(), and the CurrentMemoryContext is TopMemoryContext

(gdb) c
Continuing.

Breakpoint 1, ao_truncate_replay (record=0x625000011950) at cdbappendonlyxlog.c:134
134		xl_ao_truncate *xlrec = (xl_ao_truncate*) XLogRecGetData(record);
(gdb) p *CurrentMemoryContext
$1 = {type = T_AllocSetContext, isReset = false, allowInCritSection = false, mem_allocated = 412728, methods = 0x1e0b1e0 <AllocSetMethods>, parent = 0x0, firstchild = 0x62500001e110, prevchild = 0x0, nextchild = 0x0, name = 0x1e10160 "TopMemoryContext", ident = 0x0, reset_cbs = 0x0}
(gdb) b palloc
Breakpoint 2 at 0x178694d: file mcxt.c, line 1141.
(gdb) b pfree
Breakpoint 3 at 0x1787192: file mcxt.c, line 1246.
(gdb) c
Continuing.

Breakpoint 2, palloc (size=128) at mcxt.c:1141
1141		MemoryContext context = CurrentMemoryContext;
(gdb) c
Continuing.

Breakpoint 1, ao_truncate_replay (record=0x625000011950) at cdbappendonlyxlog.c:134
134		xl_ao_truncate *xlrec = (xl_ao_truncate*) XLogRecGetData(record);
(gdb) c
Continuing.

Breakpoint 2, palloc (size=128) at mcxt.c:1141
1141		MemoryContext context = CurrentMemoryContext;
(gdb) c
Continuing.

Breakpoint 1, ao_truncate_replay (record=0x625000011950) at cdbappendonlyxlog.c:134
134		xl_ao_truncate *xlrec = (xl_ao_truncate*) XLogRecGetData(record);
(gdb) c
Continuing.

Breakpoint 2, palloc (size=128) at mcxt.c:1141
1141		MemoryContext context = CurrentMemoryContext;
@ashwinstar
Copy link
Contributor

ashwinstar commented Nov 23, 2020

Agree @hidva wish to open PR to fix it?

@hidva
Copy link
Contributor Author

hidva commented Nov 23, 2020

hidva added a commit to hidva/gpdb that referenced this issue Nov 25, 2020
The value returned by GetDatabasePath is palloc'd, and
CurrentMemoryContext is TopMemoryContext when we enter
ao_truncate_replay(), so we should do a pfree.

Fix greenplum-db#11202
kainwen-zz pushed a commit that referenced this issue Nov 25, 2020
The value returned by GetDatabasePath is palloc'd, and
CurrentMemoryContext is TopMemoryContext when we enter
ao_truncate_replay(), so we should do a pfree.

Fix Github issue 11202 (#11202)
ashwinstar pushed a commit that referenced this issue Nov 25, 2020
The value returned by GetDatabasePath is palloc'd, and
CurrentMemoryContext is TopMemoryContext when we enter
ao_truncate_replay(), so we should do a pfree.

Fix Github issue 11202 (#11202)

(cherry picked from commit 9bb8bce)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants