Greg Stark <st...@mit.edu> writes:
> So here's my attempt to rewrite this logic. I ended up refactoring a
> bit because I found it unnecessarily confusing having the mode
> branches in several places. I think it's much clearer just having two
> separate pieces of logic for RBM_NEW and the extension cases since all
> they have in common is the ReadBuffer call.

I don't like that at all.  It's a lot of unnecessary churn in what is
indeed hard-to-test code, and personally I don't find it clearer.
Nor, if you're going to complain about the cost of smgrnblocks, does
it seem like a great idea to be doing that *twice* per page rather
than once.

How about the attached instead?

                        regards, tom lane

diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index 4cd82df..f1918f6 100644
*** a/src/backend/access/transam/xlogutils.c
--- b/src/backend/access/transam/xlogutils.c
*************** XLogReadBufferExtended(RelFileNode rnode
*** 338,352 ****
  		/* we do this in recovery only - no rel-extension lock needed */
  		Assert(InRecovery);
  		buffer = InvalidBuffer;
! 		while (blkno >= lastblock)
  		{
  			if (buffer != InvalidBuffer)
  				ReleaseBuffer(buffer);
  			buffer = ReadBufferWithoutRelcache(rnode, forknum,
  											   P_NEW, mode, NULL);
- 			lastblock++;
  		}
! 		Assert(BufferGetBlockNumber(buffer) == blkno);
  	}
  
  	if (mode == RBM_NORMAL)
--- 338,358 ----
  		/* we do this in recovery only - no rel-extension lock needed */
  		Assert(InRecovery);
  		buffer = InvalidBuffer;
! 		do
  		{
  			if (buffer != InvalidBuffer)
  				ReleaseBuffer(buffer);
  			buffer = ReadBufferWithoutRelcache(rnode, forknum,
  											   P_NEW, mode, NULL);
  		}
! 		while (BufferGetBlockNumber(buffer) < blkno);
! 		/* Handle the corner case that P_NEW returns non-consecutive pages */
! 		if (BufferGetBlockNumber(buffer) != blkno)
! 		{
! 			ReleaseBuffer(buffer);
! 			buffer = ReadBufferWithoutRelcache(rnode, forknum, blkno,
! 											   mode, NULL);
! 		}
  	}
  
  	if (mode == RBM_NORMAL)
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to