Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit a712487

Browse files
committed
Fix transient mdsync() errors of truncated relations due to 72a98a6.
Unfortunately the segment size checks from 72a98a6 had the negative side-effect of breaking a corner case in mdsync(): When processing a fsync request for a truncated away segment mdsync() could fail with "could not fsync file" (if previous segment < RELSEG_SIZE) because _mdfd_getseg() now wouldn't return the relevant segment anymore. The cleanest fix seems to be to allow the caller of _mdfd_getseg() to specify whether checks for RELSEG_SIZE are performed. To allow doing so, change the ExtensionBehavior enum into a bitmask. Besides allowing for the addition of EXTENSION_DONT_CHECK_SIZE, this makes for a nicer implementation of EXTENSION_REALLY_RETURN_NULL. Besides mdsync() the only callsite that should change behaviour due to this is mdprefetch() which now doesn't create segments anymore, even in recovery. Given the uses of mdprefetch() that seems better. Reported-By: Thom Brown Discussion: CAA-aLv72QazLvPdKZYpVn4a_Eh+i4_cxuB03k+iCuZM_xjc+6Q@mail.gmail.com
1 parent 613fb29 commit a712487

File tree

1 file changed

+47
-34
lines changed
  • src/backend/storage/smgr

1 file changed

+47
-34
lines changed

src/backend/storage/smgr/md.c

+47-34
Original file line numberDiff line numberDiff line change
@@ -163,23 +163,29 @@ static CycleCtr mdsync_cycle_ctr = 0;
163163
static CycleCtr mdckpt_cycle_ctr = 0;
164164

165165

166-
typedef enum /* behavior for mdopen & _mdfd_getseg */
167-
{
168-
/* ereport if segment not present, create in recovery */
169-
EXTENSION_FAIL,
170-
/* return NULL if not present, create in recovery */
171-
EXTENSION_RETURN_NULL,
172-
/* return NULL if not present */
173-
EXTENSION_REALLY_RETURN_NULL,
174-
/* create new segments as needed */
175-
EXTENSION_CREATE
176-
} ExtensionBehavior;
166+
/*** behavior for mdopen & _mdfd_getseg ***/
167+
/* ereport if segment not present */
168+
#define EXTENSION_FAIL (1 << 0)
169+
/* return NULL if segment not present */
170+
#define EXTENSION_RETURN_NULL (1 << 1)
171+
/* create new segments as needed */
172+
#define EXTENSION_CREATE (1 << 2)
173+
/* create new segments if needed during recovery */
174+
#define EXTENSION_CREATE_RECOVERY (1 << 3)
175+
/*
176+
* Allow opening segments which are preceded by segments smaller than
177+
* RELSEG_SIZE, e.g. inactive segments (see above). Note that this is breaks
178+
* mdnblocks() and related functionality henceforth - which currently is ok,
179+
* because this is only required in the checkpointer which never uses
180+
* mdnblocks().
181+
*/
182+
#define EXTENSION_DONT_CHECK_SIZE (1 << 4)
183+
177184

178185
/* local routines */
179186
static void mdunlinkfork(RelFileNodeBackend rnode, ForkNumber forkNum,
180187
bool isRedo);
181-
static MdfdVec *mdopen(SMgrRelation reln, ForkNumber forknum,
182-
ExtensionBehavior behavior);
188+
static MdfdVec *mdopen(SMgrRelation reln, ForkNumber forknum, int behavior);
183189
static void register_dirty_segment(SMgrRelation reln, ForkNumber forknum,
184190
MdfdVec *seg);
185191
static void register_unlink(RelFileNodeBackend rnode);
@@ -189,7 +195,7 @@ static char *_mdfd_segpath(SMgrRelation reln, ForkNumber forknum,
189195
static MdfdVec *_mdfd_openseg(SMgrRelation reln, ForkNumber forkno,
190196
BlockNumber segno, int oflags);
191197
static MdfdVec *_mdfd_getseg(SMgrRelation reln, ForkNumber forkno,
192-
BlockNumber blkno, bool skipFsync, ExtensionBehavior behavior);
198+
BlockNumber blkno, bool skipFsync, int behavior);
193199
static BlockNumber _mdnblocks(SMgrRelation reln, ForkNumber forknum,
194200
MdfdVec *seg);
195201

@@ -570,7 +576,7 @@ mdextend(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum,
570576
* invent one out of whole cloth.
571577
*/
572578
static MdfdVec *
573-
mdopen(SMgrRelation reln, ForkNumber forknum, ExtensionBehavior behavior)
579+
mdopen(SMgrRelation reln, ForkNumber forknum, int behavior)
574580
{
575581
MdfdVec *mdfd;
576582
char *path;
@@ -596,8 +602,7 @@ mdopen(SMgrRelation reln, ForkNumber forknum, ExtensionBehavior behavior)
596602
fd = PathNameOpenFile(path, O_RDWR | O_CREAT | O_EXCL | PG_BINARY, 0600);
597603
if (fd < 0)
598604
{
599-
if ((behavior == EXTENSION_RETURN_NULL ||
600-
behavior == EXTENSION_REALLY_RETURN_NULL) &&
605+
if ((behavior & EXTENSION_RETURN_NULL) &&
601606
FILE_POSSIBLY_DELETED(errno))
602607
{
603608
pfree(path);
@@ -690,8 +695,8 @@ mdwriteback(SMgrRelation reln, ForkNumber forknum,
690695
int segnum_start,
691696
segnum_end;
692697

693-
v = _mdfd_getseg(reln, forknum, blocknum, false,
694-
EXTENSION_REALLY_RETURN_NULL);
698+
v = _mdfd_getseg(reln, forknum, blocknum, true /* not used */ ,
699+
EXTENSION_RETURN_NULL);
695700

696701
/*
697702
* We might be flushing buffers of already removed relations, that's
@@ -737,7 +742,8 @@ mdread(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum,
737742
reln->smgr_rnode.node.relNode,
738743
reln->smgr_rnode.backend);
739744

740-
v = _mdfd_getseg(reln, forknum, blocknum, false, EXTENSION_FAIL);
745+
v = _mdfd_getseg(reln, forknum, blocknum, false,
746+
EXTENSION_FAIL | EXTENSION_CREATE_RECOVERY);
741747

742748
seekpos = (off_t) BLCKSZ *(blocknum % ((BlockNumber) RELSEG_SIZE));
743749

@@ -812,7 +818,8 @@ mdwrite(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum,
812818
reln->smgr_rnode.node.relNode,
813819
reln->smgr_rnode.backend);
814820

815-
v = _mdfd_getseg(reln, forknum, blocknum, skipFsync, EXTENSION_FAIL);
821+
v = _mdfd_getseg(reln, forknum, blocknum, skipFsync,
822+
EXTENSION_FAIL | EXTENSION_CREATE_RECOVERY);
816823

817824
seekpos = (off_t) BLCKSZ *(blocknum % ((BlockNumber) RELSEG_SIZE));
818825

@@ -1219,7 +1226,9 @@ mdsync(void)
12191226
/* Attempt to open and fsync the target segment */
12201227
seg = _mdfd_getseg(reln, forknum,
12211228
(BlockNumber) segno * (BlockNumber) RELSEG_SIZE,
1222-
false, EXTENSION_RETURN_NULL);
1229+
false,
1230+
EXTENSION_RETURN_NULL
1231+
| EXTENSION_DONT_CHECK_SIZE);
12231232

12241233
INSTR_TIME_SET_CURRENT(sync_start);
12251234

@@ -1773,14 +1782,18 @@ _mdfd_openseg(SMgrRelation reln, ForkNumber forknum, BlockNumber segno,
17731782
*/
17741783
static MdfdVec *
17751784
_mdfd_getseg(SMgrRelation reln, ForkNumber forknum, BlockNumber blkno,
1776-
bool skipFsync, ExtensionBehavior behavior)
1785+
bool skipFsync, int behavior)
17771786
{
17781787
MdfdVec *v = mdopen(reln, forknum, behavior);
17791788
BlockNumber targetseg;
17801789
BlockNumber nextsegno;
17811790

1791+
/* some way to handle non-existant segments needs to be specified */
1792+
Assert(behavior &
1793+
(EXTENSION_FAIL | EXTENSION_CREATE | EXTENSION_RETURN_NULL));
1794+
17821795
if (!v)
1783-
return NULL; /* if EXTENSION_(REALLY_)RETURN_NULL */
1796+
return NULL; /* if behavior & EXTENSION_RETURN_NULL */
17841797

17851798
targetseg = blkno / ((BlockNumber) RELSEG_SIZE);
17861799
for (nextsegno = 1; nextsegno <= targetseg; nextsegno++)
@@ -1795,8 +1808,8 @@ _mdfd_getseg(SMgrRelation reln, ForkNumber forknum, BlockNumber blkno,
17951808
if (nblocks > ((BlockNumber) RELSEG_SIZE))
17961809
elog(FATAL, "segment too big");
17971810

1798-
if (behavior == EXTENSION_CREATE ||
1799-
(InRecovery && behavior != EXTENSION_REALLY_RETURN_NULL))
1811+
if ((behavior & EXTENSION_CREATE) ||
1812+
(InRecovery && (behavior & EXTENSION_CREATE_RECOVERY)))
18001813
{
18011814
/*
18021815
* Normally we will create new segments only if authorized by
@@ -1827,15 +1840,16 @@ _mdfd_getseg(SMgrRelation reln, ForkNumber forknum, BlockNumber blkno,
18271840
}
18281841
flags = O_CREAT;
18291842
}
1830-
else if (nblocks < ((BlockNumber) RELSEG_SIZE))
1843+
else if (!(behavior & EXTENSION_DONT_CHECK_SIZE) &&
1844+
nblocks < ((BlockNumber) RELSEG_SIZE))
18311845
{
18321846
/*
1833-
* When not extending, only open the next segment if the
1834-
* current one is exactly RELSEG_SIZE. If not (this branch),
1835-
* either return NULL or fail.
1847+
* When not extending (or explicitly including truncated
1848+
* segments), only open the next segment if the current one is
1849+
* exactly RELSEG_SIZE. If not (this branch), either return
1850+
* NULL or fail.
18361851
*/
1837-
if (behavior == EXTENSION_RETURN_NULL ||
1838-
behavior == EXTENSION_REALLY_RETURN_NULL)
1852+
if (behavior & EXTENSION_RETURN_NULL)
18391853
{
18401854
/*
18411855
* Some callers discern between reasons for _mdfd_getseg()
@@ -1858,8 +1872,7 @@ _mdfd_getseg(SMgrRelation reln, ForkNumber forknum, BlockNumber blkno,
18581872

18591873
if (v->mdfd_chain == NULL)
18601874
{
1861-
if ((behavior == EXTENSION_RETURN_NULL ||
1862-
behavior == EXTENSION_REALLY_RETURN_NULL) &&
1875+
if ((behavior & EXTENSION_RETURN_NULL) &&
18631876
FILE_POSSIBLY_DELETED(errno))
18641877
return NULL;
18651878
ereport(ERROR,

0 commit comments

Comments
 (0)