On Mon, Jan 25, 2016 at 11:59 AM, Dilip Kumar <dilipbal...@gmail.com> wrote:
1.
>> Patch is not getting compiled.
>>
>> 1>src/backend/access/heap/hio.c(480): error C2065: 'buf' : undeclared
>> identifier
>>
> Oh, My mistake, my preprocessor is ignoring this error and relacing it
> with BLKSIZE
>
FIXED
> 3. I think you don't need to multi-extend a relation if
>> HEAP_INSERT_SKIP_FSM is used, as for that case it anyways try to
>> get a new page by extending a relation.
>>
>
> So i will change this..
>
FIXED
>
> 4. Again why do you need this multi-extend optimization for local
>> relations (those only accessible to current backend)?
>>
>
> I think we can change this while adding the table level
> "extend_by_blocks" for local table we will not allow this property, so no
> need to change at this place.
> What do you think ?
>
Now I have added table level parameter for specifying the number of blocks,
So do you think that we still need to block it, as user can control it,
Moreover i think if user want to use for local table then no harm in it at
least by extending in one shot he avoid multiple call of Extension lock,
though there will be no contention.
What is your opinion ?
5. Do we need this for nbtree as well? One way to check that
>> is by Copying large data in table having index.
>>
>> Ok, i will try this test and update.
>
I tried to load data to table with Index and tried to analyze bottleneck
using perf, and found btcompare was taking maximum time, still i don't deny
that it can not get benefited by multi extend.
So i tried quick POC for this, but i realize that even though we extend
multiple page for index and add to FSM, it will be updated only in current
page, Information about new free page will be propagated to root page only
during vacuum, And unlike heap Btree always search FSM from root and it
will not find the extra added pages.
So i think we can analyze this topic separately for index multi extend and
find is there are cases where index multi extend can give benefit.
Note: Test with both data and WAL on Magnetic Disk : No significant
>>> improvement visible
>>> -- I think wall write is becoming bottleneck in this case.
>>>
>>>
>> In that case, can you try the same test with un-logged tables?
>>
> Date with un-logged table
Test Init:
------------
./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 "create unlogged table data (data text)" --> base
./psql -d postgres -c "create unlogged table data (data text)
with(extend_by_blocks=50)" --patch
test_script:
------------
./psql -d postgres -c "truncate table data"
./psql -d postgres -c "checkpoint"
./pgbench -f copy_script -T 120 -c$ -j$ postgres
Shared Buffer 48GB
Table: Unlogged Table
ench -c$ -j$ -f -M Prepared postgres
Clients Base patch
1 178 180
2 337 338
4 265 601
8 167 805
Also, it is good to check the performance of patch with read-write work
>> load to ensure that extending relation in multiple-chunks should not
>> regress such cases.
>>
>
> Ok
>
I did not find in regression in normal case.
Note: I tested it with previous patch extend_num_pages=10 (guc parameter)
so that we can see any impact on overall system.
> 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 ?
>>>
>>>
>> I think we should have a new storage_parameter at table level
>> extend_by_blocks or something like that instead of GUC. The
>> default value of this parameter should be 1 which means retain
>> current behaviour by default.
>>
> +1
>
Changed it to table level storage parameter. I kept max_limit to 100 any
suggestion on this ? should we increase it ?
latest patch is attached..
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index 86b9ae1..76f9a21 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -268,6 +268,16 @@ static relopt_int intRelOpts[] =
#endif
},
+ {
+ {
+ "extend_by_blocks",
+ "Number of blocks to be added to relation in every extend call",
+ RELOPT_KIND_HEAP,
+ AccessExclusiveLock
+ },
+ 1, 1, 100
+ },
+
/* list terminator */
{{NULL}}
};
@@ -1291,7 +1301,9 @@ default_reloptions(Datum reloptions, bool validate, relopt_kind kind)
{"autovacuum_analyze_scale_factor", RELOPT_TYPE_REAL,
offsetof(StdRdOptions, autovacuum) +offsetof(AutoVacOpts, analyze_scale_factor)},
{"user_catalog_table", RELOPT_TYPE_BOOL,
- offsetof(StdRdOptions, user_catalog_table)}
+ offsetof(StdRdOptions, user_catalog_table)},
+ {"extend_by_blocks", RELOPT_TYPE_INT,
+ offsetof(StdRdOptions, extend_by_blocks)}
};
options = parseRelOptions(reloptions, validate, kind, &numoptions);
diff --git a/src/backend/access/heap/hio.c b/src/backend/access/heap/hio.c
index 8140418..ec430fc 100644
--- a/src/backend/access/heap/hio.c
+++ b/src/backend/access/heap/hio.c
@@ -238,6 +238,7 @@ RelationGetBufferForTuple(Relation relation, Size len,
BlockNumber targetBlock,
otherBlock;
bool needLock;
+ int extraBlocks;
len = MAXALIGN(len); /* be conservative */
@@ -443,25 +444,49 @@ RelationGetBufferForTuple(Relation relation, Size len,
if (needLock)
LockRelationForExtension(relation, ExclusiveLock);
+ if (use_fsm)
+ extraBlocks = RelationGetExtendBlocks(relation) -1;
+ else
+ extraBlocks = 0;
/*
* XXX This does an lseek - rather expensive - but at the moment it is the
* only way to accurately determine how many blocks are in a relation. Is
* 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);
+ do {
+ buffer = ReadBufferBI(relation, P_NEW, bistate);
- /*
- * Now acquire lock on the new page.
- */
- LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE);
+ /*
+ * We can be certain that locking the otherBuffer first is OK, since it
+ * must have a lower page number.
+ */
+ if ((otherBuffer != InvalidBuffer) && !extraBlocks)
+ LockBuffer(otherBuffer, BUFFER_LOCK_EXCLUSIVE);
+
+ /*
+ * Now acquire lock on the new page.
+ */
+ LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE);
+
+ if (extraBlocks)
+ {
+ Page page;
+ Size freespace;
+ BlockNumber blockNum;
+
+ page = BufferGetPage(buffer);
+ PageInit(page, BufferGetPageSize(buffer), 0);
+
+ freespace = PageGetHeapFreeSpace(page);
+ MarkBufferDirty(buffer);
+ blockNum = BufferGetBlockNumber(buffer);
+ UnlockReleaseBuffer(buffer);
+ RecordPageWithFreeSpace(relation, blockNum, freespace);
+ }
+
+ }while (extraBlocks--);
/*
* Release the file-extension lock; it's now OK for someone else to extend
diff --git a/src/include/access/hio.h b/src/include/access/hio.h
index a174b34..40c3941 100644
--- a/src/include/access/hio.h
+++ b/src/include/access/hio.h
@@ -19,7 +19,6 @@
#include "utils/relcache.h"
#include "storage/buf.h"
-
/*
* state for bulk inserts --- private to heapam.c and hio.c
*
diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h
index f2bebf2..26f6b8e 100644
--- a/src/include/utils/rel.h
+++ b/src/include/utils/rel.h
@@ -203,6 +203,7 @@ typedef struct StdRdOptions
AutoVacOpts autovacuum; /* autovacuum-related options */
bool user_catalog_table; /* use as an additional catalog
* relation */
+ int extend_by_blocks;
} StdRdOptions;
#define HEAP_MIN_FILLFACTOR 10
@@ -239,6 +240,13 @@ typedef struct StdRdOptions
((relation)->rd_options ? \
((StdRdOptions *) (relation)->rd_options)->user_catalog_table : false)
+/*
+ * RelationGetExtendBlocks
+ * Returns the relation's number of block to be extended one time.
+ */
+#define RelationGetExtendBlocks(relation) \
+ ((relation)->rd_options ? \
+ ((StdRdOptions *) (relation)->rd_options)->extend_by_blocks : 1)
/*
* ViewOptions
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
