Skip to content

Commit ab1aa37

Browse files
kulaginmGrigory Smolkin
kulaginm
authored and
Grigory Smolkin
committed
[refactoring] remove duplicate fio_unlink() and fio_delete() and merge into new fio_remove() with proper error checking
1 parent b95cf02 commit ab1aa37

File tree

10 files changed

+153
-148
lines changed

10 files changed

+153
-148
lines changed

‎src/archive.c‎

Lines changed: 48 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -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)
@@ -770,7 +783,8 @@ push_file_internal_gz(const char *wal_file_name, const char *pg_xlog_dir,
770783
/* cleanup */
771784
fclose(in);
772785
fio_gzclose(out);
773-
fio_unlink(to_fullpath_gz_part, FIO_BACKUP_HOST);
786+
if (fio_remove(to_fullpath_gz_part, false, FIO_BACKUP_HOST) != 0)
787+
elog(WARNING, "Cannot remove compressed temp WAL file \"%s\": %s", to_fullpath_gz_part, strerror(errno));
774788
return 1;
775789
}
776790
else
@@ -783,7 +797,8 @@ push_file_internal_gz(const char *wal_file_name, const char *pg_xlog_dir,
783797
/* Overwriting is forbidden,
784798
* so we must unlink partial file and exit with error.
785799
*/
786-
fio_unlink(to_fullpath_gz_part, FIO_BACKUP_HOST);
800+
if (fio_remove(to_fullpath_gz_part, false, FIO_BACKUP_HOST) != 0)
801+
elog(WARNING, "Cannot remove compressed temp WAL file \"%s\": %s", to_fullpath_gz_part, strerror(errno));
787802
elog(ERROR, "WAL file already exists in archive with "
788803
"different checksum: \"%s\"", to_fullpath_gz);
789804
}
@@ -800,16 +815,20 @@ push_file_internal_gz(const char *wal_file_name, const char *pg_xlog_dir,
800815

801816
if (ferror(in))
802817
{
803-
fio_unlink(to_fullpath_gz_part, FIO_BACKUP_HOST);
818+
int save_errno = errno;
819+
if (fio_remove(to_fullpath_gz_part, false, FIO_BACKUP_HOST) != 0)
820+
elog(WARNING, "Cannot remove compressed temp WAL file \"%s\": %s", to_fullpath_gz_part, strerror(errno));
804821
elog(ERROR, "Cannot read from source file \"%s\": %s",
805-
from_fullpath, strerror(errno));
822+
from_fullpath, strerror(save_errno));
806823
}
807824

808825
if (read_len > 0 && fio_gzwrite(out, buf, read_len) != read_len)
809826
{
810-
fio_unlink(to_fullpath_gz_part, FIO_BACKUP_HOST);
827+
int save_errno = errno;
828+
if (fio_remove(to_fullpath_gz_part, false, FIO_BACKUP_HOST) != 0)
829+
elog(WARNING, "Cannot cleanup compressed temp WAL file \"%s\": %s", to_fullpath_gz_part, strerror(errno));
811830
elog(ERROR, "Cannot write to compressed temp WAL file \"%s\": %s",
812-
to_fullpath_gz_part, get_gz_error(out, errno));
831+
to_fullpath_gz_part, get_gz_error(out, save_errno));
813832
}
814833

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

830850
/* close temp file, TODO: make it synchronous */
831851
if (fio_gzclose(out) != 0)
832852
{
833-
fio_unlink(to_fullpath_gz_part, FIO_BACKUP_HOST);
853+
int save_errno = errno;
854+
if (fio_remove(to_fullpath_gz_part, false, FIO_BACKUP_HOST) != 0)
855+
elog(WARNING, "Cannot cleanup compressed temp WAL file \"%s\": %s", to_fullpath_gz_part, strerror(errno));
834856
elog(ERROR, "Cannot close compressed temp WAL file \"%s\": %s",
835-
to_fullpath_gz_part, strerror(errno));
857+
to_fullpath_gz_part, strerror(save_errno));
836858
}
837859

838860
/* sync temp file to disk */
@@ -851,9 +873,11 @@ push_file_internal_gz(const char *wal_file_name, const char *pg_xlog_dir,
851873
/* Rename temp file to destination file */
852874
if (fio_rename(to_fullpath_gz_part, to_fullpath_gz, FIO_BACKUP_HOST) < 0)
853875
{
854-
fio_unlink(to_fullpath_gz_part, FIO_BACKUP_HOST);
876+
int save_errno = errno;
877+
if (fio_remove(to_fullpath_gz_part, false, FIO_BACKUP_HOST) != 0)
878+
elog(WARNING, "Cannot cleanup compressed temp WAL file \"%s\": %s", to_fullpath_gz_part, strerror(errno));
855879
elog(ERROR, "Cannot rename file \"%s\" to \"%s\": %s",
856-
to_fullpath_gz_part, to_fullpath_gz, strerror(errno));
880+
to_fullpath_gz_part, to_fullpath_gz, strerror(save_errno));
857881
}
858882

859883
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

@@ -2480,7 +2486,8 @@ write_backup(pgBackup *backup, bool strict)
24802486
if (!strict && (save_errno == ENOSPC))
24812487
{
24822488
fclose(fp);
2483-
fio_unlink(path_temp, FIO_BACKUP_HOST);
2489+
if (fio_remove(path_temp, false, FIO_BACKUP_HOST) != 0)
2490+
elog(elevel, "Additionally cannot remove file \"%s\": %s", path_temp, strerror(errno));
24842491
return;
24852492
}
24862493
}

‎src/catchup.c‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -942,11 +942,11 @@ do_catchup(const char *source_pgdata, const char *dest_pgdata, int num_threads,
942942
char fullpath[MAXPGPATH];
943943

944944
join_path_components(fullpath, dest_pgdata, file->rel_path);
945-
if (!dry_run)
946-
{
947-
fio_delete(file->mode, fullpath, FIO_LOCAL_HOST);
948-
}
949-
elog(LOG, "Deleted file \"%s\"", fullpath);
945+
946+
if (dry_run || fio_remove(fullpath, false, FIO_LOCAL_HOST) == 0)
947+
elog(VERBOSE, "Deleted file \"%s\"", fullpath);
948+
else
949+
elog(ERROR, "Cannot delete redundant file in destination \"%s\": %s", fullpath, strerror(errno));
950950

951951
/* shrink dest pgdata list */
952952
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
*/
@@ -775,7 +775,8 @@ delete_backup_files(pgBackup *backup)
775775
elog(INFO, "Progress: (%zd/%zd). Delete file \"%s\"",
776776
i + 1, num_files, full_path);
777777

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

781782
parray_walk(files, pgFileFree);
@@ -941,13 +942,11 @@ delete_walfiles_in_tli(InstanceState *instanceState, XLogRecPtr keep_lsn, timeli
941942
continue;
942943
}
943944

944-
/* unlink segment */
945-
if (fio_unlink(wal_fullpath, FIO_BACKUP_HOST) < 0)
945+
/* remove segment, missing file is not considered as error condition */
946+
if (fio_remove(wal_fullpath, true, FIO_BACKUP_HOST) < 0)
946947
{
947-
/* Missing file is not considered as error condition */
948-
if (errno != ENOENT)
949-
elog(ERROR, "Could not remove file \"%s\": %s",
950-
wal_fullpath, strerror(errno));
948+
elog(ERROR, "Could not remove file \"%s\": %s",
949+
wal_fullpath, strerror(errno));
951950
}
952951
else
953952
{

0 commit comments

Comments
 (0)