Skip to content

Commit

Permalink
Truncate segment files of AO relations before unlink (#675)
Browse files Browse the repository at this point in the history
Problem description:
Segment files of the AO tables were not truncated before unlink. As a result,
after relation drop, if there were file descriptors of segment files, not closed
by some backend process, disk space was not returned to the OS.

Root cause:
In mdunlinkfork, mdunlink_ao is called for an AO relation instead of truncating
all segment files. And mdunlink_ao doesn't perform truncation of segment files.

Fix:
Truncation of segment files for AO tables was added into mdunlink_ao_perFile,
which is called from mdunlink_ao for each segment file. For that purpose, static
function do_truncate was changed to global. Plus aomd unit test was updated.
Reason - unit test calls mdunlink_ao with filenames of not-existing files. Unit
test replaces unlink, so it can handle such filenames. But now
mdunlink_ao_perFile returns before invoking unlink, because do_truncate fails to
do truncation of these files. To fix it, do_truncate in aomd unit test was
replaced with a stub during the test.

Changes from original commit:
1. Added RESET for `default_tablespace` to previous test.

(cherry picked from commit 6f0a2c1)
  • Loading branch information
whitehawk authored and silent-observer committed Nov 27, 2024
1 parent e8aa067 commit 99968c6
Show file tree
Hide file tree
Showing 6 changed files with 63 additions and 1 deletion.
8 changes: 8 additions & 0 deletions src/backend/access/appendonly/aomd.c
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
#include "common/relpath.h"
#include "pgstat.h"
#include "storage/bufmgr.h"
#include "storage/md.h"
#include "storage/sync.h"
#include "utils/faultinjector.h"
#include "utils/guc.h"
Expand Down Expand Up @@ -370,6 +371,13 @@ mdunlink_ao_perFile(const int segno, void *ctx)
SYNC_HANDLER_AO);
RegisterSyncRequest(&tag, SYNC_FORGET_REQUEST, true);

/*
* unlink is not enough to return disk space to the OS immediately, because
* the file can be still opened by other process
*/
if (do_truncate(segPath) < 0 && errno == ENOENT)
return false;

/* Next unlink the file */
if (unlink(segPath) != 0)
{
Expand Down
13 changes: 13 additions & 0 deletions src/backend/access/appendonly/test/aomd_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ static bool file_present[MAX_SEGNO_FILES];
static int num_unlink_called = 0;
static bool unlink_passing = true;

int __wrap_do_truncate(const char *path);

static void
setup_test_structures()
{
Expand Down Expand Up @@ -59,6 +61,17 @@ mock_unlink(const char *path)
#endif
return ec;
}

/*
* Make do_truncate return always Ok during unit test,
* because otherwise mdunlink_ao will return before actual invoke of unlink,
* if called with test file names.
*/
int
__wrap_do_truncate(const char *path)
{
return 0;
}
/*
*******************************************************************************
*/
Expand Down
2 changes: 1 addition & 1 deletion src/backend/storage/smgr/md.c
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ mdunlink(RelFileNodeBackend rnode, ForkNumber forkNum, bool isRedo)
/*
* Truncate a file to release disk space.
*/
static int
int
do_truncate(const char *path)
{
int save_errno;
Expand Down
2 changes: 2 additions & 0 deletions src/include/storage/md.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,4 +50,6 @@ extern int mdunlinkfiletag(const FileTag *ftag, char *path);
extern bool mdfiletagmatches(const FileTag *ftag, const FileTag *candidate);

extern int aosyncfiletag(const FileTag *ftag, char *path);

extern int do_truncate(const char *path);
#endif /* MD_H */
22 changes: 22 additions & 0 deletions src/test/regress/input/tablespace.source
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,7 @@ ALTER TABLE ALL IN TABLESPACE regress_tblspace_renamed SET TABLESPACE pg_default

-- Should succeed
DROP TABLESPACE regress_tblspace_renamed;
RESET default_tablespace;

DROP SCHEMA testschema CASCADE;

Expand All @@ -333,3 +334,24 @@ SELECT c.relname, t.spcname FROM pg_class c LEFT JOIN pg_tablespace t ON c.relta

DROP TABLE tablespace_part;
DROP TABLESPACE myts;

-- Create a new tablespace and check that within this tablespace,
-- after dropping the table, the size of all deleted (but still opened) files is 0.
CREATE TABLESPACE testspace LOCATION '@testtablespace@';
-- start_ignore
DROP TABLE IF EXISTS t;
-- end_ignore

-- Checking that the AO row-oriented table doesn't leave non-zero files.
CREATE TABLE t(a int, b int) WITH (appendoptimized=true, orientation=row) TABLESPACE testspace;
INSERT INTO t SELECT i, i FROM generate_series(1, 1000) i;
DROP TABLE t;
\! find /proc/[0-9]*/fd -lname '@testtablespace@*(deleted)' -exec stat -Lc '%s' {} \; 2>/dev/null | awk 'BEGIN {sum=0} {sum += $1} END {print "size:", sum}';

-- Checking that the AO column-oriented table doesn't leave non-zero files.
CREATE TABLE t(a int, b int) WITH (appendoptimized=true, orientation=column) TABLESPACE testspace;
INSERT INTO t SELECT i, i FROM generate_series(1, 1000) i;
DROP TABLE t;
\! find /proc/[0-9]*/fd -lname '@testtablespace@*(deleted)' -exec stat -Lc '%s' {} \; 2>/dev/null | awk 'BEGIN {sum=0} {sum += $1} END {print "size:", sum}';

DROP TABLESPACE testspace;
17 changes: 17 additions & 0 deletions src/test/regress/output/tablespace.source
Original file line number Diff line number Diff line change
Expand Up @@ -702,6 +702,7 @@ ALTER TABLE ALL IN TABLESPACE regress_tblspace_renamed SET TABLESPACE pg_default
NOTICE: no matching relations in tablespace "regress_tblspace_renamed" found
-- Should succeed
DROP TABLESPACE regress_tblspace_renamed;
RESET default_tablespace;
DROP SCHEMA testschema CASCADE;
NOTICE: drop cascades to 6 other objects
DETAIL: drop cascades to table testschema.foo
Expand Down Expand Up @@ -737,3 +738,19 @@ SELECT c.relname, t.spcname FROM pg_class c LEFT JOIN pg_tablespace t ON c.relta

DROP TABLE tablespace_part;
DROP TABLESPACE myts;
-- Create a new tablespace and check that within this tablespace,
-- after dropping the table, the size of all deleted (but still opened) files is 0.
CREATE TABLESPACE testspace LOCATION '@testtablespace@';
-- Checking that the AO row-oriented table doesn't leave non-zero files.
CREATE TABLE t(a int, b int) WITH (appendoptimized=true, orientation=row) TABLESPACE testspace;
INSERT INTO t SELECT i, i FROM generate_series(1, 1000) i;
DROP TABLE t;
\! find /proc/[0-9]*/fd -lname '@testtablespace@*(deleted)' -exec stat -Lc '%s' {} \; 2>/dev/null | awk 'BEGIN {sum=0} {sum += $1} END {print "size:", sum}';
size: 0
-- Checking that the AO column-oriented table doesn't leave non-zero files.
CREATE TABLE t(a int, b int) WITH (appendoptimized=true, orientation=column) TABLESPACE testspace;
INSERT INTO t SELECT i, i FROM generate_series(1, 1000) i;
DROP TABLE t;
\! find /proc/[0-9]*/fd -lname '@testtablespace@*(deleted)' -exec stat -Lc '%s' {} \; 2>/dev/null | awk 'BEGIN {sum=0} {sum += $1} END {print "size:", sum}';
size: 0
DROP TABLESPACE testspace;

0 comments on commit 99968c6

Please sign in to comment.