Skip to content

Commit

Permalink
Fix orphaned file descriptors
Browse files Browse the repository at this point in the history
Problem:
In cases where an AO table is created, some data is inserted, and then the
table is dropped, file descriptor of segfile will be opened still. It
stays open until session ends. Thus, disc space is not freed until the end
of the session.

Cause:
There are 2 issues:
1) mdcreate_ao is used to create a segment file during data insertion.
For creation, it calls PathNameOpenFile without closing.
File descriptor returned by PathNameOpenFile in mdcreate_ao is not stored
anywhere except the virtual file level (fd.c), where it will be cleaned
some time later only when the number of opened descriptors reaches threshold.
2) second issue happens when we run out of space. This problem is related to
file descriptors left open because of an abnormal abort of the insert command.
In function BufferedAppendWrite, once free space has ended, ereport is thrown.
Before that, the file is not closed, so after the long jump it remained open.

Fix:
Add FileClose to mdcreate_ao and BufferedAppendWrite (before throwing ereport).
  • Loading branch information
whitehawk committed Dec 28, 2023
1 parent e730004 commit 20aa6e1
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 0 deletions.
7 changes: 7 additions & 0 deletions src/backend/cdb/cdbbufferedappend.c
Original file line number Diff line number Diff line change
Expand Up @@ -180,11 +180,18 @@ BufferedAppendWrite(BufferedAppend *bufferedAppend, bool needsWAL)
(char *) largeWriteMemory + bytestotal,
bytesleft);
if (byteswritten < 0)
{
/*
* Close the file before throwing the ereport
* to avoid orphaned file descriptors if we run out of space
*/
FileClose(bufferedAppend->file);
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not write in table \"%s\" to segment file \"%s\": %m",
bufferedAppend->relationName,
bufferedAppend->filePathName)));
}

bytesleft -= byteswritten;
bytestotal += byteswritten;
Expand Down
8 changes: 8 additions & 0 deletions src/backend/storage/smgr/md.c
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,14 @@ mdcreate_ao(RelFileNodeBackend rnode, int32 segmentFileNum, bool isRedo)
}
}

/*
* This function is intended only to a create file.
* So, once the file is created successfully, close it.
* If someone wants to access this file afterward,
* he will need to open it again.
*/
FileClose(fd);

if (path != buf)
pfree(path);
}
Expand Down
10 changes: 10 additions & 0 deletions src/test/regress/input/tablespace.source
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,16 @@ CREATE TABLE tablespace_table (i int) TABLESPACE testspace; -- fail
ALTER TABLE testschema.tablespace_acl ALTER c TYPE bigint;
RESET ROLE;

CREATE TEMP TABLE test_table(a int) WITH (APPENDONLY = TRUE) TABLESPACE testspace DISTRIBUTED BY (a);
INSERT INTO test_table SELECT generate_series(1,100);
DROP TABLE test_table;

CREATE TABLE test_table(a int) WITH (APPENDONLY = TRUE) TABLESPACE testspace DISTRIBUTED BY (a);
INSERT INTO test_table SELECT generate_series(1,100);
DROP TABLE test_table;
-- Check there is zero opened file descriptors after query
\! lsof +L1 | grep postgres | grep testtablespace

ALTER TABLESPACE testspace RENAME TO testspace_renamed;

-- Test that default_tablespace GUC is honored even after gang reset.
Expand Down
8 changes: 8 additions & 0 deletions src/test/regress/output/tablespace.source
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,14 @@ ALTER TABLE testschema.tablespace_acl ALTER c TYPE bigint;
ERROR: cannot alter indexed column
HINT: DROP the index first, and recreate it after the ALTER
RESET ROLE;
CREATE TEMP TABLE test_table(a int) WITH (APPENDONLY = TRUE) TABLESPACE testspace DISTRIBUTED BY (a);
INSERT INTO test_table SELECT generate_series(1,100);
DROP TABLE test_table;
CREATE TABLE test_table(a int) WITH (APPENDONLY = TRUE) TABLESPACE testspace DISTRIBUTED BY (a);
INSERT INTO test_table SELECT generate_series(1,100);
DROP TABLE test_table;
-- Check there is zero opened file descriptors after query
\! lsof +L1 | grep postgres | grep testtablespace
ALTER TABLESPACE testspace RENAME TO testspace_renamed;
-- Test that default_tablespace GUC is honored even after gang reset.
CREATE OR REPLACE FUNCTION cleanupAllGangs() RETURNS BOOL
Expand Down

0 comments on commit 20aa6e1

Please sign in to comment.