On Thu, Jan 7, 2016 at 4:53 PM, Andres Freund <and...@anarazel.de> wrote:

> On 2016-01-07 16:48:53 +0530, Amit Kapila wrote:
>
> I think it's a worthwhile approach to pursue. But until it actually
> fixes the problem of leaving around uninitialized pages I don't think
> it's very meaningful to do performance comparisons.
>

Attached patch solves this issue, I am allocating the buffer for each page
and initializing the page, only after that adding to FSM.


> > a. Extend the relation page by page and add it to FSM without
> initializing
> > it.  I think this is what the current patch of Dilip seems to be doing.
> If
> > we
> I think that's pretty much unacceptable, for the non-error path at
> least.
>

Performance results:
----------------------------
Test Case:
------------
./psql -d postgres -c "COPY (select g.i::text FROM generate_series(1,
10000) g(i)) TO '/tmp/copybinary' WITH BINARY";

echo COPY data from '/tmp/copybinary' WITH BINARY; > copy_script

./psql -d postgres -c "truncate table data"
./psql -d postgres -c "checkpoint"
./pgbench -f copy_script -T 120 -c$ -j$ postgres

Test Summary:
--------------------
1. I have measured the performance of base and patch.
2. With patch there are multiple results, that are with different values of
"extend_num_pages" (parameter which says how many extra block to extend)

Test with Data on magnetic Disk and WAL on SSD
--------------------------------------------------------------------
Shared Buffer :    48GB
max_wal_size :    10GB
Storage          :  Magnetic Disk
WAL               :  SSD

                         tps with different value of extend_num_page

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

Client   Base     10-Page   20-Page   50-Page

1        105          103         157         129
2        217          219         255         288
4        210          421         494         486
8        166          605         702         701
16       145          484         563         686
32       124          477         480         745

Test with Data and WAL on SSD
-----------------------------------------------

Shared Buffer :   48GB
Max Wal Size :   10GB
Storage          :   SSD

                         tps with different value of extend_num_page

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

Client   Base     10-Page   20-Page   50-Page   100-Page

1          152          153          155          147          157
2          281          281          292          275          287
4          236          505          502          508          514
8          171          662          687          767          764
16         145          527          639          826          907

Note: Test with both data and WAL on Magnetic Disk : No significant
improvement visible
-- I think wall write is becoming bottleneck in this case.

Currently i have kept extend_num_page as session level parameter but i
think later we can make this as table property.
Any suggestion on this ?

Apart from this approach, I also tried extending the file in multiple block
in one extend call, but this approach (extending one by one) is performing
better.

-- 
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
*** a/src/backend/access/heap/hio.c
--- b/src/backend/access/heap/hio.c
***************
*** 24,29 ****
--- 24,30 ----
  #include "storage/lmgr.h"
  #include "storage/smgr.h"
  
+ int extend_num_pages = 0;
  
  /*
   * RelationPutHeapTuple - place tuple at specified page
***************
*** 238,243 **** RelationGetBufferForTuple(Relation relation, Size len,
--- 239,245 ----
  	BlockNumber targetBlock,
  				otherBlock;
  	bool		needLock;
+ 	int			totalBlocks;
  
  	len = MAXALIGN(len);		/* be conservative */
  
***************
*** 449,467 **** RelationGetBufferForTuple(Relation relation, Size len,
  	 * it worth keeping an accurate file length in shared memory someplace,
  	 * rather than relying on the kernel to do it for us?
  	 */
! 	buffer = ReadBufferBI(relation, P_NEW, bistate);
! 
! 	/*
! 	 * We can be certain that locking the otherBuffer first is OK, since it
! 	 * must have a lower page number.
! 	 */
! 	if (otherBuffer != InvalidBuffer)
! 		LockBuffer(otherBuffer, BUFFER_LOCK_EXCLUSIVE);
! 
! 	/*
! 	 * Now acquire lock on the new page.
! 	 */
! 	LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE);
  
  	/*
  	 * Release the file-extension lock; it's now OK for someone else to extend
--- 451,491 ----
  	 * it worth keeping an accurate file length in shared memory someplace,
  	 * rather than relying on the kernel to do it for us?
  	 */
! 
! 	totalBlocks = extend_num_pages;
! 
! 	do {
! 
! 
! 		buffer = ReadBufferBI(relation, P_NEW, bistate);
! 
! 		/*
! 		 * We can be certain that locking the otherBuffer first is OK, since it
! 		 * must have a lower page number.
! 		 */
! 		if ((otherBuffer != InvalidBuffer) && !totalBlocks)
! 			LockBuffer(otherBuffer, BUFFER_LOCK_EXCLUSIVE);
! 
! 		/*
! 		 * Now acquire lock on the new page.
! 		 */
! 		LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE);
! 
! 		if (totalBlocks)
! 		{
! 			Page page;
! 			Size freespace;
! 
! 			page = BufferGetPage(buffer);
! 			PageInit(page, BufferGetPageSize(buf), 0);
! 
! 			freespace = PageGetHeapFreeSpace(page);
! 			MarkBufferDirty(buffer);
! 			UnlockReleaseBuffer(buffer);
! 			RecordPageWithFreeSpace(relation, BufferGetBlockNumber(buffer), freespace);
! 		}
! 
! 	}while (totalBlocks--);
  
  	/*
  	 * Release the file-extension lock; it's now OK for someone else to extend
*** a/src/backend/utils/misc/guc.c
--- b/src/backend/utils/misc/guc.c
***************
*** 31,36 ****
--- 31,37 ----
  #include "access/transam.h"
  #include "access/twophase.h"
  #include "access/xact.h"
+ #include "access/hio.h"
  #include "catalog/namespace.h"
  #include "commands/async.h"
  #include "commands/prepare.h"
***************
*** 2683,2688 **** static struct config_int ConfigureNamesInt[] =
--- 2684,2699 ----
  		NULL, NULL, NULL
  	},
  
+ 	{
+ 		{"extend_num_pages", PGC_USERSET, UNGROUPED,
+ 			gettext_noop("Sets the Number of pages to extended at one time."),
+ 			NULL
+ 		},
+ 		&extend_num_pages,
+ 		0, 0, 100,
+ 		NULL, NULL, NULL
+ 	},
+ 
  	/* End-of-list marker */
  	{
  		{NULL, 0, 0, NULL, NULL}, NULL, 0, 0, 0, NULL, NULL, NULL
*** a/src/backend/utils/misc/postgresql.conf.sample
--- b/src/backend/utils/misc/postgresql.conf.sample
***************
*** 139,144 ****
--- 139,146 ----
  
  #temp_file_limit = -1			# limits per-session temp file space
  					# in kB, or -1 for no limit
+ #extend_num_pages = 0			# number of extra pages allocate during extend
+ 					# min 0 max 100 pages
  
  # - Kernel Resource Usage -
  
*** a/src/include/access/hio.h
--- b/src/include/access/hio.h
***************
*** 19,25 ****
  #include "utils/relcache.h"
  #include "storage/buf.h"
  
! 
  /*
   * state for bulk inserts --- private to heapam.c and hio.c
   *
--- 19,25 ----
  #include "utils/relcache.h"
  #include "storage/buf.h"
  
! extern int extend_num_pages;
  /*
   * state for bulk inserts --- private to heapam.c and hio.c
   *
-- 
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