Skip to content

Commit f22c5fc

Browse files
kulaginmkulaginm
committed
[refactoring] remove duplicate fio_unlink() and fio_delete() and merge into new fio_remove() with proper error checking
1 parent 1f7d26f commit f22c5fc

File tree

10 files changed

+164
-154
lines changed

10 files changed

+164
-154
lines changed

‎src/archive.c

Lines changed: 49 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* archive.c: - pg_probackup specific archive commands for archive backups.
44
*
55
*
6-
* Portions Copyright (c) 2018-2021, Postgres Professional
6+
* Portions Copyright (c) 2018-2022, Postgres Professional
77
*
88
*-------------------------------------------------------------------------
99
*/
@@ -497,7 +497,8 @@ push_file_internal_uncompressed(const char *wal_file_name, const char *pg_xlog_d
497497

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

502503
out = fio_open(to_fullpath_part, O_RDWR | O_CREAT | O_EXCL | PG_BINARY, FIO_BACKUP_HOST);
503504
if (out < 0)
@@ -522,7 +523,8 @@ push_file_internal_uncompressed(const char *wal_file_name, const char *pg_xlog_d
522523
/* cleanup */
523524
fclose(in);
524525
fio_close(out);
525-
fio_unlink(to_fullpath_part, FIO_BACKUP_HOST);
526+
if (fio_remove(to_fullpath_part, false, FIO_BACKUP_HOST) != 0)
527+
elog(WARNING, "Cannot remove temp WAL file \"%s\": %s", to_fullpath_part, strerror(errno));
526528
return 1;
527529
}
528530
else
@@ -535,7 +537,8 @@ push_file_internal_uncompressed(const char *wal_file_name, const char *pg_xlog_d
535537
/* Overwriting is forbidden,
536538
* so we must unlink partial file and exit with error.
537539
*/
538-
fio_unlink(to_fullpath_part, FIO_BACKUP_HOST);
540+
if (fio_remove(to_fullpath_part, false, FIO_BACKUP_HOST) != 0)
541+
elog(WARNING, "Cannot remove temp WAL file \"%s\": %s", to_fullpath_part, strerror(errno));
539542
elog(ERROR, "WAL file already exists in archive with "
540543
"different checksum: \"%s\"", to_fullpath);
541544
}
@@ -552,16 +555,20 @@ push_file_internal_uncompressed(const char *wal_file_name, const char *pg_xlog_d
552555

553556
if (ferror(in))
554557
{
555-
fio_unlink(to_fullpath_part, FIO_BACKUP_HOST);
558+
int save_errno = errno;
559+
if (fio_remove(to_fullpath_part, false, FIO_BACKUP_HOST) != 0)
560+
elog(WARNING, "Cannot remove temp WAL file \"%s\": %s", to_fullpath_part, strerror(errno));
556561
elog(ERROR, "Cannot read source file \"%s\": %s",
557-
from_fullpath, strerror(errno));
562+
from_fullpath, strerror(save_errno));
558563
}
559564

560565
if (read_len > 0 && fio_write_async(out, buf, read_len) != read_len)
561566
{
562-
fio_unlink(to_fullpath_part, FIO_BACKUP_HOST);
567+
int save_errno = errno;
568+
if (fio_remove(to_fullpath_part, false, FIO_BACKUP_HOST) != 0)
569+
elog(WARNING, "Cannot cleanup temp WAL file \"%s\": %s", to_fullpath_part, strerror(errno));
563570
elog(ERROR, "Cannot write to destination temp file \"%s\": %s",
564-
to_fullpath_part, strerror(errno));
571+
to_fullpath_part, strerror(save_errno));
565572
}
566573

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

582590
/* close temp file */
583591
if (fio_close(out) != 0)
584592
{
585-
fio_unlink(to_fullpath_part, FIO_BACKUP_HOST);
593+
int save_errno = errno;
594+
if (fio_remove(to_fullpath_part, false, FIO_BACKUP_HOST) != 0)
595+
elog(WARNING, "Cannot cleanup temp WAL file \"%s\": %s", to_fullpath_part, strerror(errno));
586596
elog(ERROR, "Cannot close temp WAL file \"%s\": %s",
587-
to_fullpath_part, strerror(errno));
597+
to_fullpath_part, strerror(save_errno));
588598
}
589599

590600
/* sync temp file to disk */
@@ -602,9 +612,11 @@ push_file_internal_uncompressed(const char *wal_file_name, const char *pg_xlog_d
602612
/* Rename temp file to destination file */
603613
if (fio_rename(to_fullpath_part, to_fullpath, FIO_BACKUP_HOST) < 0)
604614
{
605-
fio_unlink(to_fullpath_part, FIO_BACKUP_HOST);
615+
int save_errno = errno;
616+
if (fio_remove(to_fullpath_part, false, FIO_BACKUP_HOST) != 0)
617+
elog(WARNING, "Cannot cleanup temp WAL file \"%s\": %s", to_fullpath_part, strerror(errno));
606618
elog(ERROR, "Cannot rename file \"%s\" to \"%s\": %s",
607-
to_fullpath_part, to_fullpath, strerror(errno));
619+
to_fullpath_part, to_fullpath, strerror(save_errno));
608620
}
609621

610622
pg_free(buf);
@@ -743,7 +755,8 @@ push_file_internal_gz(const char *wal_file_name, const char *pg_xlog_dir,
743755

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

748761
out = fio_gzopen(to_fullpath_gz_part, PG_BINARY_W, compress_level, FIO_BACKUP_HOST);
749762
if (out == NULL)
@@ -771,7 +784,8 @@ push_file_internal_gz(const char *wal_file_name, const char *pg_xlog_dir,
771784
/* cleanup */
772785
fclose(in);
773786
fio_gzclose(out);
774-
fio_unlink(to_fullpath_gz_part, FIO_BACKUP_HOST);
787+
if (fio_remove(to_fullpath_gz_part, false, FIO_BACKUP_HOST) != 0)
788+
elog(WARNING, "Cannot remove compressed temp WAL file \"%s\": %s", to_fullpath_gz_part, strerror(errno));
775789
return 1;
776790
}
777791
else
@@ -784,7 +798,8 @@ push_file_internal_gz(const char *wal_file_name, const char *pg_xlog_dir,
784798
/* Overwriting is forbidden,
785799
* so we must unlink partial file and exit with error.
786800
*/
787-
fio_unlink(to_fullpath_gz_part, FIO_BACKUP_HOST);
801+
if (fio_remove(to_fullpath_gz_part, false, FIO_BACKUP_HOST) != 0)
802+
elog(WARNING, "Cannot remove compressed temp WAL file \"%s\": %s", to_fullpath_gz_part, strerror(errno));
788803
elog(ERROR, "WAL file already exists in archive with "
789804
"different checksum: \"%s\"", to_fullpath_gz);
790805
}
@@ -801,16 +816,20 @@ push_file_internal_gz(const char *wal_file_name, const char *pg_xlog_dir,
801816

802817
if (ferror(in))
803818
{
804-
fio_unlink(to_fullpath_gz_part, FIO_BACKUP_HOST);
819+
int save_errno = errno;
820+
if (fio_remove(to_fullpath_gz_part, false, FIO_BACKUP_HOST) != 0)
821+
elog(WARNING, "Cannot remove compressed temp WAL file \"%s\": %s", to_fullpath_gz_part, strerror(errno));
805822
elog(ERROR, "Cannot read from source file \"%s\": %s",
806-
from_fullpath, strerror(errno));
823+
from_fullpath, strerror(save_errno));
807824
}
808825

809826
if (read_len > 0 && fio_gzwrite(out, buf, read_len) != read_len)
810827
{
811-
fio_unlink(to_fullpath_gz_part, FIO_BACKUP_HOST);
828+
int save_errno = errno;
829+
if (fio_remove(to_fullpath_gz_part, false, FIO_BACKUP_HOST) != 0)
830+
elog(WARNING, "Cannot cleanup compressed temp WAL file \"%s\": %s", to_fullpath_gz_part, strerror(errno));
812831
elog(ERROR, "Cannot write to compressed temp WAL file \"%s\": %s",
813-
to_fullpath_gz_part, get_gz_error(out, errno));
832+
to_fullpath_gz_part, get_gz_error(out, save_errno));
814833
}
815834

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

831851
/* close temp file, TODO: make it synchronous */
832852
if (fio_gzclose(out) != 0)
833853
{
834-
fio_unlink(to_fullpath_gz_part, FIO_BACKUP_HOST);
854+
int save_errno = errno;
855+
if (fio_remove(to_fullpath_gz_part, false, FIO_BACKUP_HOST) != 0)
856+
elog(WARNING, "Cannot cleanup compressed temp WAL file \"%s\": %s", to_fullpath_gz_part, strerror(errno));
835857
elog(ERROR, "Cannot close compressed temp WAL file \"%s\": %s",
836-
to_fullpath_gz_part, strerror(errno));
858+
to_fullpath_gz_part, strerror(save_errno));
837859
}
838860

839861
/* sync temp file to disk */
@@ -852,9 +874,11 @@ push_file_internal_gz(const char *wal_file_name, const char *pg_xlog_dir,
852874
/* Rename temp file to destination file */
853875
if (fio_rename(to_fullpath_gz_part, to_fullpath_gz, FIO_BACKUP_HOST) < 0)
854876
{
855-
fio_unlink(to_fullpath_gz_part, FIO_BACKUP_HOST);
877+
int save_errno = errno;
878+
if (fio_remove(to_fullpath_gz_part, false, FIO_BACKUP_HOST) != 0)
879+
elog(WARNING, "Cannot cleanup compressed temp WAL file \"%s\": %s", to_fullpath_gz_part, strerror(errno));
856880
elog(ERROR, "Cannot rename file \"%s\" to \"%s\": %s",
857-
to_fullpath_gz_part, to_fullpath_gz, strerror(errno));
881+
to_fullpath_gz_part, to_fullpath_gz, strerror(save_errno));
858882
}
859883

860884
pg_free(buf);

‎src/catalog.c

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* catalog.c: backup catalog operation
44
*
55
* Portions Copyright (c) 2009-2011, NIPPON TELEGRAPH AND TELEPHONE CORPORATION
6-
* Portions Copyright (c) 2015-2019, Postgres Professional
6+
* Portions Copyright (c) 2015-2022, Postgres Professional
77
*
88
*-------------------------------------------------------------------------
99
*/
@@ -451,7 +451,7 @@ grab_excl_lock_file(const char *root_dir, const char *backup_id, bool strict)
451451
* it. Need a loop because of possible race condition against other
452452
* would-be creators.
453453
*/
454-
if (fio_unlink(lock_file, FIO_BACKUP_HOST) < 0)
454+
if (fio_remove(lock_file, false, FIO_BACKUP_HOST) < 0)
455455
{
456456
if (errno == ENOENT)
457457
continue; /* race condition, again */
@@ -476,7 +476,8 @@ grab_excl_lock_file(const char *root_dir, const char *backup_id, bool strict)
476476
int save_errno = errno;
477477

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

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

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

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

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

516-
if (!strict && errno == ENOSPC)
519+
if (!strict && save_errno == ENOSPC)
517520
return LOCK_FAIL_ENOSPC;
518521
else
519522
elog(ERROR, "Could not close lock file \"%s\": %s",
@@ -608,8 +611,9 @@ wait_shared_owners(pgBackup *backup)
608611
return 1;
609612
}
610613

611-
/* unlink shared lock file */
612-
fio_unlink(lock_file, FIO_BACKUP_HOST);
614+
/* remove shared lock file */
615+
if (fio_remove(lock_file, true, FIO_BACKUP_HOST) != 0)
616+
elog(ERROR, "Cannot remove shared lock file \"%s\": %s", lock_file, strerror(errno));
613617
return 0;
614618
}
615619

@@ -727,8 +731,9 @@ release_excl_lock_file(const char *backup_dir)
727731

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

730-
/* unlink pid file */
731-
fio_unlink(lock_file, FIO_BACKUP_HOST);
734+
/* remove pid file */
735+
if (fio_remove(lock_file, false, FIO_BACKUP_HOST) != 0)
736+
elog(ERROR, "Cannot remove exclusive lock file \"%s\": %s", lock_file, strerror(errno));
732737
}
733738

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

@@ -2462,7 +2468,8 @@ write_backup(pgBackup *backup, bool strict)
24622468
if (!strict && (save_errno == ENOSPC))
24632469
{
24642470
fclose(fp);
2465-
fio_unlink(path_temp, FIO_BACKUP_HOST);
2471+
if (fio_remove(path_temp, false, FIO_BACKUP_HOST) != 0)
2472+
elog(elevel, "Additionally cannot remove file \"%s\": %s", path_temp, strerror(errno));
24662473
return;
24672474
}
24682475
}

‎src/catchup.c

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
*
33
* catchup.c: sync DB cluster
44
*
5-
* Copyright (c) 2021, Postgres Professional
5+
* Copyright (c) 2021-2022, Postgres Professional
66
*
77
*-------------------------------------------------------------------------
88
*/
@@ -930,8 +930,10 @@ do_catchup(const char *source_pgdata, const char *dest_pgdata, int num_threads,
930930
char fullpath[MAXPGPATH];
931931

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

936938
/* shrink dest pgdata list */
937939
pgFileFree(file);

‎src/delete.c

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* delete.c: delete backup files.
44
*
55
* Portions Copyright (c) 2009-2013, NIPPON TELEGRAPH AND TELEPHONE CORPORATION
6-
* Portions Copyright (c) 2015-2019, Postgres Professional
6+
* Portions Copyright (c) 2015-2022, Postgres Professional
77
*
88
*-------------------------------------------------------------------------
99
*/
@@ -782,7 +782,8 @@ delete_backup_files(pgBackup *backup)
782782
elog(INFO, "Progress: (%zd/%zd). Delete file \"%s\"",
783783
i + 1, num_files, full_path);
784784

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

788789
parray_walk(files, pgFileFree);
@@ -948,13 +949,11 @@ delete_walfiles_in_tli(InstanceState *instanceState, XLogRecPtr keep_lsn, timeli
948949
continue;
949950
}
950951

951-
/* unlink segment */
952-
if (fio_unlink(wal_fullpath, FIO_BACKUP_HOST) < 0)
952+
/* remove segment, missing file is not considered as error condition */
953+
if (fio_remove(wal_fullpath, true, FIO_BACKUP_HOST) < 0)
953954
{
954-
/* Missing file is not considered as error condition */
955-
if (errno != ENOENT)
956-
elog(ERROR, "Could not remove file \"%s\": %s",
957-
wal_fullpath, strerror(errno));
955+
elog(ERROR, "Could not remove file \"%s\": %s",
956+
wal_fullpath, strerror(errno));
958957
}
959958
else
960959
{

0 commit comments

Comments
 (0)