Skip to content

Release 2 6 refactoring #464

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

Merged
merged 18 commits into from
Jan 10, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
[refactoring] remove duplicate fio_unlink() and fio_delete() and merg…
…e into new fio_remove() with proper error checking
  • Loading branch information
kulaginm committed Jan 6, 2022
commit 6216dc7109a8e8db8004c4a194fd73d87a6a9392
74 changes: 49 additions & 25 deletions src/archive.c
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* archive.c: - pg_probackup specific archive commands for archive backups.
*
*
* Portions Copyright (c) 2018-2021, Postgres Professional
* Portions Copyright (c) 2018-2022, Postgres Professional
*
*-------------------------------------------------------------------------
*/
Expand Down Expand Up @@ -497,7 +497,8 @@ push_file_internal_uncompressed(const char *wal_file_name, const char *pg_xlog_d

/* Partial segment is considered stale, so reuse it */
elog(LOG, "Reusing stale temp WAL file \"%s\"", to_fullpath_part);
fio_unlink(to_fullpath_part, FIO_BACKUP_HOST);
if (fio_remove(to_fullpath_part, false, FIO_BACKUP_HOST) != 0)
elog(ERROR, "Cannot remove stale temp WAL file \"%s\": %s", to_fullpath_part, strerror(errno));

out = fio_open(to_fullpath_part, O_RDWR | O_CREAT | O_EXCL | PG_BINARY, FIO_BACKUP_HOST);
if (out < 0)
Expand All @@ -522,7 +523,8 @@ push_file_internal_uncompressed(const char *wal_file_name, const char *pg_xlog_d
/* cleanup */
fclose(in);
fio_close(out);
fio_unlink(to_fullpath_part, FIO_BACKUP_HOST);
if (fio_remove(to_fullpath_part, false, FIO_BACKUP_HOST) != 0)
elog(WARNING, "Cannot remove temp WAL file \"%s\": %s", to_fullpath_part, strerror(errno));
return 1;
}
else
Expand All @@ -535,7 +537,8 @@ push_file_internal_uncompressed(const char *wal_file_name, const char *pg_xlog_d
/* Overwriting is forbidden,
* so we must unlink partial file and exit with error.
*/
fio_unlink(to_fullpath_part, FIO_BACKUP_HOST);
if (fio_remove(to_fullpath_part, false, FIO_BACKUP_HOST) != 0)
elog(WARNING, "Cannot remove temp WAL file \"%s\": %s", to_fullpath_part, strerror(errno));
elog(ERROR, "WAL file already exists in archive with "
"different checksum: \"%s\"", to_fullpath);
}
Expand All @@ -552,16 +555,20 @@ push_file_internal_uncompressed(const char *wal_file_name, const char *pg_xlog_d

if (ferror(in))
{
fio_unlink(to_fullpath_part, FIO_BACKUP_HOST);
int save_errno = errno;
if (fio_remove(to_fullpath_part, false, FIO_BACKUP_HOST) != 0)
elog(WARNING, "Cannot remove temp WAL file \"%s\": %s", to_fullpath_part, strerror(errno));
elog(ERROR, "Cannot read source file \"%s\": %s",
from_fullpath, strerror(errno));
from_fullpath, strerror(save_errno));
}

if (read_len > 0 && fio_write_async(out, buf, read_len) != read_len)
{
fio_unlink(to_fullpath_part, FIO_BACKUP_HOST);
int save_errno = errno;
if (fio_remove(to_fullpath_part, false, FIO_BACKUP_HOST) != 0)
elog(WARNING, "Cannot cleanup temp WAL file \"%s\": %s", to_fullpath_part, strerror(errno));
elog(ERROR, "Cannot write to destination temp file \"%s\": %s",
to_fullpath_part, strerror(errno));
to_fullpath_part, strerror(save_errno));
}

if (feof(in))
Expand All @@ -574,17 +581,20 @@ push_file_internal_uncompressed(const char *wal_file_name, const char *pg_xlog_d
/* Writing is asynchronous in case of push in remote mode, so check agent status */
if (fio_check_error_fd(out, &errmsg))
{
fio_unlink(to_fullpath_part, FIO_BACKUP_HOST);
if (fio_remove(to_fullpath_part, false, FIO_BACKUP_HOST) != 0)
elog(WARNING, "Cannot cleanup temp WAL file \"%s\": %s", to_fullpath_part, strerror(errno));
elog(ERROR, "Cannot write to the remote file \"%s\": %s",
to_fullpath_part, errmsg);
}

/* close temp file */
if (fio_close(out) != 0)
{
fio_unlink(to_fullpath_part, FIO_BACKUP_HOST);
int save_errno = errno;
if (fio_remove(to_fullpath_part, false, FIO_BACKUP_HOST) != 0)
elog(WARNING, "Cannot cleanup temp WAL file \"%s\": %s", to_fullpath_part, strerror(errno));
elog(ERROR, "Cannot close temp WAL file \"%s\": %s",
to_fullpath_part, strerror(errno));
to_fullpath_part, strerror(save_errno));
}

/* sync temp file to disk */
Expand All @@ -602,9 +612,11 @@ push_file_internal_uncompressed(const char *wal_file_name, const char *pg_xlog_d
/* Rename temp file to destination file */
if (fio_rename(to_fullpath_part, to_fullpath, FIO_BACKUP_HOST) < 0)
{
fio_unlink(to_fullpath_part, FIO_BACKUP_HOST);
int save_errno = errno;
if (fio_remove(to_fullpath_part, false, FIO_BACKUP_HOST) != 0)
elog(WARNING, "Cannot cleanup temp WAL file \"%s\": %s", to_fullpath_part, strerror(errno));
elog(ERROR, "Cannot rename file \"%s\" to \"%s\": %s",
to_fullpath_part, to_fullpath, strerror(errno));
to_fullpath_part, to_fullpath, strerror(save_errno));
}

pg_free(buf);
Expand Down Expand Up @@ -743,7 +755,8 @@ push_file_internal_gz(const char *wal_file_name, const char *pg_xlog_dir,

/* Partial segment is considered stale, so reuse it */
elog(LOG, "Reusing stale temp WAL file \"%s\"", to_fullpath_gz_part);
fio_unlink(to_fullpath_gz_part, FIO_BACKUP_HOST);
if (fio_remove(to_fullpath_gz_part, false, FIO_BACKUP_HOST) != 0)
elog(ERROR, "Cannot remove stale compressed temp WAL file \"%s\": %s", to_fullpath_gz_part, strerror(errno));

out = fio_gzopen(to_fullpath_gz_part, PG_BINARY_W, compress_level, FIO_BACKUP_HOST);
if (out == NULL)
Expand Down Expand Up @@ -771,7 +784,8 @@ push_file_internal_gz(const char *wal_file_name, const char *pg_xlog_dir,
/* cleanup */
fclose(in);
fio_gzclose(out);
fio_unlink(to_fullpath_gz_part, FIO_BACKUP_HOST);
if (fio_remove(to_fullpath_gz_part, false, FIO_BACKUP_HOST) != 0)
elog(WARNING, "Cannot remove compressed temp WAL file \"%s\": %s", to_fullpath_gz_part, strerror(errno));
return 1;
}
else
Expand All @@ -784,7 +798,8 @@ push_file_internal_gz(const char *wal_file_name, const char *pg_xlog_dir,
/* Overwriting is forbidden,
* so we must unlink partial file and exit with error.
*/
fio_unlink(to_fullpath_gz_part, FIO_BACKUP_HOST);
if (fio_remove(to_fullpath_gz_part, false, FIO_BACKUP_HOST) != 0)
elog(WARNING, "Cannot remove compressed temp WAL file \"%s\": %s", to_fullpath_gz_part, strerror(errno));
elog(ERROR, "WAL file already exists in archive with "
"different checksum: \"%s\"", to_fullpath_gz);
}
Expand All @@ -801,16 +816,20 @@ push_file_internal_gz(const char *wal_file_name, const char *pg_xlog_dir,

if (ferror(in))
{
fio_unlink(to_fullpath_gz_part, FIO_BACKUP_HOST);
int save_errno = errno;
if (fio_remove(to_fullpath_gz_part, false, FIO_BACKUP_HOST) != 0)
elog(WARNING, "Cannot remove compressed temp WAL file \"%s\": %s", to_fullpath_gz_part, strerror(errno));
elog(ERROR, "Cannot read from source file \"%s\": %s",
from_fullpath, strerror(errno));
from_fullpath, strerror(save_errno));
}

if (read_len > 0 && fio_gzwrite(out, buf, read_len) != read_len)
{
fio_unlink(to_fullpath_gz_part, FIO_BACKUP_HOST);
int save_errno = errno;
if (fio_remove(to_fullpath_gz_part, false, FIO_BACKUP_HOST) != 0)
elog(WARNING, "Cannot cleanup compressed temp WAL file \"%s\": %s", to_fullpath_gz_part, strerror(errno));
elog(ERROR, "Cannot write to compressed temp WAL file \"%s\": %s",
to_fullpath_gz_part, get_gz_error(out, errno));
to_fullpath_gz_part, get_gz_error(out, save_errno));
}

if (feof(in))
Expand All @@ -823,17 +842,20 @@ push_file_internal_gz(const char *wal_file_name, const char *pg_xlog_dir,
/* Writing is asynchronous in case of push in remote mode, so check agent status */
if (fio_check_error_fd_gz(out, &errmsg))
{
fio_unlink(to_fullpath_gz_part, FIO_BACKUP_HOST);
if (fio_remove(to_fullpath_gz_part, false, FIO_BACKUP_HOST) != 0)
elog(WARNING, "Cannot cleanup remote compressed temp WAL file \"%s\": %s", to_fullpath_gz_part, strerror(errno));
elog(ERROR, "Cannot write to the remote compressed file \"%s\": %s",
to_fullpath_gz_part, errmsg);
}

/* close temp file, TODO: make it synchronous */
if (fio_gzclose(out) != 0)
{
fio_unlink(to_fullpath_gz_part, FIO_BACKUP_HOST);
int save_errno = errno;
if (fio_remove(to_fullpath_gz_part, false, FIO_BACKUP_HOST) != 0)
elog(WARNING, "Cannot cleanup compressed temp WAL file \"%s\": %s", to_fullpath_gz_part, strerror(errno));
elog(ERROR, "Cannot close compressed temp WAL file \"%s\": %s",
to_fullpath_gz_part, strerror(errno));
to_fullpath_gz_part, strerror(save_errno));
}

/* sync temp file to disk */
Expand All @@ -852,9 +874,11 @@ push_file_internal_gz(const char *wal_file_name, const char *pg_xlog_dir,
/* Rename temp file to destination file */
if (fio_rename(to_fullpath_gz_part, to_fullpath_gz, FIO_BACKUP_HOST) < 0)
{
fio_unlink(to_fullpath_gz_part, FIO_BACKUP_HOST);
int save_errno = errno;
if (fio_remove(to_fullpath_gz_part, false, FIO_BACKUP_HOST) != 0)
elog(WARNING, "Cannot cleanup compressed temp WAL file \"%s\": %s", to_fullpath_gz_part, strerror(errno));
elog(ERROR, "Cannot rename file \"%s\" to \"%s\": %s",
to_fullpath_gz_part, to_fullpath_gz, strerror(errno));
to_fullpath_gz_part, to_fullpath_gz, strerror(save_errno));
}

pg_free(buf);
Expand Down
31 changes: 19 additions & 12 deletions src/catalog.c
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* catalog.c: backup catalog operation
*
* Portions Copyright (c) 2009-2011, NIPPON TELEGRAPH AND TELEPHONE CORPORATION
* Portions Copyright (c) 2015-2019, Postgres Professional
* Portions Copyright (c) 2015-2022, Postgres Professional
*
*-------------------------------------------------------------------------
*/
Expand Down Expand Up @@ -451,7 +451,7 @@ grab_excl_lock_file(const char *root_dir, const char *backup_id, bool strict)
* it. Need a loop because of possible race condition against other
* would-be creators.
*/
if (fio_unlink(lock_file, FIO_BACKUP_HOST) < 0)
if (fio_remove(lock_file, false, FIO_BACKUP_HOST) < 0)
{
if (errno == ENOENT)
continue; /* race condition, again */
Expand All @@ -476,7 +476,8 @@ grab_excl_lock_file(const char *root_dir, const char *backup_id, bool strict)
int save_errno = errno;

fio_close(fd);
fio_unlink(lock_file, FIO_BACKUP_HOST);
if (fio_remove(lock_file, false, FIO_BACKUP_HOST) != 0)
elog(WARNING, "Cannot remove lock file \"%s\": %s", lock_file, strerror(errno));

/* In lax mode if we failed to grab lock because of 'out of space error',
* then treat backup as locked.
Expand All @@ -494,7 +495,8 @@ grab_excl_lock_file(const char *root_dir, const char *backup_id, bool strict)
int save_errno = errno;

fio_close(fd);
fio_unlink(lock_file, FIO_BACKUP_HOST);
if (fio_remove(lock_file, false, FIO_BACKUP_HOST) != 0)
elog(WARNING, "Cannot remove lock file \"%s\": %s", lock_file, strerror(errno));

/* In lax mode if we failed to grab lock because of 'out of space error',
* then treat backup as locked.
Expand All @@ -511,9 +513,10 @@ grab_excl_lock_file(const char *root_dir, const char *backup_id, bool strict)
{
int save_errno = errno;

fio_unlink(lock_file, FIO_BACKUP_HOST);
if (fio_remove(lock_file, false, FIO_BACKUP_HOST) != 0)
elog(WARNING, "Cannot remove lock file \"%s\": %s", lock_file, strerror(errno));

if (!strict && errno == ENOSPC)
if (!strict && save_errno == ENOSPC)
return LOCK_FAIL_ENOSPC;
else
elog(ERROR, "Could not close lock file \"%s\": %s",
Expand Down Expand Up @@ -608,8 +611,9 @@ wait_shared_owners(pgBackup *backup)
return 1;
}

/* unlink shared lock file */
fio_unlink(lock_file, FIO_BACKUP_HOST);
/* remove shared lock file */
if (fio_remove(lock_file, true, FIO_BACKUP_HOST) != 0)
elog(ERROR, "Cannot remove shared lock file \"%s\": %s", lock_file, strerror(errno));
return 0;
}

Expand Down Expand Up @@ -727,8 +731,9 @@ release_excl_lock_file(const char *backup_dir)

/* TODO Sanity check: maybe we should check, that pid in lock file is my_pid */

/* unlink pid file */
fio_unlink(lock_file, FIO_BACKUP_HOST);
/* remove pid file */
if (fio_remove(lock_file, false, FIO_BACKUP_HOST) != 0)
elog(ERROR, "Cannot remove exclusive lock file \"%s\": %s", lock_file, strerror(errno));
}

void
Expand Down Expand Up @@ -792,7 +797,8 @@ release_shared_lock_file(const char *backup_dir)
/* if there is no active pid left, then there is nothing to do */
if (buffer_len == 0)
{
fio_unlink(lock_file, FIO_BACKUP_HOST);
if (fio_remove(lock_file, false, FIO_BACKUP_HOST) != 0)
elog(ERROR, "Cannot remove shared lock file \"%s\": %s", lock_file, strerror(errno));
return;
}

Expand Down Expand Up @@ -2462,7 +2468,8 @@ write_backup(pgBackup *backup, bool strict)
if (!strict && (save_errno == ENOSPC))
{
fclose(fp);
fio_unlink(path_temp, FIO_BACKUP_HOST);
if (fio_remove(path_temp, false, FIO_BACKUP_HOST) != 0)
elog(elevel, "Additionally cannot remove file \"%s\": %s", path_temp, strerror(errno));
return;
}
}
Expand Down
8 changes: 5 additions & 3 deletions src/catchup.c
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
*
* catchup.c: sync DB cluster
*
* Copyright (c) 2021, Postgres Professional
* Copyright (c) 2021-2022, Postgres Professional
*
*-------------------------------------------------------------------------
*/
Expand Down Expand Up @@ -930,8 +930,10 @@ do_catchup(const char *source_pgdata, const char *dest_pgdata, int num_threads,
char fullpath[MAXPGPATH];

join_path_components(fullpath, dest_pgdata, file->rel_path);
fio_delete(file->mode, fullpath, FIO_LOCAL_HOST);
elog(VERBOSE, "Deleted file \"%s\"", fullpath);
if (fio_remove(fullpath, false, FIO_LOCAL_HOST) == 0)
elog(VERBOSE, "Deleted file \"%s\"", fullpath);
else
elog(ERROR, "Cannot delete redundant file in destination \"%s\": %s", fullpath, strerror(errno));

/* shrink dest pgdata list */
pgFileFree(file);
Expand Down
15 changes: 7 additions & 8 deletions src/delete.c
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* delete.c: delete backup files.
*
* Portions Copyright (c) 2009-2013, NIPPON TELEGRAPH AND TELEPHONE CORPORATION
* Portions Copyright (c) 2015-2019, Postgres Professional
* Portions Copyright (c) 2015-2022, Postgres Professional
*
*-------------------------------------------------------------------------
*/
Expand Down Expand Up @@ -782,7 +782,8 @@ delete_backup_files(pgBackup *backup)
elog(INFO, "Progress: (%zd/%zd). Delete file \"%s\"",
i + 1, num_files, full_path);

pgFileDelete(file->mode, full_path);
if (fio_remove(full_path, false, FIO_BACKUP_HOST) != 0)
elog(ERROR, "Cannot remove file or directory \"%s\": %s", full_path, strerror(errno));
}

parray_walk(files, pgFileFree);
Expand Down Expand Up @@ -948,13 +949,11 @@ delete_walfiles_in_tli(InstanceState *instanceState, XLogRecPtr keep_lsn, timeli
continue;
}

/* unlink segment */
if (fio_unlink(wal_fullpath, FIO_BACKUP_HOST) < 0)
/* remove segment, missing file is not considered as error condition */
if (fio_remove(wal_fullpath, true, FIO_BACKUP_HOST) < 0)
{
/* Missing file is not considered as error condition */
if (errno != ENOENT)
elog(ERROR, "Could not remove file \"%s\": %s",
wal_fullpath, strerror(errno));
elog(ERROR, "Could not remove file \"%s\": %s",
wal_fullpath, strerror(errno));
}
else
{
Expand Down
Loading