Avoid unnecessary lseek() calls by cleanups in md.c. mdfd_lstbcnt was

not being consulted anywhere, so remove it and remove the _mdnblocks()
calls that were used to set it.  Change smgrextend interface to pass in
the target block number (ie, current file length) --- the caller always
knows this already, having already done smgrnblocks(), so it's silly to
do it over again inside mdextend.  Net result: extension of a file now
takes one lseek(SEEK_END) and a write(), not three lseeks and a write.

Author: tglsfdc

src/backend/commands/sequence.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/commands/sequence.c,v 1.54 2001/04/04 15:43:25 vadim Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/commands/sequence.c,v 1.55 2001/05/10 20:38:49 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -569,7 +569,7 @@ read_info(char *caller, SeqTable elm, Buffer *buf)
 	sequence_magic *sm;
 	Form_pg_sequence seq;
 
-	if (RelationGetNumberOfBlocks(elm->rel) != 1)
+	if (elm->rel->rd_nblocks > 1)
 		elog(ERROR, "%s.%s: invalid number of blocks in sequence",
 			 elm->name, caller);
 

src/backend/storage/buffer/bufmgr.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/storage/buffer/bufmgr.c,v 1.109 2001/03/22 03:59:44 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/storage/buffer/bufmgr.c,v 1.110 2001/05/10 20:38:49 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -173,9 +173,9 @@ ReadBufferWithBufferLock(Relation reln,
 						 bool bufferLockHeld)
 {
 	BufferDesc *bufHdr;
-	int			extend;			/* extending the file by one block */
 	int			status;
 	bool		found;
+	bool		extend;			/* extending the file by one block */
 	bool		isLocalBuf;
 
 	extend = (blockNum == P_NEW);
@@ -207,21 +207,13 @@ ReadBufferWithBufferLock(Relation reln,
 	/* if it's already in the buffer pool, we're done */
 	if (found)
 	{
-
 		/*
-		 * This happens when a bogus buffer was returned previously and is
-		 * floating around in the buffer pool.	A routine calling this
-		 * would want this extended.
+		 * Could see found && extend if a buffer was already created for
+		 * the next page position, but then smgrextend failed to write
+		 * the page.  Must fall through and try to extend file again.
 		 */
-		if (extend)
-		{
-			/* new buffers are zero-filled */
-			MemSet((char *) MAKE_PTR(bufHdr->data), 0, BLCKSZ);
-			smgrextend(DEFAULT_SMGR, reln,
-					   (char *) MAKE_PTR(bufHdr->data));
-		}
-		return BufferDescriptorGetBuffer(bufHdr);
-
+		if (!extend)
+			return BufferDescriptorGetBuffer(bufHdr);
 	}
 
 	/*
@@ -232,17 +224,25 @@ ReadBufferWithBufferLock(Relation reln,
 	{
 		/* new buffers are zero-filled */
 		MemSet((char *) MAKE_PTR(bufHdr->data), 0, BLCKSZ);
-		status = smgrextend(DEFAULT_SMGR, reln,
+		status = smgrextend(DEFAULT_SMGR, reln, bufHdr->tag.blockNum,
 							(char *) MAKE_PTR(bufHdr->data));
 	}
 	else
 	{
-		status = smgrread(DEFAULT_SMGR, reln, blockNum,
+		status = smgrread(DEFAULT_SMGR, reln, bufHdr->tag.blockNum,
 						  (char *) MAKE_PTR(bufHdr->data));
 	}
 
 	if (isLocalBuf)
+	{
+		/* No shared buffer state to update... */
+		if (status == SM_FAIL)
+		{
+			bufHdr->flags |= BM_IO_ERROR;
+			return InvalidBuffer;
+		}
 		return BufferDescriptorGetBuffer(bufHdr);
+	}
 
 	/* lock buffer manager again to update IO IN PROGRESS */
 	SpinAcquire(BufMgrLock);
@@ -302,13 +302,11 @@ BufferAlloc(Relation reln,
 			   *buf2;
 	BufferTag	newTag;			/* identity of requested block */
 	bool		inProgress;		/* buffer undergoing IO */
-	bool		newblock = FALSE;
 
 	/* create a new tag so we can lookup the buffer */
 	/* assume that the relation is already open */
 	if (blockNum == P_NEW)
 	{
-		newblock = TRUE;
 		blockNum = smgrnblocks(DEFAULT_SMGR, reln);
 	}
 
@@ -1102,7 +1100,8 @@ BufferReplace(BufferDesc *bufHdr)
 
 /*
  * RelationGetNumberOfBlocks
- *		Returns the buffer descriptor associated with a page in a relation.
+ *		Determines the current number of pages in the relation.
+ *		Side effect: relation->rd_nblocks is updated.
  *
  * Note:
  *		XXX may fail for huge relations.
@@ -1112,9 +1111,16 @@ BufferReplace(BufferDesc *bufHdr)
 BlockNumber
 RelationGetNumberOfBlocks(Relation relation)
 {
-	return ((relation->rd_myxactonly) ? relation->rd_nblocks :
-			((relation->rd_rel->relkind == RELKIND_VIEW) ? 0 :
-			 smgrnblocks(DEFAULT_SMGR, relation)));
+	/*
+	 * relation->rd_nblocks should be accurate already if the relation
+	 * is myxactonly.  (XXX how safe is that really?)  Don't call smgr
+	 * on a view, either.
+	 */
+	if (relation->rd_rel->relkind == RELKIND_VIEW)
+		relation->rd_nblocks = 0;
+	else if (!relation->rd_myxactonly)
+		relation->rd_nblocks = smgrnblocks(DEFAULT_SMGR, relation);
+	return relation->rd_nblocks;
 }
 
 /* ---------------------------------------------------------------------

src/backend/storage/smgr/md.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/storage/smgr/md.c,v 1.83 2001/04/02 23:20:24 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/storage/smgr/md.c,v 1.84 2001/05/10 20:38:49 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -54,10 +54,9 @@ typedef struct _MdfdVec
 	int			mdfd_flags;		/* fd status flags */
 
 /* these are the assigned bits in mdfd_flags: */
-#define MDFD_FREE		(1 << 0)/* unused entry */
+#define MDFD_FREE	(1 << 0)	/* unused entry */
 
-	int			mdfd_lstbcnt;	/* most recent block count */
-	int			mdfd_nextFree;	/* next free vector */
+	int			mdfd_nextFree;	/* link to next freelist member, if free */
 #ifndef LET_OS_MANAGE_FILESIZE
 	struct _MdfdVec *mdfd_chain;/* for large relations */
 #endif
@@ -164,7 +163,6 @@ mdcreate(Relation reln)
 
 	Md_fdvec[vfd].mdfd_vfd = fd;
 	Md_fdvec[vfd].mdfd_flags = (uint16) 0;
-	Md_fdvec[vfd].mdfd_lstbcnt = 0;
 #ifndef LET_OS_MANAGE_FILESIZE
 	Md_fdvec[vfd].mdfd_chain = (MdfdVec *) NULL;
 #endif
@@ -225,52 +223,69 @@ mdunlink(RelFileNode rnode)
 /*
  *	mdextend() -- Add a block to the specified relation.
  *
+ *		The semantics are basically the same as mdwrite(): write at the
+ *		specified position.  However, we are expecting to extend the
+ *		relation (ie, blocknum is the current EOF), and so in case of
+ *		failure we clean up by truncating.
+ *
  *		This routine returns SM_FAIL or SM_SUCCESS, with errno set as
  *		appropriate.
+ *
+ * Note: this routine used to call mdnblocks() to get the block position
+ * to write at, but that's pretty silly since the caller needs to know where
+ * the block will be written, and accordingly must have done mdnblocks()
+ * already.  Might as well pass in the position and save a seek.
  */
 int
-mdextend(Relation reln, char *buffer)
+mdextend(Relation reln, BlockNumber blocknum, char *buffer)
 {
-	long		pos,
-				nbytes;
-	int			nblocks;
+	long		seekpos;
+	int			nbytes;
 	MdfdVec    *v;
 
-	nblocks = mdnblocks(reln);
-	v = _mdfd_getseg(reln, nblocks);
+	v = _mdfd_getseg(reln, blocknum);
 
-	if ((pos = FileSeek(v->mdfd_vfd, 0L, SEEK_END)) < 0)
-		return SM_FAIL;
+#ifndef LET_OS_MANAGE_FILESIZE
+	seekpos = (long) (BLCKSZ * (blocknum % RELSEG_SIZE));
+#ifdef DIAGNOSTIC
+	if (seekpos >= BLCKSZ * RELSEG_SIZE)
+		elog(FATAL, "seekpos too big!");
+#endif
+#else
+	seekpos = (long) (BLCKSZ * (blocknum));
+#endif
 
-	if (pos % BLCKSZ != 0)		/* the last block is incomplete */
-	{
-		pos -= pos % BLCKSZ;
-		if (FileSeek(v->mdfd_vfd, pos, SEEK_SET) < 0)
-			return SM_FAIL;
-	}
+	/*
+	 * Note: because caller obtained blocknum by calling mdnblocks, which
+	 * did a seek(SEEK_END), this seek is often redundant and will be
+	 * optimized away by fd.c.  It's not redundant, however, if there is a
+	 * partial page at the end of the file.  In that case we want to try to
+	 * overwrite the partial page with a full page.  It's also not redundant
+	 * if bufmgr.c had to dump another buffer of the same file to make room
+	 * for the new page's buffer.
+	 */
+	if (FileSeek(v->mdfd_vfd, seekpos, SEEK_SET) != seekpos)
+		return SM_FAIL;
 
 	if ((nbytes = FileWrite(v->mdfd_vfd, buffer, BLCKSZ)) != BLCKSZ)
 	{
 		if (nbytes > 0)
 		{
-			FileTruncate(v->mdfd_vfd, pos);
-			FileSeek(v->mdfd_vfd, pos, SEEK_SET);
+			int		save_errno = errno;
+
+			/* Remove the partially-written page */
+			FileTruncate(v->mdfd_vfd, seekpos);
+			FileSeek(v->mdfd_vfd, seekpos, SEEK_SET);
+			errno = save_errno;
 		}
 		return SM_FAIL;
 	}
 
-	/* try to keep the last block count current, though it's just a hint */
 #ifndef LET_OS_MANAGE_FILESIZE
-	if ((v->mdfd_lstbcnt = (++nblocks % RELSEG_SIZE)) == 0)
-		v->mdfd_lstbcnt = RELSEG_SIZE;
-
 #ifdef DIAGNOSTIC
-	if (_mdnblocks(v->mdfd_vfd, BLCKSZ) > RELSEG_SIZE
-		|| v->mdfd_lstbcnt > RELSEG_SIZE)
+	if (_mdnblocks(v->mdfd_vfd, BLCKSZ) > RELSEG_SIZE)
 		elog(FATAL, "segment too big!");
 #endif
-#else
-	v->mdfd_lstbcnt = ++nblocks;
 #endif
 
 	return SM_SUCCESS;
@@ -319,12 +334,11 @@ mdopen(Relation reln)
 
 	Md_fdvec[vfd].mdfd_vfd = fd;
 	Md_fdvec[vfd].mdfd_flags = (uint16) 0;
-	Md_fdvec[vfd].mdfd_lstbcnt = _mdnblocks(fd, BLCKSZ);
 #ifndef LET_OS_MANAGE_FILESIZE
 	Md_fdvec[vfd].mdfd_chain = (MdfdVec *) NULL;
 
 #ifdef DIAGNOSTIC
-	if (Md_fdvec[vfd].mdfd_lstbcnt > RELSEG_SIZE)
+	if (_mdnblocks(fd, BLCKSZ) > RELSEG_SIZE)
 		elog(FATAL, "segment too big on relopen!");
 #endif
 #endif
@@ -440,9 +454,12 @@ mdread(Relation reln, BlockNumber blocknum, char *buffer)
 	status = SM_SUCCESS;
 	if ((nbytes = FileRead(v->mdfd_vfd, buffer, BLCKSZ)) != BLCKSZ)
 	{
-		if (nbytes == 0)
-			MemSet(buffer, 0, BLCKSZ);
-		else if (blocknum == 0 && nbytes > 0 && mdnblocks(reln) == 0)
+		/*
+		 * If we are at EOF, return zeroes without complaining.
+		 * (XXX Is this still necessary/a good idea??)
+		 */
+		if (nbytes == 0 ||
+			(nbytes > 0 && mdnblocks(reln) == blocknum))
 			MemSet(buffer, 0, BLCKSZ);
 		else
 			status = SM_FAIL;
@@ -459,7 +476,6 @@ mdread(Relation reln, BlockNumber blocknum, char *buffer)
 int
 mdwrite(Relation reln, BlockNumber blocknum, char *buffer)
 {
-	int			status;
 	long		seekpos;
 	MdfdVec    *v;
 
@@ -478,11 +494,10 @@ mdwrite(Relation reln, BlockNumber blocknum, char *buffer)
 	if (FileSeek(v->mdfd_vfd, seekpos, SEEK_SET) != seekpos)
 		return SM_FAIL;
 
-	status = SM_SUCCESS;
 	if (FileWrite(v->mdfd_vfd, buffer, BLCKSZ) != BLCKSZ)
-		status = SM_FAIL;
+		return SM_FAIL;
 
-	return status;
+	return SM_SUCCESS;
 }
 
 /*
@@ -662,31 +677,29 @@ mdnblocks(Relation reln)
 		nblocks = _mdnblocks(v->mdfd_vfd, BLCKSZ);
 		if (nblocks > RELSEG_SIZE)
 			elog(FATAL, "segment too big in mdnblocks!");
-		v->mdfd_lstbcnt = nblocks;
-		if (nblocks == RELSEG_SIZE)
-		{
-			segno++;
+		if (nblocks < RELSEG_SIZE)
+			return (segno * RELSEG_SIZE) + nblocks;
+		/*
+		 * If segment is exactly RELSEG_SIZE, advance to next one.
+		 */
+		segno++;
 
+		if (v->mdfd_chain == (MdfdVec *) NULL)
+		{
+			/*
+			 * Because we pass O_CREAT, we will create the next
+			 * segment (with zero length) immediately, if the last
+			 * segment is of length REL_SEGSIZE.  This is unnecessary
+			 * but harmless, and testing for the case would take more
+			 * cycles than it seems worth.
+			 */
+			v->mdfd_chain = _mdfd_openseg(reln, segno, O_CREAT);
 			if (v->mdfd_chain == (MdfdVec *) NULL)
-			{
-
-				/*
-				 * Because we pass O_CREAT, we will create the next
-				 * segment (with zero length) immediately, if the last
-				 * segment is of length REL_SEGSIZE.  This is unnecessary
-				 * but harmless, and testing for the case would take more
-				 * cycles than it seems worth.
-				 */
-				v->mdfd_chain = _mdfd_openseg(reln, segno, O_CREAT);
-				if (v->mdfd_chain == (MdfdVec *) NULL)
-					elog(ERROR, "cannot count blocks for %s -- open failed: %m",
-						 RelationGetRelationName(reln));
-			}
-
-			v = v->mdfd_chain;
+				elog(ERROR, "cannot count blocks for %s -- open failed: %m",
+					 RelationGetRelationName(reln));
 		}
-		else
-			return (segno * RELSEG_SIZE) + nblocks;
+
+		v = v->mdfd_chain;
 	}
 #else
 	return _mdnblocks(v->mdfd_vfd, BLCKSZ);
@@ -761,7 +774,6 @@ mdtruncate(Relation reln, int nblocks)
 
 			if (FileTruncate(v->mdfd_vfd, lastsegblocks * BLCKSZ) < 0)
 				return -1;
-			v->mdfd_lstbcnt = lastsegblocks;
 			v = v->mdfd_chain;
 			ov->mdfd_chain = (MdfdVec *) NULL;
 		}
@@ -779,7 +791,6 @@ mdtruncate(Relation reln, int nblocks)
 #else
 	if (FileTruncate(v->mdfd_vfd, nblocks * BLCKSZ) < 0)
 		return -1;
-	v->mdfd_lstbcnt = nblocks;
 #endif
 
 	return nblocks;
@@ -958,13 +969,12 @@ _mdfd_openseg(Relation reln, int segno, int oflags)
 	/* fill the entry */
 	v->mdfd_vfd = fd;
 	v->mdfd_flags = (uint16) 0;
-	v->mdfd_lstbcnt = _mdnblocks(fd, BLCKSZ);
 #ifndef LET_OS_MANAGE_FILESIZE
 	v->mdfd_chain = (MdfdVec *) NULL;
 
 #ifdef DIAGNOSTIC
-	if (v->mdfd_lstbcnt > RELSEG_SIZE)
-		elog(FATAL, "segment too big on open!");
+	if (_mdnblocks(fd, BLCKSZ) > RELSEG_SIZE)
+		elog(FATAL, "segment too big on openseg!");
 #endif
 #endif