Skip to content

Commit db74d1b

Browse files
author
Sokolov Yura
committed
cfs: use PG_ENSURE_ERROR_CLEANUP for cfs_control_gc_unlock
I don't really understand if it is really needed. Just sanity check.
1 parent a1ceeec commit db74d1b

File tree

4 files changed

+28
-40
lines changed

4 files changed

+28
-40
lines changed

‎src/backend/storage/file/cfs.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1508,7 +1508,7 @@ void cfs_control_gc_lock(void)
15081508
}
15091509

15101510
/* Enable garbage collection. */
1511-
void cfs_control_gc_unlock(void)
1511+
void cfs_control_gc_unlock() /* argument could be given by PG_ENSURE_ERROR_CLEANUP */
15121512
{
15131513
pg_atomic_fetch_sub_u32(&cfs_state->gc_disabled, 1);
15141514
}

‎src/backend/storage/file/copydir.c

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#include "storage/copydir.h"
2626
#include "storage/fd.h"
2727
#include "storage/cfs.h"
28+
#include "storage/ipc.h"
2829
#include "access/transam.h"
2930
#include "miscadmin.h"
3031

@@ -56,7 +57,7 @@ copydir(char *fromdir, char *todir, bool recurse)
5657

5758
cfs_control_gc_lock(); /* disable GC during copy */
5859

59-
PG_TRY();
60+
PG_ENSURE_ERROR_CLEANUP(cfs_control_gc_unlock, (Datum)NULL);
6061
{
6162
while ((xlde = ReadDir(xldir, fromdir)) != NULL)
6263
{
@@ -88,12 +89,7 @@ copydir(char *fromdir, char *todir, bool recurse)
8889
}
8990
FreeDir(xldir);
9091
}
91-
PG_CATCH();
92-
{
93-
cfs_control_gc_unlock();
94-
PG_RE_THROW();
95-
}
96-
PG_END_TRY();
92+
PG_END_ENSURE_ERROR_CLEANUP(cfs_control_gc_unlock, (Datum)NULL);
9793
cfs_control_gc_unlock();
9894

9995
/*
@@ -167,7 +163,7 @@ copyzipdir(char *fromdir, bool from_compressed,
167163

168164
cfs_control_gc_lock(); /* disable GC during copy */
169165

170-
PG_TRY();
166+
PG_ENSURE_ERROR_CLEANUP(cfs_control_gc_unlock, (Datum)NULL);
171167
{
172168
while ((xlde = ReadDir(xldir, fromdir)) != NULL)
173169
{
@@ -195,12 +191,7 @@ copyzipdir(char *fromdir, bool from_compressed,
195191
}
196192
FreeDir(xldir);
197193
}
198-
PG_CATCH();
199-
{
200-
cfs_control_gc_unlock();
201-
PG_RE_THROW();
202-
}
203-
PG_END_TRY();
194+
PG_END_ENSURE_ERROR_CLEANUP(cfs_control_gc_unlock, (Datum)NULL);
204195
cfs_control_gc_unlock();
205196

206197
/*

‎src/backend/storage/smgr/md.c

Lines changed: 21 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
#include "storage/fd.h"
3636
#include "storage/cfs.h"
3737
#include "storage/bufmgr.h"
38+
#include "storage/ipc.h"
3839
#include "storage/relfilenode.h"
3940
#include "storage/smgr.h"
4041
#include "utils/hsearch.h"
@@ -401,6 +402,19 @@ mdcreate(SMgrRelation reln, ForkNumber forkNum, bool isRedo)
401402
reln->md_fd[forkNum]->mdfd_chain = NULL;
402403
}
403404

405+
static void
406+
do_mdunlink(RelFileNodeBackend rnode, ForkNumber forkNum, bool isRedo)
407+
{
408+
/* Now do the per-fork work */
409+
if (forkNum == InvalidForkNumber)
410+
{
411+
for (forkNum = 0; forkNum <= MAX_FORKNUM; forkNum++)
412+
mdunlinkfork(rnode, forkNum, isRedo);
413+
}
414+
else
415+
mdunlinkfork(rnode, forkNum, isRedo);
416+
}
417+
404418
/*
405419
* mdunlink() -- Unlink a relation.
406420
*
@@ -451,7 +465,6 @@ mdcreate(SMgrRelation reln, ForkNumber forkNum, bool isRedo)
451465
void
452466
mdunlink(RelFileNodeBackend rnode, ForkNumber forkNum, bool isRedo)
453467
{
454-
bool cfs_gc_locked = false;
455468
/*
456469
* We have to clean out any pending fsync requests for the doomed
457470
* relation, else the next mdsync() will fail. There can't be any such
@@ -462,34 +475,18 @@ mdunlink(RelFileNodeBackend rnode, ForkNumber forkNum, bool isRedo)
462475
if (!RelFileNodeBackendIsTemp(rnode))
463476
ForgetRelationFsyncRequests(rnode.node, forkNum);
464477

465-
if (md_use_compression(rnode, forkNum == InvalidForkNumber ? MAIN_FORKNUM : forkNum))
478+
if (!md_use_compression(rnode, forkNum == InvalidForkNumber ? MAIN_FORKNUM : forkNum))
479+
do_mdunlink(rnode, forkNum, isRedo);
480+
else
466481
{
467-
cfs_gc_locked = true;
468482
cfs_control_gc_lock();
469-
}
470-
471-
PG_TRY();
472-
{
473-
/* Now do the per-fork work */
474-
if (forkNum == InvalidForkNumber)
475-
{
476-
for (forkNum = 0; forkNum <= MAX_FORKNUM; forkNum++)
477-
mdunlinkfork(rnode, forkNum, isRedo);
478-
}
479-
else
483+
PG_ENSURE_ERROR_CLEANUP(cfs_control_gc_unlock, BoolGetDatum(false));
480484
{
481-
mdunlinkfork(rnode, forkNum, isRedo);
485+
do_mdunlink(rnode, forkNum, isRedo);
482486
}
483-
}
484-
PG_CATCH();
485-
{
486-
if (cfs_gc_locked)
487-
cfs_control_gc_unlock();
488-
PG_RE_THROW();
489-
}
490-
PG_END_TRY();
491-
if (cfs_gc_locked)
487+
PG_END_ENSURE_ERROR_CLEANUP(cfs_control_gc_unlock, BoolGetDatum(false));
492488
cfs_control_gc_unlock();
489+
}
493490
}
494491

495492
static void

‎src/include/storage/cfs.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ void cfs_unlock_file(FileMap* map, char const* path);
130130
uint32 cfs_alloc_page(FileMap* map, uint32 oldSize, uint32 newSize);
131131
void cfs_extend(FileMap* map, uint32 pos);
132132
void cfs_control_gc_lock(void);
133-
void cfs_control_gc_unlock(void);
133+
void cfs_control_gc_unlock(); /* argument could be given by PG_ENSURE_ERROR_CLEANUP */
134134
int cfs_msync(FileMap* map);
135135
FileMap* cfs_mmap(int md);
136136
int cfs_munmap(FileMap* map);

0 commit comments

Comments
 (0)