Jaime Casanova wrote:
> Found another problem with the this steps:
>
> create table t1 (i int);
> create index idx_t1_i on t1 using minmax(i);
> insert into t1 select generate_series(1, 2000000);
> ERROR: could not read block 1 in file "base/12645/16397_vm": read
> only 0 of 8192 bytes
Thanks. This was a trivial off-by-one bug; fixed in the attached patch.
While studying it, I noticed that I was also failing to notice extension
of the fork by another process. I have tried to fix that also in the
current patch, but I'm afraid that a fully robust solution for this will
involve having a cached fork size in the index's relcache entry -- just
like we have smgr_vm_nblocks. In fact, since the revmap fork is
currently reusing the VM forknum, I might even be able to use the same
variable to keep track of the fork size. But I don't really like this
bit of reusing the VM forknum for revmap, so I've refrained from
extending that assumption into further code for the time being.
There was also a bug that we would try to initialize a revmap page twice
during recovery, if two backends thought they needed to extend it; that
would cause the data written by the first extender to be lost.
This patch applies on top of the two previous incremental patches. I
will send a full patch later, including all those fixes and the fix for
the opr_sanity regression test.
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
*** a/src/backend/access/minmax/mmrevmap.c
--- b/src/backend/access/minmax/mmrevmap.c
***************
*** 30,36 ****
#define HEAPBLK_TO_REVMAP_INDEX(pagesPerRange, heapBlk) \
((heapBlk / pagesPerRange) % IDXITEMS_PER_PAGE)
! static void mmRevmapExtend(mmRevmapAccess *rmAccess, BlockNumber blkno);
/* typedef appears in minmax_revmap.h */
struct mmRevmapAccess
--- 30,36 ----
#define HEAPBLK_TO_REVMAP_INDEX(pagesPerRange, heapBlk) \
((heapBlk / pagesPerRange) % IDXITEMS_PER_PAGE)
! static bool mmRevmapExtend(mmRevmapAccess *rmAccess, BlockNumber blkno);
/* typedef appears in minmax_revmap.h */
struct mmRevmapAccess
***************
*** 52,62 **** mmRevmapAccessInit(Relation idxrel, BlockNumber pagesPerRange)
{
mmRevmapAccess *rmAccess = palloc(sizeof(mmRevmapAccess));
rmAccess->idxrel = idxrel;
rmAccess->pagesPerRange = pagesPerRange;
rmAccess->currBuf = InvalidBuffer;
rmAccess->physPagesInRevmap =
! RelationGetNumberOfBlocksInFork(idxrel, MM_REVMAP_FORKNUM);
return rmAccess;
}
--- 52,64 ----
{
mmRevmapAccess *rmAccess = palloc(sizeof(mmRevmapAccess));
+ RelationOpenSmgr(idxrel);
+
rmAccess->idxrel = idxrel;
rmAccess->pagesPerRange = pagesPerRange;
rmAccess->currBuf = InvalidBuffer;
rmAccess->physPagesInRevmap =
! smgrnblocks(idxrel->rd_smgr, MM_REVMAP_FORKNUM);
return rmAccess;
}
***************
*** 111,121 **** mmSetHeapBlockItemptr(mmRevmapAccess *rmAccess, BlockNumber heapBlk,
/*
* If the revmap is out of space, extend it first.
*/
! if (mapBlk > rmAccess->physPagesInRevmap - 1)
! {
! mmRevmapExtend(rmAccess, mapBlk);
! extend = true;
! }
/*
* Obtain the buffer from which we need to read. If we already have the
--- 113,120 ----
/*
* If the revmap is out of space, extend it first.
*/
! if (mapBlk >= rmAccess->physPagesInRevmap)
! extend = mmRevmapExtend(rmAccess, mapBlk);
/*
* Obtain the buffer from which we need to read. If we already have the
***************
*** 202,211 **** mmGetHeapBlockItemptr(mmRevmapAccess *rmAccess, BlockNumber heapBlk,
mapBlk = HEAPBLK_TO_REVMAP_BLK(rmAccess->pagesPerRange, heapBlk);
! if (mapBlk > rmAccess->physPagesInRevmap)
{
! ItemPointerSetInvalid(out);
! return;
}
if (rmAccess->currBuf == InvalidBuffer ||
--- 201,229 ----
mapBlk = HEAPBLK_TO_REVMAP_BLK(rmAccess->pagesPerRange, heapBlk);
! /*
! * If we are asked for a block of the map which is beyond what we know
! * about it, try to see if our fork has grown since we last checked its
! * size; a concurrent inserter could have extended it.
! */
! if (mapBlk >= rmAccess->physPagesInRevmap)
{
! RelationOpenSmgr(rmAccess->idxrel);
! LockRelationForExtension(rmAccess->idxrel, ShareLock);
! rmAccess->physPagesInRevmap =
! smgrnblocks(rmAccess->idxrel->rd_smgr, MM_REVMAP_FORKNUM);
!
! if (mapBlk >= rmAccess->physPagesInRevmap)
! {
! /* definitely not in range */
!
! UnlockRelationForExtension(rmAccess->idxrel, ShareLock);
! ItemPointerSetInvalid(out);
! return;
! }
!
! /* the block exists now, proceed */
! UnlockRelationForExtension(rmAccess->idxrel, ShareLock);
}
if (rmAccess->currBuf == InvalidBuffer ||
***************
*** 273,286 **** mmRevmapCreate(Relation idxrel)
}
/*
! * Extend the reverse range map to cover the given block number.
*
* NB -- caller is responsible for ensuring this action is properly WAL-logged.
*/
! static void
mmRevmapExtend(mmRevmapAccess *rmAccess, BlockNumber blkno)
{
char page[BLCKSZ];
MemSet(page, 0, sizeof(page));
PageInit(page, BLCKSZ, 0);
--- 291,307 ----
}
/*
! * Extend the reverse range map to cover the given block number. Return false
! * if the map already covered the requested range (no extension actually done),
! * true otherwise.
*
* NB -- caller is responsible for ensuring this action is properly WAL-logged.
*/
! static bool
mmRevmapExtend(mmRevmapAccess *rmAccess, BlockNumber blkno)
{
char page[BLCKSZ];
+ bool extended = false;
MemSet(page, 0, sizeof(page));
PageInit(page, BLCKSZ, 0);
***************
*** 291,316 **** mmRevmapExtend(mmRevmapAccess *rmAccess, BlockNumber blkno)
* first, refresh our idea of the current size; it might well have grown
* up to what we need since we last checked.
*/
rmAccess->physPagesInRevmap =
! RelationGetNumberOfBlocksInFork(rmAccess->idxrel,
! MM_REVMAP_FORKNUM);
/*
* Now extend it one page at a time. This might seem a bit inefficient,
* but normally we'd be extending for a single page anyway.
*/
! while (blkno > rmAccess->physPagesInRevmap - 1)
{
smgrextend(rmAccess->idxrel->rd_smgr, MM_REVMAP_FORKNUM,
rmAccess->physPagesInRevmap, page, false);
rmAccess->physPagesInRevmap++;
}
Assert(rmAccess->physPagesInRevmap ==
! RelationGetNumberOfBlocksInFork(rmAccess->idxrel,
! MM_REVMAP_FORKNUM));
UnlockRelationForExtension(rmAccess->idxrel, ExclusiveLock);
}
/*
--- 312,339 ----
* first, refresh our idea of the current size; it might well have grown
* up to what we need since we last checked.
*/
+ RelationOpenSmgr(rmAccess->idxrel);
rmAccess->physPagesInRevmap =
! smgrnblocks(rmAccess->idxrel->rd_smgr, MM_REVMAP_FORKNUM);
/*
* Now extend it one page at a time. This might seem a bit inefficient,
* but normally we'd be extending for a single page anyway.
*/
! while (blkno >= rmAccess->physPagesInRevmap)
{
+ extended = true;
smgrextend(rmAccess->idxrel->rd_smgr, MM_REVMAP_FORKNUM,
rmAccess->physPagesInRevmap, page, false);
rmAccess->physPagesInRevmap++;
}
Assert(rmAccess->physPagesInRevmap ==
! smgrnblocks(rmAccess->idxrel->rd_smgr, MM_REVMAP_FORKNUM));
UnlockRelationForExtension(rmAccess->idxrel, ExclusiveLock);
+
+ return extended;
}
/*
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
