Skip site navigation (1) Skip section navigation (2)

Re: BufMgr cleanup

From: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
To: Manfred Koizar <mkoi-pg(at)aon(dot)at>
Cc: pgsql-patches(at)postgresql(dot)org
Subject: Re: BufMgr cleanup
Date: 2002-06-25 17:40:34
Message-ID: 200206251740.g5PHeYS00143@candle.pha.pa.us (view raw or flat)
Thread:
Lists: pgsql-patches
Your patch has been added to the PostgreSQL unapplied patches list at:

	http://candle.pha.pa.us/cgi-bin/pgpatches

I will try to apply it within the next 48 hours.

---------------------------------------------------------------------------


Manfred Koizar wrote:
> Minor code cleanup in bufmgr.c and bufmgr.h, mainly by moving repeated
> lines of code into internal routines (drop_relfilenode_buffers,
> release_buffer) and by hiding unused routines (PrintBufferDescs,
> PrintPinnedBufs) behind #ifdef NOT_USED.
> Remove AbortBufferIO() declaration from bufmgr.c (already declared
> in bufmgr.h)
> 
> diff -ru ../orig/src/backend/storage/buffer/bufmgr.c src/backend/storage/buffer/bufmgr.c
> --- ../orig/src/backend/storage/buffer/bufmgr.c	2002-06-13 09:32:40.000000000 +0200
> +++ src/backend/storage/buffer/bufmgr.c	2002-06-13 11:02:47.000000000 +0200
> @@ -71,7 +71,6 @@
>  static void StartBufferIO(BufferDesc *buf, bool forInput);
>  static void TerminateBufferIO(BufferDesc *buf);
>  static void ContinueBufferIO(BufferDesc *buf, bool forInput);
> -extern void AbortBufferIO(void);
>  
>  /*
>   * Macro : BUFFER_IS_BROKEN
> @@ -85,9 +84,14 @@
>  			bool *foundPtr);
>  static int	ReleaseBufferWithBufferLock(Buffer buffer);
>  static int	BufferReplace(BufferDesc *bufHdr);
> +#ifdef NOT_USED
>  void		PrintBufferDescs(void);
> +#endif
>  
>  static void write_buffer(Buffer buffer, bool unpin);
> +static void drop_relfilenode_buffers(RelFileNode rnode,
> +                                     bool do_local, bool do_both);
> +static int release_buffer(Buffer buffer, bool havelock);
>  
>  /*
>   * ReadBuffer -- returns a buffer containing the requested
> @@ -1097,34 +1101,26 @@
>  	return relation->rd_nblocks;
>  }
>  
> -/* ---------------------------------------------------------------------
> - *		DropRelationBuffers
> - *
> - *		This function removes all the buffered pages for a relation
> - *		from the buffer pool.  Dirty pages are simply dropped, without
> - *		bothering to write them out first.	This is NOT rollback-able,
> - *		and so should be used only with extreme caution!
> - *
> - *		We assume that the caller holds an exclusive lock on the relation,
> - *		which should assure that no new buffers will be acquired for the rel
> - *		meanwhile.
> +/*
> + * drop_relfilenode_buffers -- common functionality for
> + *                             DropRelationBuffers and
> + *                             DropRelFileNodeBuffers
>   *
>   *		XXX currently it sequentially searches the buffer pool, should be
>   *		changed to more clever ways of searching.
> - * --------------------------------------------------------------------
>   */
> -void
> -DropRelationBuffers(Relation rel)
> +static void
> +drop_relfilenode_buffers(RelFileNode rnode, bool do_local, bool do_both)
>  {
>  	int			i;
>  	BufferDesc *bufHdr;
>  
> -	if (rel->rd_myxactonly)
> +	if (do_local)
>  	{
>  		for (i = 0; i < NLocBuffer; i++)
>  		{
>  			bufHdr = &LocalBufferDescriptors[i];
> -			if (RelFileNodeEquals(bufHdr->tag.rnode, rel->rd_node))
> +			if (RelFileNodeEquals(bufHdr->tag.rnode, rnode))
>  			{
>  				bufHdr->flags &= ~(BM_DIRTY | BM_JUST_DIRTIED);
>  				bufHdr->cntxDirty = false;
> @@ -1132,7 +1128,8 @@
>  				bufHdr->tag.rnode.relNode = InvalidOid;
>  			}
>  		}
> -		return;
> +		if (!do_both)
> +			return;
>  	}
>  
>  	LWLockAcquire(BufMgrLock, LW_EXCLUSIVE);
> @@ -1141,7 +1138,7 @@
>  	{
>  		bufHdr = &BufferDescriptors[i - 1];
>  recheck:
> -		if (RelFileNodeEquals(bufHdr->tag.rnode, rel->rd_node))
> +		if (RelFileNodeEquals(bufHdr->tag.rnode, rnode))
>  		{
>  			/*
>  			 * If there is I/O in progress, better wait till it's done;
> @@ -1188,6 +1185,25 @@
>  }
>  
>  /* ---------------------------------------------------------------------
> + *		DropRelationBuffers
> + *
> + *		This function removes all the buffered pages for a relation
> + *		from the buffer pool.  Dirty pages are simply dropped, without
> + *		bothering to write them out first.	This is NOT rollback-able,
> + *		and so should be used only with extreme caution!
> + *
> + *		We assume that the caller holds an exclusive lock on the relation,
> + *		which should assure that no new buffers will be acquired for the rel
> + *		meanwhile.
> + * --------------------------------------------------------------------
> + */
> +void
> +DropRelationBuffers(Relation rel)
> +{
> +	drop_relfilenode_buffers(rel->rd_node, rel->rd_myxactonly, false);
> +}
> +
> +/* ---------------------------------------------------------------------
>   *		DropRelFileNodeBuffers
>   *
>   *		This is the same as DropRelationBuffers, except that the target
> @@ -1201,73 +1217,8 @@
>  void
>  DropRelFileNodeBuffers(RelFileNode rnode)
>  {
> -	int			i;
> -	BufferDesc *bufHdr;
> -
>  	/* We have to search both local and shared buffers... */
> -
> -	for (i = 0; i < NLocBuffer; i++)
> -	{
> -		bufHdr = &LocalBufferDescriptors[i];
> -		if (RelFileNodeEquals(bufHdr->tag.rnode, rnode))
> -		{
> -			bufHdr->flags &= ~(BM_DIRTY | BM_JUST_DIRTIED);
> -			bufHdr->cntxDirty = false;
> -			LocalRefCount[i] = 0;
> -			bufHdr->tag.rnode.relNode = InvalidOid;
> -		}
> -	}
> -
> -	LWLockAcquire(BufMgrLock, LW_EXCLUSIVE);
> -
> -	for (i = 1; i <= NBuffers; i++)
> -	{
> -		bufHdr = &BufferDescriptors[i - 1];
> -recheck:
> -		if (RelFileNodeEquals(bufHdr->tag.rnode, rnode))
> -		{
> -			/*
> -			 * If there is I/O in progress, better wait till it's done;
> -			 * don't want to delete the relation out from under someone
> -			 * who's just trying to flush the buffer!
> -			 */
> -			if (bufHdr->flags & BM_IO_IN_PROGRESS)
> -			{
> -				WaitIO(bufHdr);
> -
> -				/*
> -				 * By now, the buffer very possibly belongs to some other
> -				 * rel, so check again before proceeding.
> -				 */
> -				goto recheck;
> -			}
> -			/* Now we can do what we came for */
> -			bufHdr->flags &= ~(BM_DIRTY | BM_JUST_DIRTIED);
> -			bufHdr->cntxDirty = false;
> -
> -			/*
> -			 * Release any refcount we may have.
> -			 *
> -			 * This is very probably dead code, and if it isn't then it's
> -			 * probably wrong.	I added the Assert to find out --- tgl
> -			 * 11/99.
> -			 */
> -			if (!(bufHdr->flags & BM_FREE))
> -			{
> -				/* Assert checks that buffer will actually get freed! */
> -				Assert(PrivateRefCount[i - 1] == 1 &&
> -					   bufHdr->refcount == 1);
> -				ReleaseBufferWithBufferLock(i);
> -			}
> -
> -			/*
> -			 * And mark the buffer as no longer occupied by this rel.
> -			 */
> -			BufTableDelete(bufHdr);
> -		}
> -	}
> -
> -	LWLockRelease(BufMgrLock);
> +	drop_relfilenode_buffers(rnode, true, true);
>  }
>  
>  /* ---------------------------------------------------------------------
> @@ -1343,6 +1294,7 @@
>   *		use only.
>   * -----------------------------------------------------------------
>   */
> +#ifdef NOT_USED
>  void
>  PrintBufferDescs()
>  {
> @@ -1375,7 +1327,9 @@
>  		}
>  	}
>  }
> +#endif
>  
> +#ifdef NOT_USED
>  void
>  PrintPinnedBufs()
>  {
> @@ -1395,6 +1349,7 @@
>  	}
>  	LWLockRelease(BufMgrLock);
>  }
> +#endif
>  
>  /*
>   * BufferPoolBlowaway
> @@ -1589,14 +1544,12 @@
>  	return 0;
>  }
>  
> -#undef ReleaseBuffer
> -
>  /*
> - * ReleaseBuffer -- remove the pin on a buffer without
> - *		marking it dirty.
> + * release_buffer -- common functionality for
> + *                   ReleaseBuffer and ReleaseBufferWithBufferLock
>   */
> -int
> -ReleaseBuffer(Buffer buffer)
> +static int
> +release_buffer(Buffer buffer, bool havelock)
>  {
>  	BufferDesc *bufHdr;
>  
> @@ -1617,14 +1570,30 @@
>  		PrivateRefCount[buffer - 1]--;
>  	else
>  	{
> -		LWLockAcquire(BufMgrLock, LW_EXCLUSIVE);
> +		if (!havelock)
> +			LWLockAcquire(BufMgrLock, LW_EXCLUSIVE);
> +
>  		UnpinBuffer(bufHdr);
> -		LWLockRelease(BufMgrLock);
> +
> +		if (!havelock)
> +			LWLockRelease(BufMgrLock);
>  	}
>  
>  	return STATUS_OK;
>  }
>  
> +#undef ReleaseBuffer
> +
> +/*
> + * ReleaseBuffer -- remove the pin on a buffer without
> + *		marking it dirty.
> + */
> +int
> +ReleaseBuffer(Buffer buffer)
> +{
> +	return release_buffer(buffer, false);
> +}
> +
>  /*
>   * ReleaseBufferWithBufferLock
>   *		Same as ReleaseBuffer except we hold the bufmgr lock
> @@ -1632,27 +1601,7 @@
>  static int
>  ReleaseBufferWithBufferLock(Buffer buffer)
>  {
> -	BufferDesc *bufHdr;
> -
> -	if (BufferIsLocal(buffer))
> -	{
> -		Assert(LocalRefCount[-buffer - 1] > 0);
> -		LocalRefCount[-buffer - 1]--;
> -		return STATUS_OK;
> -	}
> -
> -	if (BAD_BUFFER_ID(buffer))
> -		return STATUS_ERROR;
> -
> -	bufHdr = &BufferDescriptors[buffer - 1];
> -
> -	Assert(PrivateRefCount[buffer - 1] > 0);
> -	if (PrivateRefCount[buffer - 1] > 1)
> -		PrivateRefCount[buffer - 1]--;
> -	else
> -		UnpinBuffer(bufHdr);
> -
> -	return STATUS_OK;
> +	return release_buffer(buffer, true);
>  }
>  
>  
> @@ -1695,7 +1644,7 @@
>  #endif
>  
>  #ifdef NOT_USED
> -int
> +Buffer
>  ReleaseAndReadBuffer_Debug(char *file,
>  						   int line,
>  						   Buffer buffer,
> @@ -2117,7 +2066,7 @@
>  {
>  	Assert(buf == InProgressBuf);
>  	LWLockRelease(buf->io_in_progress_lock);
> -	InProgressBuf = (BufferDesc *) 0;
> +	InProgressBuf = (BufferDesc *) NULL;
>  }
>  
>  /*
> @@ -2142,7 +2091,7 @@
>  void
>  InitBufferIO(void)
>  {
> -	InProgressBuf = (BufferDesc *) 0;
> +	InProgressBuf = (BufferDesc *) NULL;
>  }
>  #endif
>  
> diff -ru ../orig/src/include/storage/bufmgr.h src/include/storage/bufmgr.h
> --- ../orig/src/include/storage/bufmgr.h	2002-06-13 09:22:59.000000000 +0200
> +++ src/include/storage/bufmgr.h	2002-06-13 10:57:22.000000000 +0200
> @@ -167,7 +167,9 @@
>  extern void DropRelationBuffers(Relation rel);
>  extern void DropRelFileNodeBuffers(RelFileNode rnode);
>  extern void DropBuffers(Oid dbid);
> +#ifdef NOT_USED
>  extern void PrintPinnedBufs(void);
> +#endif
>  extern int	BufferShmemSize(void);
>  extern RelFileNode BufferGetFileNode(Buffer buffer);
>  
> 
> 
> ---------------------------(end of broadcast)---------------------------
> TIP 5: Have you checked our extensive FAQ?
> 
> http://www.postgresql.org/users-lounge/docs/faq.html
> 

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman(at)candle(dot)pha(dot)pa(dot)us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026



In response to

pgsql-patches by date

Next:From: Tom LaneDate: 2002-06-25 17:41:31
Subject: Re: several minor cleanups
Previous:From: Bruce MomjianDate: 2002-06-25 17:27:17
Subject: Re: several minor cleanups

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group