From ea6bd2b96e9d8c499b7c926f5f4b05b39dfbc540 Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Fri, 22 Apr 2016 09:45:26 -0700 Subject: [PATCH] Don't open formally non-existant segments in _mdfd_getseg(). Discussion: CAA-aLv6Dp_ZsV-44QA-2zgkqWKQq=GedBX2dRSrWpxqovXK=Pg@mail.gmail.com Fixes: 428b1d6b29ca599c5700d4bc4f4ce4c5880369bf --- src/backend/storage/smgr/md.c | 86 ++++++++++++++++++++++++++++++------------- 1 file changed, 60 insertions(+), 26 deletions(-) diff --git a/src/backend/storage/smgr/md.c b/src/backend/storage/smgr/md.c index 578276d..89a20d1 100644 --- a/src/backend/storage/smgr/md.c +++ b/src/backend/storage/smgr/md.c @@ -165,9 +165,14 @@ static CycleCtr mdckpt_cycle_ctr = 0; typedef enum /* behavior for mdopen & _mdfd_getseg */ { - EXTENSION_FAIL, /* ereport if segment not present */ - EXTENSION_RETURN_NULL, /* return NULL if not present */ - EXTENSION_CREATE /* create new segments as needed */ + /* ereport if segment not present, create in recovery */ + EXTENSION_FAIL, + /* return NULL if not present, create in recovery */ + EXTENSION_RETURN_NULL, + /* return NULL if not present */ + EXTENSION_REALLY_RETURN_NULL, + /* create new segments as needed */ + EXTENSION_CREATE } ExtensionBehavior; /* local routines */ @@ -591,7 +596,8 @@ mdopen(SMgrRelation reln, ForkNumber forknum, ExtensionBehavior behavior) fd = PathNameOpenFile(path, O_RDWR | O_CREAT | O_EXCL | PG_BINARY, 0600); if (fd < 0) { - if (behavior == EXTENSION_RETURN_NULL && + if ((behavior == EXTENSION_RETURN_NULL || + behavior == EXTENSION_REALLY_RETURN_NULL) && FILE_POSSIBLY_DELETED(errno)) { pfree(path); @@ -685,7 +691,7 @@ mdwriteback(SMgrRelation reln, ForkNumber forknum, segnum_end; v = _mdfd_getseg(reln, forknum, blocknum, false, - EXTENSION_RETURN_NULL); + EXTENSION_REALLY_RETURN_NULL); /* * We might be flushing buffers of already removed relations, that's @@ -1774,7 +1780,7 @@ _mdfd_getseg(SMgrRelation reln, ForkNumber forknum, BlockNumber blkno, BlockNumber nextsegno; if (!v) - return NULL; /* only possible if EXTENSION_RETURN_NULL */ + return NULL; /* if EXTENSION_(REALLY_)RETURN_NULL */ targetseg = blkno / ((BlockNumber) RELSEG_SIZE); for (nextsegno = 1; nextsegno <= targetseg; nextsegno++) @@ -1783,23 +1789,30 @@ _mdfd_getseg(SMgrRelation reln, ForkNumber forknum, BlockNumber blkno, if (v->mdfd_chain == NULL) { - /* - * Normally we will create new segments only if authorized by the - * caller (i.e., we are doing mdextend()). But when doing WAL - * recovery, create segments anyway; this allows cases such as - * replaying WAL data that has a write into a high-numbered - * segment of a relation that was later deleted. We want to go - * ahead and create the segments so we can finish out the replay. - * - * We have to maintain the invariant that segments before the last - * active segment are of size RELSEG_SIZE; therefore, pad them out - * with zeroes if needed. (This only matters if caller is - * extending the relation discontiguously, but that can happen in - * hash indexes.) - */ - if (behavior == EXTENSION_CREATE || InRecovery) + BlockNumber nblocks = _mdnblocks(reln, forknum, v); + + if (nblocks > ((BlockNumber) RELSEG_SIZE)) + elog(FATAL, "segment too big"); + + if (behavior == EXTENSION_CREATE || + (InRecovery && behavior != EXTENSION_REALLY_RETURN_NULL)) { - if (_mdnblocks(reln, forknum, v) < RELSEG_SIZE) + /* + * Normally we will create new segments only if authorized by + * the caller (i.e., we are doing mdextend()). But when doing + * WAL recovery, create segments anyway; this allows cases + * such as replaying WAL data that has a write into a + * high-numbered segment of a relation that was later deleted. + * We want to go ahead and create the segments so we can + * finish out the replay. + * + * We have to maintain the invariant that segments before the + * last active segment are of size RELSEG_SIZE; therefore, if + * extending, pad them out with zeroes if needed. (This only + * matters if caller is extending the relation + * discontiguously, but that can happen in hash indexes.) + */ + if (nblocks < ((BlockNumber) RELSEG_SIZE)) { char *zerobuf = palloc0(BLCKSZ); @@ -1810,14 +1823,35 @@ _mdfd_getseg(SMgrRelation reln, ForkNumber forknum, BlockNumber blkno, } v->mdfd_chain = _mdfd_openseg(reln, forknum, +nextsegno, O_CREAT); } - else + else if (nblocks < ((BlockNumber) RELSEG_SIZE)) { - /* We won't create segment if not existent */ - v->mdfd_chain = _mdfd_openseg(reln, forknum, nextsegno, 0); + /* + * When not extending, only open the next segment if the + * current one is exactly RELSEG_SIZE. + */ + if (behavior == EXTENSION_RETURN_NULL || + behavior == EXTENSION_REALLY_RETURN_NULL) + { + errno = ENOENT; /* some callers check errno, yuck */ + return NULL; + } + else + { + ereport(ERROR, + (errcode_for_file_access(), + errmsg("could not open file \"%s\" (target block %u): previous segment is only %u blocks", + _mdfd_segpath(reln, forknum, nextsegno), + blkno, nblocks))); + } } + + /* We won't create segment if not existent */ + v->mdfd_chain = _mdfd_openseg(reln, forknum, nextsegno, 0); + if (v->mdfd_chain == NULL) { - if (behavior == EXTENSION_RETURN_NULL && + if ((behavior == EXTENSION_RETURN_NULL || + behavior == EXTENSION_REALLY_RETURN_NULL) && FILE_POSSIBLY_DELETED(errno)) return NULL; ereport(ERROR, -- 2.7.0.229.g701fa7f