PENDING_LIST_CLEANUP_SIZE - maximum size of GIN pending list Re: HEAD seems to generate larger WAL regarding GIN index
On Mon, Apr 14, 2014 at 11:40 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Fujii Masao <masao.fujii@gmail.com> writes:
On Tue, Apr 1, 2014 at 1:41 AM, Robert Haas <robertmhaas@gmail.com> wrote:
Should we try to install some hack around fastupdate for 9.4? I fear
the divergence between reasonable values of work_mem and reasonable
sizes for that list is only going to continue to get bigger. I'm sure
there's somebody out there who has work_mem = 16GB, and stuff like
263865a48973767ce8ed7b7788059a38a24a9f37 is only going to increase the
appeal of large values.Controlling the threshold of the size of pending list only by GUC doesn't
seem reasonable. Users may want to increase the threshold only for the
GIN index which can be updated heavily, and decrease it otherwise. So
I think that it's better to add new storage parameter for GIN index to control
the threshold, or both storage parameter and GUC.Yeah, -1 for a GUC. A GIN-index-specific storage parameter seems more
appropriate.
The attached patch introduces the GIN index storage parameter
"PENDING_LIST_CLEANUP_SIZE" which specifies the maximum size of
GIN pending list. If it's not set, work_mem is used as that maximum size,
instead. So this patch doesn't break the existing application which
currently uses work_mem as the threshold of cleanup operation of
the pending list. It has only not to set PENDING_LIST_CLEANUP_SIZE.
This is an index storage parameter, and allows us to specify each
threshold per GIN index. So, for example, we can increase the threshold
only for the GIN index which can be updated heavily, and decrease it otherwise.
This patch uses another patch that I proposed (*1) as an infrastructure.
Please apply that infrastructure patch first if you apply this patch.
(*1)
/messages/by-id/CAHGQGwEanQ_e8WLHL25=bm_8Z5zkyZw0K0yiR+kdMV2HgnE9FQ@mail.gmail.com
Regards,
--
Fujii Masao
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Aug 8, 2014 at 11:43 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
On Mon, Apr 14, 2014 at 11:40 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Fujii Masao <masao.fujii@gmail.com> writes:
On Tue, Apr 1, 2014 at 1:41 AM, Robert Haas <robertmhaas@gmail.com> wrote:
Should we try to install some hack around fastupdate for 9.4? I fear
the divergence between reasonable values of work_mem and reasonable
sizes for that list is only going to continue to get bigger. I'm sure
there's somebody out there who has work_mem = 16GB, and stuff like
263865a48973767ce8ed7b7788059a38a24a9f37 is only going to increase the
appeal of large values.Controlling the threshold of the size of pending list only by GUC doesn't
seem reasonable. Users may want to increase the threshold only for the
GIN index which can be updated heavily, and decrease it otherwise. So
I think that it's better to add new storage parameter for GIN index to control
the threshold, or both storage parameter and GUC.Yeah, -1 for a GUC. A GIN-index-specific storage parameter seems more
appropriate.The attached patch introduces the GIN index storage parameter
"PENDING_LIST_CLEANUP_SIZE" which specifies the maximum size of
GIN pending list. If it's not set, work_mem is used as that maximum size,
instead. So this patch doesn't break the existing application which
currently uses work_mem as the threshold of cleanup operation of
the pending list. It has only not to set PENDING_LIST_CLEANUP_SIZE.This is an index storage parameter, and allows us to specify each
threshold per GIN index. So, for example, we can increase the threshold
only for the GIN index which can be updated heavily, and decrease it otherwise.This patch uses another patch that I proposed (*1) as an infrastructure.
Please apply that infrastructure patch first if you apply this patch.(*1)
/messages/by-id/CAHGQGwEanQ_e8WLHL25=bm_8Z5zkyZw0K0yiR+kdMV2HgnE9FQ@mail.gmail.comRegards,
--
Fujii Masao
Sorry, I forgot to attached the patch.... This time, attached.
Regards,
--
Fujii Masao
Attachments:
pending_list_cleanup_size_v1.patchtext/x-patch; charset=US-ASCII; name=pending_list_cleanup_size_v1.patchDownload
*** a/doc/src/sgml/gin.sgml
--- b/doc/src/sgml/gin.sgml
***************
*** 728,735 ****
from the indexed item). As of <productname>PostgreSQL</productname> 8.4,
<acronym>GIN</> is capable of postponing much of this work by inserting
new tuples into a temporary, unsorted list of pending entries.
! When the table is vacuumed, or if the pending list becomes too large
! (larger than <xref linkend="guc-work-mem">), the entries are moved to the
main <acronym>GIN</acronym> data structure using the same bulk insert
techniques used during initial index creation. This greatly improves
<acronym>GIN</acronym> index update speed, even counting the additional
--- 728,736 ----
from the indexed item). As of <productname>PostgreSQL</productname> 8.4,
<acronym>GIN</> is capable of postponing much of this work by inserting
new tuples into a temporary, unsorted list of pending entries.
! When the table is vacuumed, or if the pending list becomes larger than
! <literal>PENDING_LIST_CLEANUP_SIZE</literal> (or
! <xref linkend="guc-work-mem"> if not set), the entries are moved to the
main <acronym>GIN</acronym> data structure using the same bulk insert
techniques used during initial index creation. This greatly improves
<acronym>GIN</acronym> index update speed, even counting the additional
***************
*** 812,829 ****
</varlistentry>
<varlistentry>
! <term><xref linkend="guc-work-mem"></term>
<listitem>
<para>
During a series of insertions into an existing <acronym>GIN</acronym>
index that has <literal>FASTUPDATE</> enabled, the system will clean up
the pending-entry list whenever the list grows larger than
! <varname>work_mem</>. To avoid fluctuations in observed response time,
! it's desirable to have pending-list cleanup occur in the background
! (i.e., via autovacuum). Foreground cleanup operations can be avoided by
! increasing <varname>work_mem</> or making autovacuum more aggressive.
! However, enlarging <varname>work_mem</> means that if a foreground
! cleanup does occur, it will take even longer.
</para>
</listitem>
</varlistentry>
--- 813,839 ----
</varlistentry>
<varlistentry>
! <term><literal>PENDING_LIST_CLEANUP_SIZE</> and
! <xref linkend="guc-work-mem"></term>
<listitem>
<para>
During a series of insertions into an existing <acronym>GIN</acronym>
index that has <literal>FASTUPDATE</> enabled, the system will clean up
the pending-entry list whenever the list grows larger than
! <literal>PENDING_LIST_CLEANUP_SIZE</> (if not set, <varname>work_mem</>
! is used as that threshold, instead). To avoid fluctuations in observed
! response time, it's desirable to have pending-list cleanup occur in the
! background (i.e., via autovacuum). Foreground cleanup operations
! can be avoided by increasing <literal>PENDING_LIST_CLEANUP_SIZE</>
! (or <varname>work_mem</>) or making autovacuum more aggressive.
! However, enlarging the threshold of the cleanup operation means that
! if a foreground cleanup does occur, it will take even longer.
! </para>
! <para>
! <literal>PENDING_LIST_CLEANUP_SIZE</> is an index storage parameter,
! and allows each GIN index to have its own cleanup threshold.
! For example, it's possible to increase the threshold only for the GIN
! index which can be updated heavily, and decrease it otherwise.
</para>
</listitem>
</varlistentry>
*** a/doc/src/sgml/ref/create_index.sgml
--- b/doc/src/sgml/ref/create_index.sgml
***************
*** 356,361 **** CREATE [ UNIQUE ] INDEX [ CONCURRENTLY ] [ <replaceable class="parameter">name</
--- 356,377 ----
</listitem>
</varlistentry>
</variablelist>
+ <variablelist>
+ <varlistentry>
+ <term><literal>PENDING_LIST_CLEANUP_SIZE</></term>
+ <listitem>
+ <para>
+ This setting specifies the maximum size of the GIN pending list which is
+ used when <literal>FASTUPDATE</> is enabled. If the list grows larger than
+ this maximum size, it is cleaned up by moving the entries in it to the
+ main GIN data structure in bulk. The value is specified in kilobytes.
+ If this is not set, <literal>work_mem</> is used as the maximum size
+ of the pending list, instead. See <xref linkend="gin-fast-update"> and
+ <xref linkend="gin-tips"> for more information.
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
</refsect2>
<refsect2 id="SQL-CREATEINDEX-CONCURRENTLY">
*** a/doc/src/sgml/ref/pg_receivexlog.sgml
--- b/doc/src/sgml/ref/pg_receivexlog.sgml
***************
*** 106,126 **** PostgreSQL documentation
</varlistentry>
<varlistentry>
- <term><option>-F <replaceable class="parameter">interval</replaceable></option></term>
- <term><option>--fsync-interval=<replaceable class="parameter">interval</replaceable></option></term>
- <listitem>
- <para>
- Specifies the maximum time to issue sync commands to ensure the
- received WAL file is safely flushed to disk, in seconds. The default
- value is zero, which disables issuing fsyncs except when WAL file is
- closed. If <literal>-1</literal> is specified, WAL file is flushed as
- soon as possible, that is, as soon as there are WAL data which has
- not been flushed yet.
- </para>
- </listitem>
- </varlistentry>
-
- <varlistentry>
<term><option>-v</option></term>
<term><option>--verbose</option></term>
<listitem>
--- 106,111 ----
*** a/src/backend/access/common/reloptions.c
--- b/src/backend/access/common/reloptions.c
***************
*** 105,111 **** static relopt_int intRelOpts[] =
"Packs btree index pages only to this percentage",
RELOPT_KIND_BTREE
},
! BTREE_DEFAULT_FILLFACTOR, BTREE_MIN_FILLFACTOR, 100
},
{
{
--- 105,111 ----
"Packs btree index pages only to this percentage",
RELOPT_KIND_BTREE
},
! BTREE_DEFAULT_FILLFACTOR, BTREE_MIN_FILLFACTOR, 100, 0
},
{
{
***************
*** 113,119 **** static relopt_int intRelOpts[] =
"Packs hash index pages only to this percentage",
RELOPT_KIND_HASH
},
! HASH_DEFAULT_FILLFACTOR, HASH_MIN_FILLFACTOR, 100
},
{
{
--- 113,119 ----
"Packs hash index pages only to this percentage",
RELOPT_KIND_HASH
},
! HASH_DEFAULT_FILLFACTOR, HASH_MIN_FILLFACTOR, 100, 0
},
{
{
***************
*** 121,127 **** static relopt_int intRelOpts[] =
"Packs gist index pages only to this percentage",
RELOPT_KIND_GIST
},
! GIST_DEFAULT_FILLFACTOR, GIST_MIN_FILLFACTOR, 100
},
{
{
--- 121,127 ----
"Packs gist index pages only to this percentage",
RELOPT_KIND_GIST
},
! GIST_DEFAULT_FILLFACTOR, GIST_MIN_FILLFACTOR, 100, 0
},
{
{
***************
*** 129,135 **** static relopt_int intRelOpts[] =
"Packs spgist index pages only to this percentage",
RELOPT_KIND_SPGIST
},
! SPGIST_DEFAULT_FILLFACTOR, SPGIST_MIN_FILLFACTOR, 100
},
{
{
--- 129,135 ----
"Packs spgist index pages only to this percentage",
RELOPT_KIND_SPGIST
},
! SPGIST_DEFAULT_FILLFACTOR, SPGIST_MIN_FILLFACTOR, 100, 0
},
{
{
***************
*** 137,143 **** static relopt_int intRelOpts[] =
"Minimum number of tuple updates or deletes prior to vacuum",
RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
},
! -1, 0, INT_MAX
},
{
{
--- 137,143 ----
"Minimum number of tuple updates or deletes prior to vacuum",
RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
},
! -1, 0, INT_MAX, 0
},
{
{
***************
*** 145,151 **** static relopt_int intRelOpts[] =
"Minimum number of tuple inserts, updates or deletes prior to analyze",
RELOPT_KIND_HEAP
},
! -1, 0, INT_MAX
},
{
{
--- 145,151 ----
"Minimum number of tuple inserts, updates or deletes prior to analyze",
RELOPT_KIND_HEAP
},
! -1, 0, INT_MAX, 0
},
{
{
***************
*** 153,159 **** static relopt_int intRelOpts[] =
"Vacuum cost delay in milliseconds, for autovacuum",
RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
},
! -1, 0, 100
},
{
{
--- 153,159 ----
"Vacuum cost delay in milliseconds, for autovacuum",
RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
},
! -1, 0, 100, GUC_UNIT_MS
},
{
{
***************
*** 161,167 **** static relopt_int intRelOpts[] =
"Vacuum cost amount available before napping, for autovacuum",
RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
},
! -1, 1, 10000
},
{
{
--- 161,167 ----
"Vacuum cost amount available before napping, for autovacuum",
RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
},
! -1, 1, 10000, 0
},
{
{
***************
*** 169,175 **** static relopt_int intRelOpts[] =
"Minimum age at which VACUUM should freeze a table row, for autovacuum",
RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
},
! -1, 0, 1000000000
},
{
{
--- 169,175 ----
"Minimum age at which VACUUM should freeze a table row, for autovacuum",
RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
},
! -1, 0, 1000000000, 0
},
{
{
***************
*** 177,183 **** static relopt_int intRelOpts[] =
"Minimum multixact age at which VACUUM should freeze a row multixact's, for autovacuum",
RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
},
! -1, 0, 1000000000
},
{
{
--- 177,183 ----
"Minimum multixact age at which VACUUM should freeze a row multixact's, for autovacuum",
RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
},
! -1, 0, 1000000000, 0
},
{
{
***************
*** 185,191 **** static relopt_int intRelOpts[] =
"Age at which to autovacuum a table to prevent transaction ID wraparound",
RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
},
! -1, 100000000, 2000000000
},
{
{
--- 185,191 ----
"Age at which to autovacuum a table to prevent transaction ID wraparound",
RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
},
! -1, 100000000, 2000000000, 0
},
{
{
***************
*** 193,214 **** static relopt_int intRelOpts[] =
"Multixact age at which to autovacuum a table to prevent multixact wraparound",
RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
},
! -1, 100000000, 2000000000
},
{
{
"autovacuum_freeze_table_age",
"Age at which VACUUM should perform a full table sweep to freeze row versions",
RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
! }, -1, 0, 2000000000
},
{
{
"autovacuum_multixact_freeze_table_age",
"Age of multixact at which VACUUM should perform a full table sweep to freeze row versions",
RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
! }, -1, 0, 2000000000
},
/* list terminator */
{{NULL}}
--- 193,222 ----
"Multixact age at which to autovacuum a table to prevent multixact wraparound",
RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
},
! -1, 100000000, 2000000000, 0
},
{
{
"autovacuum_freeze_table_age",
"Age at which VACUUM should perform a full table sweep to freeze row versions",
RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
! }, -1, 0, 2000000000, 0
},
{
{
"autovacuum_multixact_freeze_table_age",
"Age of multixact at which VACUUM should perform a full table sweep to freeze row versions",
RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
! }, -1, 0, 2000000000, 0
},
+ {
+ {
+ "pending_list_cleanup_size",
+ "Maximum size of the pending list for this GIN index, in kilobytes.",
+ RELOPT_KIND_GIN
+ },
+ -1, 0, MAX_KILOBYTES, GUC_UNIT_KB
+ },
/* list terminator */
{{NULL}}
***************
*** 503,509 **** add_bool_reloption(bits32 kinds, char *name, char *desc, bool default_val)
*/
void
add_int_reloption(bits32 kinds, char *name, char *desc, int default_val,
! int min_val, int max_val)
{
relopt_int *newoption;
--- 511,517 ----
*/
void
add_int_reloption(bits32 kinds, char *name, char *desc, int default_val,
! int min_val, int max_val, int flags_val)
{
relopt_int *newoption;
***************
*** 512,517 **** add_int_reloption(bits32 kinds, char *name, char *desc, int default_val,
--- 520,526 ----
newoption->default_val = default_val;
newoption->min = min_val;
newoption->max = max_val;
+ newoption->flags = flags_val;
add_reloption((relopt_gen *) newoption);
}
***************
*** 1003,1014 **** parse_one_reloption(relopt_value *option, char *text_str, int text_len,
case RELOPT_TYPE_INT:
{
relopt_int *optint = (relopt_int *) option->gen;
! parsed = parse_int(value, &option->values.int_val, 0, NULL);
if (validate && !parsed)
ereport(ERROR,
(errmsg("invalid value for integer option \"%s\": %s",
! option->gen->name, value)));
if (validate && (option->values.int_val < optint->min ||
option->values.int_val > optint->max))
ereport(ERROR,
--- 1012,1026 ----
case RELOPT_TYPE_INT:
{
relopt_int *optint = (relopt_int *) option->gen;
+ const char *hintmsg;
! parsed = parse_int(value, &option->values.int_val,
! optint->flags, &hintmsg);
if (validate && !parsed)
ereport(ERROR,
(errmsg("invalid value for integer option \"%s\": %s",
! option->gen->name, value),
! hintmsg ? errhint("%s", _(hintmsg)) : 0));
if (validate && (option->values.int_val < optint->min ||
option->values.int_val > optint->max))
ereport(ERROR,
*** a/src/backend/access/gin/ginfast.c
--- b/src/backend/access/gin/ginfast.c
***************
*** 227,232 **** ginHeapTupleFastInsert(GinState *ginstate, GinTupleCollector *collector)
--- 227,233 ----
ginxlogUpdateMeta data;
bool separateList = false;
bool needCleanup = false;
+ int cleanupSize;
if (collector->ntuples == 0)
return;
***************
*** 421,431 **** ginHeapTupleFastInsert(GinState *ginstate, GinTupleCollector *collector)
* ginInsertCleanup could take significant amount of time, so we prefer to
* call it when it can do all the work in a single collection cycle. In
* non-vacuum mode, it shouldn't require maintenance_work_mem, so fire it
! * while pending list is still small enough to fit into work_mem.
*
* ginInsertCleanup() should not be called inside our CRIT_SECTION.
*/
! if (metadata->nPendingPages * GIN_PAGE_FREESIZE > work_mem * 1024L)
needCleanup = true;
UnlockReleaseBuffer(metabuffer);
--- 422,436 ----
* ginInsertCleanup could take significant amount of time, so we prefer to
* call it when it can do all the work in a single collection cycle. In
* non-vacuum mode, it shouldn't require maintenance_work_mem, so fire it
! * while pending list is still small enough to fit into
! * pending_list_cleanup_size (or work_mem if not set).
*
* ginInsertCleanup() should not be called inside our CRIT_SECTION.
*/
! cleanupSize = GinGetPendingListCleanupSize(index);
! if (cleanupSize == GIN_DEFAULT_PENDING_LIST_CLEANUP_SIZE)
! cleanupSize = work_mem;
! if (metadata->nPendingPages * GIN_PAGE_FREESIZE > cleanupSize * 1024L)
needCleanup = true;
UnlockReleaseBuffer(metabuffer);
*** a/src/backend/access/gin/ginutil.c
--- b/src/backend/access/gin/ginutil.c
***************
*** 524,530 **** ginoptions(PG_FUNCTION_ARGS)
GinOptions *rdopts;
int numoptions;
static const relopt_parse_elt tab[] = {
! {"fastupdate", RELOPT_TYPE_BOOL, offsetof(GinOptions, useFastUpdate)}
};
options = parseRelOptions(reloptions, validate, RELOPT_KIND_GIN,
--- 524,532 ----
GinOptions *rdopts;
int numoptions;
static const relopt_parse_elt tab[] = {
! {"fastupdate", RELOPT_TYPE_BOOL, offsetof(GinOptions, useFastUpdate)},
! {"pending_list_cleanup_size", RELOPT_TYPE_INT, offsetof(GinOptions,
! pendingListCleanupSize)}
};
options = parseRelOptions(reloptions, validate, RELOPT_KIND_GIN,
*** a/src/backend/utils/misc/guc.c
--- b/src/backend/utils/misc/guc.c
***************
*** 95,108 ****
#define CONFIG_EXEC_PARAMS_NEW "global/config_exec_params.new"
#endif
- /* upper limit for GUC variables measured in kilobytes of memory */
- /* note that various places assume the byte size fits in a "long" variable */
- #if SIZEOF_SIZE_T > 4 && SIZEOF_LONG > 4
- #define MAX_KILOBYTES INT_MAX
- #else
- #define MAX_KILOBYTES (INT_MAX / 1024)
- #endif
-
#define KB_PER_MB (1024)
#define KB_PER_GB (1024*1024)
#define KB_PER_TB (1024*1024*1024)
--- 95,100 ----
*** a/src/bin/pg_basebackup/pg_basebackup.c
--- b/src/bin/pg_basebackup/pg_basebackup.c
***************
*** 371,377 **** LogStreamerMain(logstreamer_param *param)
if (!ReceiveXlogStream(param->bgconn, param->startptr, param->timeline,
param->sysidentifier, param->xlogdir,
reached_end_position, standby_message_timeout,
! NULL, 0))
/*
* Any errors will already have been reported in the function process,
--- 371,377 ----
if (!ReceiveXlogStream(param->bgconn, param->startptr, param->timeline,
param->sysidentifier, param->xlogdir,
reached_end_position, standby_message_timeout,
! NULL))
/*
* Any errors will already have been reported in the function process,
*** a/src/bin/pg_basebackup/pg_receivexlog.c
--- b/src/bin/pg_basebackup/pg_receivexlog.c
***************
*** 36,42 **** static char *basedir = NULL;
static int verbose = 0;
static int noloop = 0;
static int standby_message_timeout = 10 * 1000; /* 10 sec = default */
- static int fsync_interval = 0; /* 0 = default */
static volatile bool time_to_abort = false;
--- 36,41 ----
***************
*** 63,70 **** usage(void)
printf(_("\nOptions:\n"));
printf(_(" -D, --directory=DIR receive transaction log files into this directory\n"));
printf(_(" -n, --no-loop do not loop on connection lost\n"));
- printf(_(" -F --fsync-interval=INTERVAL\n"
- " frequency of syncs to transaction log files (in seconds)\n"));
printf(_(" -v, --verbose output verbose messages\n"));
printf(_(" -V, --version output version information, then exit\n"));
printf(_(" -?, --help show this help, then exit\n"));
--- 62,67 ----
***************
*** 333,340 **** StreamLog(void)
starttli);
ReceiveXlogStream(conn, startpos, starttli, NULL, basedir,
! stop_streaming, standby_message_timeout, ".partial",
! fsync_interval);
PQfinish(conn);
}
--- 330,336 ----
starttli);
ReceiveXlogStream(conn, startpos, starttli, NULL, basedir,
! stop_streaming, standby_message_timeout, ".partial");
PQfinish(conn);
}
***************
*** 364,370 **** main(int argc, char **argv)
{"port", required_argument, NULL, 'p'},
{"username", required_argument, NULL, 'U'},
{"no-loop", no_argument, NULL, 'n'},
- {"fsync-interval", required_argument, NULL, 'F'},
{"no-password", no_argument, NULL, 'w'},
{"password", no_argument, NULL, 'W'},
{"status-interval", required_argument, NULL, 's'},
--- 360,365 ----
***************
*** 394,400 **** main(int argc, char **argv)
}
}
! while ((c = getopt_long(argc, argv, "D:d:h:p:U:s:nF:wWv",
long_options, &option_index)) != -1)
{
switch (c)
--- 389,395 ----
}
}
! while ((c = getopt_long(argc, argv, "D:d:h:p:U:s:nwWv",
long_options, &option_index)) != -1)
{
switch (c)
***************
*** 441,455 **** main(int argc, char **argv)
case 'n':
noloop = 1;
break;
- case 'F':
- fsync_interval = atoi(optarg) * 1000;
- if (fsync_interval < -1000)
- {
- fprintf(stderr, _("%s: invalid fsync interval \"%s\"\n"),
- progname, optarg);
- exit(1);
- }
- break;
case 'v':
verbose++;
break;
--- 436,441 ----
*** a/src/bin/pg_basebackup/receivelog.c
--- b/src/bin/pg_basebackup/receivelog.c
***************
*** 31,44 **** static char current_walfile_name[MAXPGPATH] = "";
static bool reportFlushPosition = false;
static XLogRecPtr lastFlushPosition = InvalidXLogRecPtr;
- static int64 last_fsync = -1; /* timestamp of last WAL file flush */
static bool still_sending = true; /* feedback still needs to be sent? */
static PGresult *HandleCopyStream(PGconn *conn, XLogRecPtr startpos,
uint32 timeline, char *basedir,
stream_stop_callback stream_stop, int standby_message_timeout,
! char *partial_suffix, XLogRecPtr *stoppos,
! int fsync_interval);
static int CopyStreamPoll(PGconn *conn, long timeout_ms);
static int CopyStreamReceive(PGconn *conn, long timeout, char **buffer);
static bool ProcessKeepaliveMsg(PGconn *conn, char *copybuf, int len,
--- 31,42 ----
static bool reportFlushPosition = false;
static XLogRecPtr lastFlushPosition = InvalidXLogRecPtr;
static bool still_sending = true; /* feedback still needs to be sent? */
static PGresult *HandleCopyStream(PGconn *conn, XLogRecPtr startpos,
uint32 timeline, char *basedir,
stream_stop_callback stream_stop, int standby_message_timeout,
! char *partial_suffix, XLogRecPtr *stoppos);
static int CopyStreamPoll(PGconn *conn, long timeout_ms);
static int CopyStreamReceive(PGconn *conn, long timeout, char **buffer);
static bool ProcessKeepaliveMsg(PGconn *conn, char *copybuf, int len,
***************
*** 50,62 **** static bool ProcessXLogDataMsg(PGconn *conn, char *copybuf, int len,
static PGresult *HandleEndOfCopyStream(PGconn *conn, char *copybuf,
XLogRecPtr blockpos, char *basedir, char *partial_suffix,
XLogRecPtr *stoppos);
- static bool CheckCopyStreamStop(PGconn *conn, XLogRecPtr blockpos,
- uint32 timeline, char *basedir,
- stream_stop_callback stream_stop,
- char *partial_suffix, XLogRecPtr *stoppos);
- static long CalculateCopyStreamSleeptime(int64 now, int standby_message_timeout,
- int64 last_status, int fsync_interval,
- XLogRecPtr blockpos);
static bool ReadEndOfStreamingResult(PGresult *res, XLogRecPtr *startpos,
uint32 *timeline);
--- 48,53 ----
***************
*** 209,215 **** close_walfile(char *basedir, char *partial_suffix, XLogRecPtr pos)
progname, current_walfile_name, partial_suffix);
lastFlushPosition = pos;
- last_fsync = feGetCurrentTimestamp();
return true;
}
--- 200,205 ----
***************
*** 440,456 **** CheckServerVersionForStreaming(PGconn *conn)
* allows you to tell the difference between partial and completed files,
* so that you can continue later where you left.
*
- * fsync_interval controls how often we flush to the received WAL file,
- * in milliseconds.
- *
* Note: The log position *must* be at a log segment start!
*/
bool
ReceiveXlogStream(PGconn *conn, XLogRecPtr startpos, uint32 timeline,
char *sysidentifier, char *basedir,
stream_stop_callback stream_stop,
! int standby_message_timeout, char *partial_suffix,
! int fsync_interval)
{
char query[128];
char slotcmd[128];
--- 430,442 ----
* allows you to tell the difference between partial and completed files,
* so that you can continue later where you left.
*
* Note: The log position *must* be at a log segment start!
*/
bool
ReceiveXlogStream(PGconn *conn, XLogRecPtr startpos, uint32 timeline,
char *sysidentifier, char *basedir,
stream_stop_callback stream_stop,
! int standby_message_timeout, char *partial_suffix)
{
char query[128];
char slotcmd[128];
***************
*** 595,601 **** ReceiveXlogStream(PGconn *conn, XLogRecPtr startpos, uint32 timeline,
/* Stream the WAL */
res = HandleCopyStream(conn, startpos, timeline, basedir, stream_stop,
standby_message_timeout, partial_suffix,
! &stoppos, fsync_interval);
if (res == NULL)
goto error;
--- 581,587 ----
/* Stream the WAL */
res = HandleCopyStream(conn, startpos, timeline, basedir, stream_stop,
standby_message_timeout, partial_suffix,
! &stoppos);
if (res == NULL)
goto error;
***************
*** 760,766 **** static PGresult *
HandleCopyStream(PGconn *conn, XLogRecPtr startpos, uint32 timeline,
char *basedir, stream_stop_callback stream_stop,
int standby_message_timeout, char *partial_suffix,
! XLogRecPtr *stoppos, int fsync_interval)
{
char *copybuf = NULL;
int64 last_status = -1;
--- 746,752 ----
HandleCopyStream(PGconn *conn, XLogRecPtr startpos, uint32 timeline,
char *basedir, stream_stop_callback stream_stop,
int standby_message_timeout, char *partial_suffix,
! XLogRecPtr *stoppos)
{
char *copybuf = NULL;
int64 last_status = -1;
***************
*** 777,812 **** HandleCopyStream(PGconn *conn, XLogRecPtr startpos, uint32 timeline,
/*
* Check if we should continue streaming, or abort at this point.
*/
! if (!CheckCopyStreamStop(conn, blockpos, timeline, basedir,
! stream_stop, partial_suffix, stoppos))
! goto error;
!
! now = feGetCurrentTimestamp();
!
! /*
! * If fsync_interval has elapsed since last WAL flush and we've written
! * some WAL data, flush them to disk.
! */
! if (lastFlushPosition < blockpos &&
! walfile != -1 &&
! ((fsync_interval > 0 &&
! feTimestampDifferenceExceeds(last_fsync, now, fsync_interval)) ||
! fsync_interval < 0))
{
! if (fsync(walfile) != 0)
{
! fprintf(stderr, _("%s: could not fsync file \"%s\": %s\n"),
! progname, current_walfile_name, strerror(errno));
goto error;
}
!
! lastFlushPosition = blockpos;
! last_fsync = now;
}
/*
* Potentially send a status message to the master
*/
if (still_sending && standby_message_timeout > 0 &&
feTimestampDifferenceExceeds(last_status, now,
standby_message_timeout))
--- 763,788 ----
/*
* Check if we should continue streaming, or abort at this point.
*/
! if (still_sending && stream_stop(blockpos, timeline, false))
{
! if (!close_walfile(basedir, partial_suffix, blockpos))
{
! /* Potential error message is written by close_walfile */
goto error;
}
! if (PQputCopyEnd(conn, NULL) <= 0 || PQflush(conn))
! {
! fprintf(stderr, _("%s: could not send copy-end packet: %s"),
! progname, PQerrorMessage(conn));
! goto error;
! }
! still_sending = false;
}
/*
* Potentially send a status message to the master
*/
+ now = feGetCurrentTimestamp();
if (still_sending && standby_message_timeout > 0 &&
feTimestampDifferenceExceeds(last_status, now,
standby_message_timeout))
***************
*** 818,875 **** HandleCopyStream(PGconn *conn, XLogRecPtr startpos, uint32 timeline,
}
/*
! * Calculate how long send/receive loops should sleep
*/
! sleeptime = CalculateCopyStreamSleeptime(now, standby_message_timeout,
! last_status, fsync_interval, blockpos);
!
! r = CopyStreamReceive(conn, sleeptime, ©buf);
! while (r != 0)
{
! if (r == -1)
! goto error;
! if (r == -2)
{
! PGresult *res = HandleEndOfCopyStream(conn, copybuf, blockpos,
! basedir, partial_suffix, stoppos);
! if (res == NULL)
! goto error;
! else
! return res;
}
! /* Check the message type. */
! if (copybuf[0] == 'k')
! {
! if (!ProcessKeepaliveMsg(conn, copybuf, r, blockpos,
! &last_status))
! goto error;
! }
! else if (copybuf[0] == 'w')
! {
! if (!ProcessXLogDataMsg(conn, copybuf, r, &blockpos,
! timeline, basedir, stream_stop, partial_suffix))
! goto error;
!
! /*
! * Check if we should continue streaming, or abort at this point.
! */
! if (!CheckCopyStreamStop(conn, blockpos, timeline, basedir,
! stream_stop, partial_suffix, stoppos))
! goto error;
! }
! else
! {
! fprintf(stderr, _("%s: unrecognized streaming header: \"%c\"\n"),
! progname, copybuf[0]);
goto error;
! }
! /*
! * Process the received data, and any subsequent data we
! * can read without blocking.
! */
! r = CopyStreamReceive(conn, 0, ©buf);
}
}
--- 794,857 ----
}
/*
! * Compute how long send/receive loops should sleep
*/
! if (standby_message_timeout && still_sending)
{
! int64 targettime;
! long secs;
! int usecs;
!
! targettime = last_status + (standby_message_timeout - 1) * ((int64) 1000);
! feTimestampDifference(now,
! targettime,
! &secs,
! &usecs);
! /* Always sleep at least 1 sec */
! if (secs <= 0)
{
! secs = 1;
! usecs = 0;
}
! sleeptime = secs * 1000 + usecs / 1000;
! }
! else
! sleeptime = -1;
!
! r = CopyStreamReceive(conn, sleeptime, ©buf);
! if (r == 0)
! continue;
! if (r == -1)
! goto error;
! if (r == -2)
! {
! PGresult *res = HandleEndOfCopyStream(conn, copybuf, blockpos,
! basedir, partial_suffix, stoppos);
! if (res == NULL)
goto error;
! else
! return res;
! }
! /* Check the message type. */
! if (copybuf[0] == 'k')
! {
! if (!ProcessKeepaliveMsg(conn, copybuf, r, blockpos,
! &last_status))
! goto error;
! }
! else if (copybuf[0] == 'w')
! {
! if (!ProcessXLogDataMsg(conn, copybuf, r, &blockpos,
! timeline, basedir, stream_stop, partial_suffix))
! goto error;
! }
! else
! {
! fprintf(stderr, _("%s: unrecognized streaming header: \"%c\"\n"),
! progname, copybuf[0]);
! goto error;
}
}
***************
*** 1211,1290 **** HandleEndOfCopyStream(PGconn *conn, char *copybuf,
*stoppos = blockpos;
return res;
}
-
- /*
- * Check if we should continue streaming, or abort at this point.
- */
- static bool
- CheckCopyStreamStop(PGconn *conn, XLogRecPtr blockpos, uint32 timeline,
- char *basedir, stream_stop_callback stream_stop,
- char *partial_suffix, XLogRecPtr *stoppos)
- {
- if (still_sending && stream_stop(blockpos, timeline, false))
- {
- if (!close_walfile(basedir, partial_suffix, blockpos))
- {
- /* Potential error message is written by close_walfile */
- return false;
- }
- if (PQputCopyEnd(conn, NULL) <= 0 || PQflush(conn))
- {
- fprintf(stderr, _("%s: could not send copy-end packet: %s"),
- progname, PQerrorMessage(conn));
- return false;
- }
- still_sending = false;
- }
-
- return true;
- }
-
- /*
- * Calculate how long send/receive loops should sleep
- */
- static long
- CalculateCopyStreamSleeptime(int64 now, int standby_message_timeout,
- int64 last_status, int fsync_interval, XLogRecPtr blockpos)
- {
- int64 targettime = 0;
- int64 status_targettime = 0;
- int64 fsync_targettime = 0;
- long sleeptime;
-
- if (standby_message_timeout && still_sending)
- status_targettime = last_status +
- (standby_message_timeout - 1) * ((int64) 1000);
-
- if (fsync_interval > 0 && lastFlushPosition < blockpos)
- fsync_targettime = last_fsync +
- (fsync_interval - 1) * ((int64) 1000);
-
- if ((status_targettime < fsync_targettime && status_targettime > 0) ||
- fsync_targettime == 0)
- targettime = status_targettime;
- else
- targettime = fsync_targettime;
-
- if (targettime > 0)
- {
- long secs;
- int usecs;
-
- feTimestampDifference(now,
- targettime,
- &secs,
- &usecs);
- /* Always sleep at least 1 sec */
- if (secs <= 0)
- {
- secs = 1;
- usecs = 0;
- }
-
- sleeptime = secs * 1000 + usecs / 1000;
- }
- else
- sleeptime = -1;
-
- return sleeptime;
- }
--- 1193,1195 ----
*** a/src/bin/pg_basebackup/receivelog.h
--- b/src/bin/pg_basebackup/receivelog.h
***************
*** 16,20 **** extern bool ReceiveXlogStream(PGconn *conn,
char *basedir,
stream_stop_callback stream_stop,
int standby_message_timeout,
! char *partial_suffix,
! int fsync_interval);
--- 16,19 ----
char *basedir,
stream_stop_callback stream_stop,
int standby_message_timeout,
! char *partial_suffix);
*** a/src/bin/psql/tab-complete.c
--- b/src/bin/psql/tab-complete.c
***************
*** 1134,1140 **** psql_completion(const char *text, int start, int end)
pg_strcasecmp(prev_wd, "(") == 0)
{
static const char *const list_INDEXOPTIONS[] =
! {"fillfactor", "fastupdate", NULL};
COMPLETE_WITH_LIST(list_INDEXOPTIONS);
}
--- 1134,1140 ----
pg_strcasecmp(prev_wd, "(") == 0)
{
static const char *const list_INDEXOPTIONS[] =
! {"fillfactor", "fastupdate", "pending_list_cleanup_size", NULL};
COMPLETE_WITH_LIST(list_INDEXOPTIONS);
}
*** a/src/include/access/gin_private.h
--- b/src/include/access/gin_private.h
***************
*** 314,325 **** typedef struct GinOptions
--- 314,331 ----
{
int32 vl_len_; /* varlena header (do not touch directly!) */
bool useFastUpdate; /* use fast updates? */
+ int pendingListCleanupSize; /* maximum size of pending list */
} GinOptions;
#define GIN_DEFAULT_USE_FASTUPDATE true
#define GinGetUseFastUpdate(relation) \
((relation)->rd_options ? \
((GinOptions *) (relation)->rd_options)->useFastUpdate : GIN_DEFAULT_USE_FASTUPDATE)
+ #define GIN_DEFAULT_PENDING_LIST_CLEANUP_SIZE -1
+ #define GinGetPendingListCleanupSize(relation) \
+ ((relation)->rd_options ? \
+ ((GinOptions *) (relation)->rd_options)->pendingListCleanupSize : \
+ GIN_DEFAULT_PENDING_LIST_CLEANUP_SIZE)
/* Macros for buffer lock/unlock operations */
*** a/src/include/access/reloptions.h
--- b/src/include/access/reloptions.h
***************
*** 92,97 **** typedef struct relopt_int
--- 92,98 ----
int default_val;
int min;
int max;
+ int flags;
} relopt_int;
typedef struct relopt_real
***************
*** 244,250 **** extern relopt_kind add_reloption_kind(void);
extern void add_bool_reloption(bits32 kinds, char *name, char *desc,
bool default_val);
extern void add_int_reloption(bits32 kinds, char *name, char *desc,
! int default_val, int min_val, int max_val);
extern void add_real_reloption(bits32 kinds, char *name, char *desc,
double default_val, double min_val, double max_val);
extern void add_string_reloption(bits32 kinds, char *name, char *desc,
--- 245,251 ----
extern void add_bool_reloption(bits32 kinds, char *name, char *desc,
bool default_val);
extern void add_int_reloption(bits32 kinds, char *name, char *desc,
! int default_val, int min_val, int max_val, int flags_val);
extern void add_real_reloption(bits32 kinds, char *name, char *desc,
double default_val, double min_val, double max_val);
extern void add_string_reloption(bits32 kinds, char *name, char *desc,
*** a/src/include/utils/guc.h
--- b/src/include/utils/guc.h
***************
*** 18,23 ****
--- 18,31 ----
#include "utils/array.h"
+ /* upper limit for GUC variables measured in kilobytes of memory */
+ /* note that various places assume the byte size fits in a "long" variable */
+ #if SIZEOF_SIZE_T > 4 && SIZEOF_LONG > 4
+ #define MAX_KILOBYTES INT_MAX
+ #else
+ #define MAX_KILOBYTES (INT_MAX / 1024)
+ #endif
+
/*
* Certain options can only be set at certain times. The rules are
* like this:
On Fri, Aug 8, 2014 at 11:43 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
The attached patch introduces...
A patch perhaps? :)
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Aug 8, 2014 at 11:45 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
On Fri, Aug 8, 2014 at 11:43 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
On Mon, Apr 14, 2014 at 11:40 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Fujii Masao <masao.fujii@gmail.com> writes:
On Tue, Apr 1, 2014 at 1:41 AM, Robert Haas <robertmhaas@gmail.com> wrote:
Should we try to install some hack around fastupdate for 9.4? I fear
the divergence between reasonable values of work_mem and reasonable
sizes for that list is only going to continue to get bigger. I'm sure
there's somebody out there who has work_mem = 16GB, and stuff like
263865a48973767ce8ed7b7788059a38a24a9f37 is only going to increase the
appeal of large values.Controlling the threshold of the size of pending list only by GUC doesn't
seem reasonable. Users may want to increase the threshold only for the
GIN index which can be updated heavily, and decrease it otherwise. So
I think that it's better to add new storage parameter for GIN index to control
the threshold, or both storage parameter and GUC.Yeah, -1 for a GUC. A GIN-index-specific storage parameter seems more
appropriate.The attached patch introduces the GIN index storage parameter
"PENDING_LIST_CLEANUP_SIZE" which specifies the maximum size of
GIN pending list. If it's not set, work_mem is used as that maximum size,
instead. So this patch doesn't break the existing application which
currently uses work_mem as the threshold of cleanup operation of
the pending list. It has only not to set PENDING_LIST_CLEANUP_SIZE.This is an index storage parameter, and allows us to specify each
threshold per GIN index. So, for example, we can increase the threshold
only for the GIN index which can be updated heavily, and decrease it otherwise.This patch uses another patch that I proposed (*1) as an infrastructure.
Please apply that infrastructure patch first if you apply this patch.(*1)
/messages/by-id/CAHGQGwEanQ_e8WLHL25=bm_8Z5zkyZw0K0yiR+kdMV2HgnE9FQ@mail.gmail.comRegards,
--
Fujii MasaoSorry, I forgot to attached the patch.... This time, attached.
I think that this patch should be rebased.
It contains garbage code.
Regards,
-------
Sawada Masahiko
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Sat, Aug 16, 2014 at 4:23 PM, Sawada Masahiko <sawada.mshk@gmail.com> wrote:
On Fri, Aug 8, 2014 at 11:45 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
On Fri, Aug 8, 2014 at 11:43 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
On Mon, Apr 14, 2014 at 11:40 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Fujii Masao <masao.fujii@gmail.com> writes:
On Tue, Apr 1, 2014 at 1:41 AM, Robert Haas <robertmhaas@gmail.com> wrote:
Should we try to install some hack around fastupdate for 9.4? I fear
the divergence between reasonable values of work_mem and reasonable
sizes for that list is only going to continue to get bigger. I'm sure
there's somebody out there who has work_mem = 16GB, and stuff like
263865a48973767ce8ed7b7788059a38a24a9f37 is only going to increase the
appeal of large values.Controlling the threshold of the size of pending list only by GUC doesn't
seem reasonable. Users may want to increase the threshold only for the
GIN index which can be updated heavily, and decrease it otherwise. So
I think that it's better to add new storage parameter for GIN index to control
the threshold, or both storage parameter and GUC.Yeah, -1 for a GUC. A GIN-index-specific storage parameter seems more
appropriate.The attached patch introduces the GIN index storage parameter
"PENDING_LIST_CLEANUP_SIZE" which specifies the maximum size of
GIN pending list. If it's not set, work_mem is used as that maximum size,
instead. So this patch doesn't break the existing application which
currently uses work_mem as the threshold of cleanup operation of
the pending list. It has only not to set PENDING_LIST_CLEANUP_SIZE.This is an index storage parameter, and allows us to specify each
threshold per GIN index. So, for example, we can increase the threshold
only for the GIN index which can be updated heavily, and decrease it otherwise.This patch uses another patch that I proposed (*1) as an infrastructure.
Please apply that infrastructure patch first if you apply this patch.(*1)
/messages/by-id/CAHGQGwEanQ_e8WLHL25=bm_8Z5zkyZw0K0yiR+kdMV2HgnE9FQ@mail.gmail.comRegards,
--
Fujii MasaoSorry, I forgot to attached the patch.... This time, attached.
I think that this patch should be rebased.
It contains garbage code.
Thanks for reviewing the patch! ISTM that I failed to make the patch from
my git repository... Attached is the rebased version.
Regards,
--
Fujii Masao
Attachments:
pending_list_cleanup_size_v2.patchtext/x-patch; charset=US-ASCII; name=pending_list_cleanup_size_v2.patchDownload
diff --git a/doc/src/sgml/gin.sgml b/doc/src/sgml/gin.sgml
index 80a578d..24c8dc1 100644
--- a/doc/src/sgml/gin.sgml
+++ b/doc/src/sgml/gin.sgml
@@ -728,8 +728,9 @@
from the indexed item). As of <productname>PostgreSQL</productname> 8.4,
<acronym>GIN</> is capable of postponing much of this work by inserting
new tuples into a temporary, unsorted list of pending entries.
- When the table is vacuumed, or if the pending list becomes too large
- (larger than <xref linkend="guc-work-mem">), the entries are moved to the
+ When the table is vacuumed, or if the pending list becomes larger than
+ <literal>PENDING_LIST_CLEANUP_SIZE</literal> (or
+ <xref linkend="guc-work-mem"> if not set), the entries are moved to the
main <acronym>GIN</acronym> data structure using the same bulk insert
techniques used during initial index creation. This greatly improves
<acronym>GIN</acronym> index update speed, even counting the additional
@@ -812,18 +813,27 @@
</varlistentry>
<varlistentry>
- <term><xref linkend="guc-work-mem"></term>
+ <term><literal>PENDING_LIST_CLEANUP_SIZE</> and
+ <xref linkend="guc-work-mem"></term>
<listitem>
<para>
During a series of insertions into an existing <acronym>GIN</acronym>
index that has <literal>FASTUPDATE</> enabled, the system will clean up
the pending-entry list whenever the list grows larger than
- <varname>work_mem</>. To avoid fluctuations in observed response time,
- it's desirable to have pending-list cleanup occur in the background
- (i.e., via autovacuum). Foreground cleanup operations can be avoided by
- increasing <varname>work_mem</> or making autovacuum more aggressive.
- However, enlarging <varname>work_mem</> means that if a foreground
- cleanup does occur, it will take even longer.
+ <literal>PENDING_LIST_CLEANUP_SIZE</> (if not set, <varname>work_mem</>
+ is used as that threshold, instead). To avoid fluctuations in observed
+ response time, it's desirable to have pending-list cleanup occur in the
+ background (i.e., via autovacuum). Foreground cleanup operations
+ can be avoided by increasing <literal>PENDING_LIST_CLEANUP_SIZE</>
+ (or <varname>work_mem</>) or making autovacuum more aggressive.
+ However, enlarging the threshold of the cleanup operation means that
+ if a foreground cleanup does occur, it will take even longer.
+ </para>
+ <para>
+ <literal>PENDING_LIST_CLEANUP_SIZE</> is an index storage parameter,
+ and allows each GIN index to have its own cleanup threshold.
+ For example, it's possible to increase the threshold only for the GIN
+ index which can be updated heavily, and decrease it otherwise.
</para>
</listitem>
</varlistentry>
diff --git a/doc/src/sgml/ref/create_index.sgml b/doc/src/sgml/ref/create_index.sgml
index e469b17..1589812 100644
--- a/doc/src/sgml/ref/create_index.sgml
+++ b/doc/src/sgml/ref/create_index.sgml
@@ -356,6 +356,22 @@ CREATE [ UNIQUE ] INDEX [ CONCURRENTLY ] [ <replaceable class="parameter">name</
</listitem>
</varlistentry>
</variablelist>
+ <variablelist>
+ <varlistentry>
+ <term><literal>PENDING_LIST_CLEANUP_SIZE</></term>
+ <listitem>
+ <para>
+ This setting specifies the maximum size of the GIN pending list which is
+ used when <literal>FASTUPDATE</> is enabled. If the list grows larger than
+ this maximum size, it is cleaned up by moving the entries in it to the
+ main GIN data structure in bulk. The value is specified in kilobytes.
+ If this is not set, <literal>work_mem</> is used as the maximum size
+ of the pending list, instead. See <xref linkend="gin-fast-update"> and
+ <xref linkend="gin-tips"> for more information.
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
</refsect2>
<refsect2 id="SQL-CREATEINDEX-CONCURRENTLY">
diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index 9a03fdc..6ffe7e8 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -209,6 +209,14 @@ static relopt_int intRelOpts[] =
RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
}, -1, 0, 2000000000, 0
},
+ {
+ {
+ "pending_list_cleanup_size",
+ "Maximum size of the pending list for this GIN index, in kilobytes.",
+ RELOPT_KIND_GIN
+ },
+ -1, 0, MAX_KILOBYTES, GUC_UNIT_KB
+ },
/* list terminator */
{{NULL}}
diff --git a/src/backend/access/gin/ginfast.c b/src/backend/access/gin/ginfast.c
index 09c3e39..230ef17 100644
--- a/src/backend/access/gin/ginfast.c
+++ b/src/backend/access/gin/ginfast.c
@@ -227,6 +227,7 @@ ginHeapTupleFastInsert(GinState *ginstate, GinTupleCollector *collector)
ginxlogUpdateMeta data;
bool separateList = false;
bool needCleanup = false;
+ int cleanupSize;
if (collector->ntuples == 0)
return;
@@ -421,11 +422,15 @@ ginHeapTupleFastInsert(GinState *ginstate, GinTupleCollector *collector)
* ginInsertCleanup could take significant amount of time, so we prefer to
* call it when it can do all the work in a single collection cycle. In
* non-vacuum mode, it shouldn't require maintenance_work_mem, so fire it
- * while pending list is still small enough to fit into work_mem.
+ * while pending list is still small enough to fit into
+ * pending_list_cleanup_size (or work_mem if not set).
*
* ginInsertCleanup() should not be called inside our CRIT_SECTION.
*/
- if (metadata->nPendingPages * GIN_PAGE_FREESIZE > work_mem * 1024L)
+ cleanupSize = GinGetPendingListCleanupSize(index);
+ if (cleanupSize == GIN_DEFAULT_PENDING_LIST_CLEANUP_SIZE)
+ cleanupSize = work_mem;
+ if (metadata->nPendingPages * GIN_PAGE_FREESIZE > cleanupSize * 1024L)
needCleanup = true;
UnlockReleaseBuffer(metabuffer);
diff --git a/src/backend/access/gin/ginutil.c b/src/backend/access/gin/ginutil.c
index 3ca0b68..a1e959a 100644
--- a/src/backend/access/gin/ginutil.c
+++ b/src/backend/access/gin/ginutil.c
@@ -524,7 +524,9 @@ ginoptions(PG_FUNCTION_ARGS)
GinOptions *rdopts;
int numoptions;
static const relopt_parse_elt tab[] = {
- {"fastupdate", RELOPT_TYPE_BOOL, offsetof(GinOptions, useFastUpdate)}
+ {"fastupdate", RELOPT_TYPE_BOOL, offsetof(GinOptions, useFastUpdate)},
+ {"pending_list_cleanup_size", RELOPT_TYPE_INT, offsetof(GinOptions,
+ pendingListCleanupSize)}
};
options = parseRelOptions(reloptions, validate, RELOPT_KIND_GIN,
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 60e4354..973127e 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -95,14 +95,6 @@
#define CONFIG_EXEC_PARAMS_NEW "global/config_exec_params.new"
#endif
-/* upper limit for GUC variables measured in kilobytes of memory */
-/* note that various places assume the byte size fits in a "long" variable */
-#if SIZEOF_SIZE_T > 4 && SIZEOF_LONG > 4
-#define MAX_KILOBYTES INT_MAX
-#else
-#define MAX_KILOBYTES (INT_MAX / 1024)
-#endif
-
#define KB_PER_MB (1024)
#define KB_PER_GB (1024*1024)
#define KB_PER_TB (1024*1024*1024)
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index b4f1856..543ad38 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1137,7 +1137,7 @@ psql_completion(const char *text, int start, int end)
pg_strcasecmp(prev_wd, "(") == 0)
{
static const char *const list_INDEXOPTIONS[] =
- {"fillfactor", "fastupdate", NULL};
+ {"fillfactor", "fastupdate", "pending_list_cleanup_size", NULL};
COMPLETE_WITH_LIST(list_INDEXOPTIONS);
}
diff --git a/src/include/access/gin_private.h b/src/include/access/gin_private.h
index 3baa9f5..ad0777d 100644
--- a/src/include/access/gin_private.h
+++ b/src/include/access/gin_private.h
@@ -314,12 +314,18 @@ typedef struct GinOptions
{
int32 vl_len_; /* varlena header (do not touch directly!) */
bool useFastUpdate; /* use fast updates? */
+ int pendingListCleanupSize; /* maximum size of pending list */
} GinOptions;
#define GIN_DEFAULT_USE_FASTUPDATE true
#define GinGetUseFastUpdate(relation) \
((relation)->rd_options ? \
((GinOptions *) (relation)->rd_options)->useFastUpdate : GIN_DEFAULT_USE_FASTUPDATE)
+#define GIN_DEFAULT_PENDING_LIST_CLEANUP_SIZE -1
+#define GinGetPendingListCleanupSize(relation) \
+ ((relation)->rd_options ? \
+ ((GinOptions *) (relation)->rd_options)->pendingListCleanupSize : \
+ GIN_DEFAULT_PENDING_LIST_CLEANUP_SIZE)
/* Macros for buffer lock/unlock operations */
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index 0a729c1..5b5bd77 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -18,6 +18,14 @@
#include "utils/array.h"
+/* upper limit for GUC variables measured in kilobytes of memory */
+/* note that various places assume the byte size fits in a "long" variable */
+#if SIZEOF_SIZE_T > 4 && SIZEOF_LONG > 4
+#define MAX_KILOBYTES INT_MAX
+#else
+#define MAX_KILOBYTES (INT_MAX / 1024)
+#endif
+
/*
* Certain options can only be set at certain times. The rules are
* like this:
On Sun, Aug 17, 2014 at 7:46 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
Thanks for reviewing the patch! ISTM that I failed to make the patch from
my git repository... Attached is the rebased version.
I get some compiler warnings on v2 of this patch:
reloptions.c:219: warning: excess elements in struct initializer
reloptions.c:219: warning: (near initialization for 'intRelOpts[15]')
Cheers,
Jeff
On Tue, Sep 9, 2014 at 1:28 AM, Jeff Janes <jeff.janes@gmail.com> wrote:
On Sun, Aug 17, 2014 at 7:46 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
Thanks for reviewing the patch! ISTM that I failed to make the patch from
my git repository... Attached is the rebased version.I get some compiler warnings on v2 of this patch:
reloptions.c:219: warning: excess elements in struct initializer
reloptions.c:219: warning: (near initialization for 'intRelOpts[15]')
Thanks for testing the patch!
Attached is the updated version of the patch.
Previously the patch depended on another infrastructure patch
(which allows a user to specify the unit in reloption (*1)). But that
infrastructure patch has serious problem and it's not easy to fix
the problem. So I changed the patch so that it doesn't depend on
that infrastructure patch at all. Even without the infrastructure
patch, the feature that this patch introduces is useful.
Also I added the regression test into the patch.
(*1)
/messages/by-id/CAHGQGwEanQ_e8WLHL25=bm_8Z5zkyZw0K0yiR+kdMV2HgnE9FQ@mail.gmail.com
Regards,
--
Fujii Masao
Attachments:
pending_list_cleanup_size_v3.patchtext/x-patch; charset=US-ASCII; name=pending_list_cleanup_size_v3.patchDownload
*** a/doc/src/sgml/gin.sgml
--- b/doc/src/sgml/gin.sgml
***************
*** 728,735 ****
from the indexed item). As of <productname>PostgreSQL</productname> 8.4,
<acronym>GIN</> is capable of postponing much of this work by inserting
new tuples into a temporary, unsorted list of pending entries.
! When the table is vacuumed, or if the pending list becomes too large
! (larger than <xref linkend="guc-work-mem">), the entries are moved to the
main <acronym>GIN</acronym> data structure using the same bulk insert
techniques used during initial index creation. This greatly improves
<acronym>GIN</acronym> index update speed, even counting the additional
--- 728,736 ----
from the indexed item). As of <productname>PostgreSQL</productname> 8.4,
<acronym>GIN</> is capable of postponing much of this work by inserting
new tuples into a temporary, unsorted list of pending entries.
! When the table is vacuumed, or if the pending list becomes larger than
! <literal>PENDING_LIST_CLEANUP_SIZE</literal> (or
! <xref linkend="guc-work-mem"> if not set), the entries are moved to the
main <acronym>GIN</acronym> data structure using the same bulk insert
techniques used during initial index creation. This greatly improves
<acronym>GIN</acronym> index update speed, even counting the additional
***************
*** 812,829 ****
</varlistentry>
<varlistentry>
! <term><xref linkend="guc-work-mem"></term>
<listitem>
<para>
During a series of insertions into an existing <acronym>GIN</acronym>
index that has <literal>FASTUPDATE</> enabled, the system will clean up
the pending-entry list whenever the list grows larger than
! <varname>work_mem</>. To avoid fluctuations in observed response time,
! it's desirable to have pending-list cleanup occur in the background
! (i.e., via autovacuum). Foreground cleanup operations can be avoided by
! increasing <varname>work_mem</> or making autovacuum more aggressive.
! However, enlarging <varname>work_mem</> means that if a foreground
! cleanup does occur, it will take even longer.
</para>
</listitem>
</varlistentry>
--- 813,839 ----
</varlistentry>
<varlistentry>
! <term><literal>PENDING_LIST_CLEANUP_SIZE</> and
! <xref linkend="guc-work-mem"></term>
<listitem>
<para>
During a series of insertions into an existing <acronym>GIN</acronym>
index that has <literal>FASTUPDATE</> enabled, the system will clean up
the pending-entry list whenever the list grows larger than
! <literal>PENDING_LIST_CLEANUP_SIZE</> (if not set, <varname>work_mem</>
! is used as that threshold, instead). To avoid fluctuations in observed
! response time, it's desirable to have pending-list cleanup occur in the
! background (i.e., via autovacuum). Foreground cleanup operations
! can be avoided by increasing <literal>PENDING_LIST_CLEANUP_SIZE</>
! (or <varname>work_mem</>) or making autovacuum more aggressive.
! However, enlarging the threshold of the cleanup operation means that
! if a foreground cleanup does occur, it will take even longer.
! </para>
! <para>
! <literal>PENDING_LIST_CLEANUP_SIZE</> is an index storage parameter,
! and allows each GIN index to have its own cleanup threshold.
! For example, it's possible to increase the threshold only for the GIN
! index which can be updated heavily, and decrease it otherwise.
</para>
</listitem>
</varlistentry>
*** a/doc/src/sgml/ref/create_index.sgml
--- b/doc/src/sgml/ref/create_index.sgml
***************
*** 356,361 **** CREATE [ UNIQUE ] INDEX [ CONCURRENTLY ] [ <replaceable class="parameter">name</
--- 356,377 ----
</listitem>
</varlistentry>
</variablelist>
+ <variablelist>
+ <varlistentry>
+ <term><literal>PENDING_LIST_CLEANUP_SIZE</></term>
+ <listitem>
+ <para>
+ This setting specifies the maximum size of the GIN pending list which is
+ used when <literal>FASTUPDATE</> is enabled. If the list grows larger than
+ this maximum size, it is cleaned up by moving the entries in it to the
+ main GIN data structure in bulk. The value is specified in kilobytes.
+ If this is not set, <literal>work_mem</> is used as the maximum size
+ of the pending list, instead. See <xref linkend="gin-fast-update"> and
+ <xref linkend="gin-tips"> for more information.
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
</refsect2>
<refsect2 id="SQL-CREATEINDEX-CONCURRENTLY">
*** a/src/backend/access/common/reloptions.c
--- b/src/backend/access/common/reloptions.c
***************
*** 209,214 **** static relopt_int intRelOpts[] =
--- 209,222 ----
RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
}, -1, 0, 2000000000
},
+ {
+ {
+ "pending_list_cleanup_size",
+ "Maximum size of the pending list for this GIN index, in kilobytes.",
+ RELOPT_KIND_GIN
+ },
+ -1, 0, MAX_KILOBYTES
+ },
/* list terminator */
{{NULL}}
*** a/src/backend/access/gin/ginfast.c
--- b/src/backend/access/gin/ginfast.c
***************
*** 227,232 **** ginHeapTupleFastInsert(GinState *ginstate, GinTupleCollector *collector)
--- 227,233 ----
ginxlogUpdateMeta data;
bool separateList = false;
bool needCleanup = false;
+ int cleanupSize;
if (collector->ntuples == 0)
return;
***************
*** 421,431 **** ginHeapTupleFastInsert(GinState *ginstate, GinTupleCollector *collector)
* ginInsertCleanup could take significant amount of time, so we prefer to
* call it when it can do all the work in a single collection cycle. In
* non-vacuum mode, it shouldn't require maintenance_work_mem, so fire it
! * while pending list is still small enough to fit into work_mem.
*
* ginInsertCleanup() should not be called inside our CRIT_SECTION.
*/
! if (metadata->nPendingPages * GIN_PAGE_FREESIZE > work_mem * 1024L)
needCleanup = true;
UnlockReleaseBuffer(metabuffer);
--- 422,436 ----
* ginInsertCleanup could take significant amount of time, so we prefer to
* call it when it can do all the work in a single collection cycle. In
* non-vacuum mode, it shouldn't require maintenance_work_mem, so fire it
! * while pending list is still small enough to fit into
! * pending_list_cleanup_size (or work_mem if not set).
*
* ginInsertCleanup() should not be called inside our CRIT_SECTION.
*/
! cleanupSize = GinGetPendingListCleanupSize(index);
! if (cleanupSize == GIN_DEFAULT_PENDING_LIST_CLEANUP_SIZE)
! cleanupSize = work_mem;
! if (metadata->nPendingPages * GIN_PAGE_FREESIZE > cleanupSize * 1024L)
needCleanup = true;
UnlockReleaseBuffer(metabuffer);
*** a/src/backend/access/gin/ginutil.c
--- b/src/backend/access/gin/ginutil.c
***************
*** 524,530 **** ginoptions(PG_FUNCTION_ARGS)
GinOptions *rdopts;
int numoptions;
static const relopt_parse_elt tab[] = {
! {"fastupdate", RELOPT_TYPE_BOOL, offsetof(GinOptions, useFastUpdate)}
};
options = parseRelOptions(reloptions, validate, RELOPT_KIND_GIN,
--- 524,532 ----
GinOptions *rdopts;
int numoptions;
static const relopt_parse_elt tab[] = {
! {"fastupdate", RELOPT_TYPE_BOOL, offsetof(GinOptions, useFastUpdate)},
! {"pending_list_cleanup_size", RELOPT_TYPE_INT, offsetof(GinOptions,
! pendingListCleanupSize)}
};
options = parseRelOptions(reloptions, validate, RELOPT_KIND_GIN,
*** a/src/backend/utils/misc/guc.c
--- b/src/backend/utils/misc/guc.c
***************
*** 95,108 ****
#define CONFIG_EXEC_PARAMS_NEW "global/config_exec_params.new"
#endif
- /* upper limit for GUC variables measured in kilobytes of memory */
- /* note that various places assume the byte size fits in a "long" variable */
- #if SIZEOF_SIZE_T > 4 && SIZEOF_LONG > 4
- #define MAX_KILOBYTES INT_MAX
- #else
- #define MAX_KILOBYTES (INT_MAX / 1024)
- #endif
-
#define KB_PER_MB (1024)
#define KB_PER_GB (1024*1024)
#define KB_PER_TB (1024*1024*1024)
--- 95,100 ----
*** a/src/bin/psql/tab-complete.c
--- b/src/bin/psql/tab-complete.c
***************
*** 1171,1177 **** psql_completion(const char *text, int start, int end)
pg_strcasecmp(prev_wd, "(") == 0)
{
static const char *const list_INDEXOPTIONS[] =
! {"fillfactor", "fastupdate", NULL};
COMPLETE_WITH_LIST(list_INDEXOPTIONS);
}
--- 1171,1177 ----
pg_strcasecmp(prev_wd, "(") == 0)
{
static const char *const list_INDEXOPTIONS[] =
! {"fillfactor", "fastupdate", "pending_list_cleanup_size", NULL};
COMPLETE_WITH_LIST(list_INDEXOPTIONS);
}
*** a/src/include/access/gin_private.h
--- b/src/include/access/gin_private.h
***************
*** 314,325 **** typedef struct GinOptions
--- 314,331 ----
{
int32 vl_len_; /* varlena header (do not touch directly!) */
bool useFastUpdate; /* use fast updates? */
+ int pendingListCleanupSize; /* maximum size of pending list */
} GinOptions;
#define GIN_DEFAULT_USE_FASTUPDATE true
#define GinGetUseFastUpdate(relation) \
((relation)->rd_options ? \
((GinOptions *) (relation)->rd_options)->useFastUpdate : GIN_DEFAULT_USE_FASTUPDATE)
+ #define GIN_DEFAULT_PENDING_LIST_CLEANUP_SIZE -1
+ #define GinGetPendingListCleanupSize(relation) \
+ ((relation)->rd_options ? \
+ ((GinOptions *) (relation)->rd_options)->pendingListCleanupSize : \
+ GIN_DEFAULT_PENDING_LIST_CLEANUP_SIZE)
/* Macros for buffer lock/unlock operations */
*** a/src/include/utils/guc.h
--- b/src/include/utils/guc.h
***************
*** 18,23 ****
--- 18,31 ----
#include "utils/array.h"
+ /* upper limit for GUC variables measured in kilobytes of memory */
+ /* note that various places assume the byte size fits in a "long" variable */
+ #if SIZEOF_SIZE_T > 4 && SIZEOF_LONG > 4
+ #define MAX_KILOBYTES INT_MAX
+ #else
+ #define MAX_KILOBYTES (INT_MAX / 1024)
+ #endif
+
/*
* Certain options can only be set at certain times. The rules are
* like this:
*** a/src/test/regress/expected/create_index.out
--- b/src/test/regress/expected/create_index.out
***************
*** 2235,2240 **** SELECT COUNT(*) FROM array_gin_test WHERE a @> '{2}';
--- 2235,2253 ----
DROP TABLE array_gin_test;
--
+ -- Test GIN index's reloptions
+ --
+ CREATE INDEX gin_relopts_test ON array_index_op_test USING gin (i)
+ WITH (FASTUPDATE=on, PENDING_LIST_CLEANUP_SIZE=32);
+ \d+ gin_relopts_test
+ Index "public.gin_relopts_test"
+ Column | Type | Definition | Storage
+ --------+---------+------------+---------
+ i | integer | i | plain
+ gin, for table "public.array_index_op_test"
+ Options: fastupdate=on, pending_list_cleanup_size=32
+
+ --
-- HASH
--
CREATE INDEX hash_i4_index ON hash_i4_heap USING hash (random int4_ops);
*** a/src/test/regress/sql/create_index.sql
--- b/src/test/regress/sql/create_index.sql
***************
*** 651,656 **** SELECT COUNT(*) FROM array_gin_test WHERE a @> '{2}';
--- 651,663 ----
DROP TABLE array_gin_test;
--
+ -- Test GIN index's reloptions
+ --
+ CREATE INDEX gin_relopts_test ON array_index_op_test USING gin (i)
+ WITH (FASTUPDATE=on, PENDING_LIST_CLEANUP_SIZE=32);
+ \d+ gin_relopts_test
+
+ --
-- HASH
--
CREATE INDEX hash_i4_index ON hash_i4_heap USING hash (random int4_ops);
(2014/09/09 22:17), Fujii Masao wrote:
On Tue, Sep 9, 2014 at 1:28 AM, Jeff Janes <jeff.janes@gmail.com> wrote:
I get some compiler warnings on v2 of this patch:
reloptions.c:219: warning: excess elements in struct initializer
reloptions.c:219: warning: (near initialization for 'intRelOpts[15]')
Attached is the updated version of the patch.
Thank you for updating the patch!
I took a quick review on the patch. It looks good to me, but one thing
I'm concerned about is
You wrote:
The attached patch introduces the GIN index storage parameter
"PENDING_LIST_CLEANUP_SIZE" which specifies the maximum size of
GIN pending list. If it's not set, work_mem is used as that
maximum size,
instead. So this patch doesn't break the existing application which
currently uses work_mem as the threshold of cleanup operation of
the pending list. It has only not to set PENDING_LIST_CLEANUP_SIZE.
As you mentioned, I think it's important to consider for the existing
applications, but I'm wondering if it would be a bit confusing users to
have two parameters, PENDING_LIST_CLEANUP_SIZE and work_mem, for this
setting. Wouldn't it be easy-to-use to have only one parameter,
PENDING_LIST_CLEANUP_SIZE? How about setting PENDING_LIST_CLEANUP_SIZE
to work_mem as the default value when running the CREATE INDEX command?
Sorry for the delay.
Best regards,
Etsuro Fujita
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Sep 10, 2014 at 12:15 PM, Etsuro Fujita
<fujita.etsuro@lab.ntt.co.jp> wrote:
(2014/09/09 22:17), Fujii Masao wrote:
On Tue, Sep 9, 2014 at 1:28 AM, Jeff Janes <jeff.janes@gmail.com> wrote:
I get some compiler warnings on v2 of this patch:
reloptions.c:219: warning: excess elements in struct initializer
reloptions.c:219: warning: (near initialization for 'intRelOpts[15]')Attached is the updated version of the patch.
Thank you for updating the patch!
I took a quick review on the patch. It looks good to me,
Thanks for reviewing the patch!
but one thing I'm
concerned about isYou wrote:
The attached patch introduces the GIN index storage parameter
"PENDING_LIST_CLEANUP_SIZE" which specifies the maximum size of
GIN pending list. If it's not set, work_mem is used as that maximum
size,
instead. So this patch doesn't break the existing application which
currently uses work_mem as the threshold of cleanup operation of
the pending list. It has only not to set PENDING_LIST_CLEANUP_SIZE.As you mentioned, I think it's important to consider for the existing
applications, but I'm wondering if it would be a bit confusing users to have
two parameters,
Yep.
PENDING_LIST_CLEANUP_SIZE and work_mem, for this setting.
Wouldn't it be easy-to-use to have only one parameter,
PENDING_LIST_CLEANUP_SIZE? How about setting PENDING_LIST_CLEANUP_SIZE to
work_mem as the default value when running the CREATE INDEX command?
That's an idea. But there might be some users who want to change
the cleanup size per session like they can do by setting work_mem,
and your idea would prevent them from doing that...
So what about introduing pending_list_cleanup_size also as GUC?
That is, users can set the threshold by using either the reloption or
GUC.
Sorry for the delay.
No problem. Thanks!
Regards,
--
Fujii Masao
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
(2014/09/10 12:31), Fujii Masao wrote:
On Wed, Sep 10, 2014 at 12:15 PM, Etsuro Fujita
<fujita.etsuro@lab.ntt.co.jp> wrote:(2014/09/09 22:17), Fujii Masao wrote:
Attached is the updated version of the patch.
I took a quick review on the patch. It looks good to me,
but one thing I'm
concerned about isYou wrote:
The attached patch introduces the GIN index storage parameter
"PENDING_LIST_CLEANUP_SIZE" which specifies the maximum size of
GIN pending list. If it's not set, work_mem is used as that maximum
size,
instead. So this patch doesn't break the existing application which
currently uses work_mem as the threshold of cleanup operation of
the pending list. It has only not to set PENDING_LIST_CLEANUP_SIZE.As you mentioned, I think it's important to consider for the existing
applications, but I'm wondering if it would be a bit confusing users to have
two parameters,Yep.
PENDING_LIST_CLEANUP_SIZE and work_mem, for this setting.
Wouldn't it be easy-to-use to have only one parameter,
PENDING_LIST_CLEANUP_SIZE? How about setting PENDING_LIST_CLEANUP_SIZE to
work_mem as the default value when running the CREATE INDEX command?That's an idea. But there might be some users who want to change
the cleanup size per session like they can do by setting work_mem,
and your idea would prevent them from doing that...
Why not use ALTER INDEX ... SET (PENDING_LIST_CLEANUP_SIZE= ...)? Maybe
I'm missing something, though.
So what about introduing pending_list_cleanup_size also as GUC?
That is, users can set the threshold by using either the reloption or
GUC.
Yeah, that's an idea. So, I'd like to hear the opinions of others.
Thanks,
Best regards,
Etsuro Fujita
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Sep 10, 2014 at 5:35 PM, Etsuro Fujita
<fujita.etsuro@lab.ntt.co.jp> wrote:
(2014/09/10 12:31), Fujii Masao wrote:
On Wed, Sep 10, 2014 at 12:15 PM, Etsuro Fujita
<fujita.etsuro@lab.ntt.co.jp> wrote:(2014/09/09 22:17), Fujii Masao wrote:
Attached is the updated version of the patch.
I took a quick review on the patch. It looks good to me,
but one thing I'm
concerned about isYou wrote:
The attached patch introduces the GIN index storage parameter
"PENDING_LIST_CLEANUP_SIZE" which specifies the maximum size of
GIN pending list. If it's not set, work_mem is used as that maximum
size,
instead. So this patch doesn't break the existing application which
currently uses work_mem as the threshold of cleanup operation of
the pending list. It has only not to set PENDING_LIST_CLEANUP_SIZE.As you mentioned, I think it's important to consider for the existing
applications, but I'm wondering if it would be a bit confusing users to
have
two parameters,Yep.
PENDING_LIST_CLEANUP_SIZE and work_mem, for this setting.
Wouldn't it be easy-to-use to have only one parameter,
PENDING_LIST_CLEANUP_SIZE? How about setting PENDING_LIST_CLEANUP_SIZE
to
work_mem as the default value when running the CREATE INDEX command?That's an idea. But there might be some users who want to change
the cleanup size per session like they can do by setting work_mem,
and your idea would prevent them from doing that...Why not use ALTER INDEX ... SET (PENDING_LIST_CLEANUP_SIZE= ...)? Maybe I'm
missing something, though.
It takes AccessExclusive lock and has an effect on every sessions
(not only specified session).
Regards,
--
Fujii Masao
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Fujii Masao wrote:
On Wed, Sep 10, 2014 at 12:15 PM, Etsuro Fujita
<fujita.etsuro@lab.ntt.co.jp> wrote:
PENDING_LIST_CLEANUP_SIZE and work_mem, for this setting.
Wouldn't it be easy-to-use to have only one parameter,
PENDING_LIST_CLEANUP_SIZE? How about setting PENDING_LIST_CLEANUP_SIZE to
work_mem as the default value when running the CREATE INDEX command?That's an idea. But there might be some users who want to change
the cleanup size per session like they can do by setting work_mem,
and your idea would prevent them from doing that...So what about introduing pending_list_cleanup_size also as GUC?
That is, users can set the threshold by using either the reloption or
GUC.
Yes, I think having both a GUC and a reloption makes sense -- the GUC
applies to all indexes, and can be tweaked for individual indexes using
the reloption.
I'm not sure about the idea of being able to change it per session,
though. Do you mean that you would like insert processes use a very
large value so that they can just append new values to the pending list,
and have vacuum use a small value so that it cleans up as soon as it
runs? Two things: 1. we could have an "autovacuum_" reloption which
only changes what autovacuum does; 2. we could have autovacuum run
index cleanup actions separately from actual vacuuming.
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Sep 10, 2014 at 10:37 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
Fujii Masao wrote:
On Wed, Sep 10, 2014 at 12:15 PM, Etsuro Fujita
<fujita.etsuro@lab.ntt.co.jp> wrote:PENDING_LIST_CLEANUP_SIZE and work_mem, for this setting.
Wouldn't it be easy-to-use to have only one parameter,
PENDING_LIST_CLEANUP_SIZE? How about setting PENDING_LIST_CLEANUP_SIZE to
work_mem as the default value when running the CREATE INDEX command?That's an idea. But there might be some users who want to change
the cleanup size per session like they can do by setting work_mem,
and your idea would prevent them from doing that...So what about introduing pending_list_cleanup_size also as GUC?
That is, users can set the threshold by using either the reloption or
GUC.Yes, I think having both a GUC and a reloption makes sense -- the GUC
applies to all indexes, and can be tweaked for individual indexes using
the reloption.
Agreed.
I'm not sure about the idea of being able to change it per session,
though. Do you mean that you would like insert processes use a very
large value so that they can just append new values to the pending list,
and have vacuum use a small value so that it cleans up as soon as it
runs? Two things: 1. we could have an "autovacuum_" reloption which
only changes what autovacuum does; 2. we could have autovacuum run
index cleanup actions separately from actual vacuuming.
Yes, I was thinking something like that. But if autovacuum
has already been able to handle that, it's nice. Anyway,
as you pointed out, it's better to have both GUC and reloption
for the cleanup size of pending list.
Regards,
--
Fujii Masao
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
(2014/09/13 2:42), Fujii Masao wrote:
On Wed, Sep 10, 2014 at 10:37 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:Fujii Masao wrote:
On Wed, Sep 10, 2014 at 12:15 PM, Etsuro Fujita
<fujita.etsuro@lab.ntt.co.jp> wrote:PENDING_LIST_CLEANUP_SIZE and work_mem, for this setting.
Wouldn't it be easy-to-use to have only one parameter,
PENDING_LIST_CLEANUP_SIZE? How about setting PENDING_LIST_CLEANUP_SIZE to
work_mem as the default value when running the CREATE INDEX command?That's an idea. But there might be some users who want to change
the cleanup size per session like they can do by setting work_mem,
and your idea would prevent them from doing that...So what about introduing pending_list_cleanup_size also as GUC?
That is, users can set the threshold by using either the reloption or
GUC.Yes, I think having both a GUC and a reloption makes sense -- the GUC
applies to all indexes, and can be tweaked for individual indexes using
the reloption.Agreed.
I'm not sure about the idea of being able to change it per session,
though. Do you mean that you would like insert processes use a very
large value so that they can just append new values to the pending list,
and have vacuum use a small value so that it cleans up as soon as it
runs? Two things: 1. we could have an "autovacuum_" reloption which
only changes what autovacuum does; 2. we could have autovacuum run
index cleanup actions separately from actual vacuuming.Yes, I was thinking something like that. But if autovacuum
has already been able to handle that, it's nice. Anyway,
as you pointed out, it's better to have both GUC and reloption
for the cleanup size of pending list.
OK, I'd vote for your idea of having both the GUC and the reloption.
So, I think the patch needs to be updated. Fujii-san, what plan do you
have about the patch?
Sorry for the delay.
Thanks,
Best regards,
Etsuro Fujita
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Sep 24, 2014 at 11:10 AM, Etsuro Fujita
<fujita.etsuro@lab.ntt.co.jp> wrote:
(2014/09/13 2:42), Fujii Masao wrote:
On Wed, Sep 10, 2014 at 10:37 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:Fujii Masao wrote:
On Wed, Sep 10, 2014 at 12:15 PM, Etsuro Fujita
<fujita.etsuro@lab.ntt.co.jp> wrote:PENDING_LIST_CLEANUP_SIZE and work_mem, for this setting.
Wouldn't it be easy-to-use to have only one parameter,
PENDING_LIST_CLEANUP_SIZE? How about setting PENDING_LIST_CLEANUP_SIZE
to
work_mem as the default value when running the CREATE INDEX command?That's an idea. But there might be some users who want to change
the cleanup size per session like they can do by setting work_mem,
and your idea would prevent them from doing that...So what about introduing pending_list_cleanup_size also as GUC?
That is, users can set the threshold by using either the reloption or
GUC.Yes, I think having both a GUC and a reloption makes sense -- the GUC
applies to all indexes, and can be tweaked for individual indexes using
the reloption.Agreed.
I'm not sure about the idea of being able to change it per session,
though. Do you mean that you would like insert processes use a very
large value so that they can just append new values to the pending list,
and have vacuum use a small value so that it cleans up as soon as it
runs? Two things: 1. we could have an "autovacuum_" reloption which
only changes what autovacuum does; 2. we could have autovacuum run
index cleanup actions separately from actual vacuuming.Yes, I was thinking something like that. But if autovacuum
has already been able to handle that, it's nice. Anyway,
as you pointed out, it's better to have both GUC and reloption
for the cleanup size of pending list.OK, I'd vote for your idea of having both the GUC and the reloption. So, I
think the patch needs to be updated. Fujii-san, what plan do you have about
the patch?
Please see the attached patch. In this patch, I introduced the GUC parameter,
pending_list_cleanup_size. I chose 4MB as the default value of the parameter.
But do you have any better idea about that default value?
BTW, I moved the CommitFest entry of this patch to next CF 2014-10.
Regards,
--
Fujii Masao
Attachments:
pending_list_cleanup_size_v4.patchtext/x-patch; charset=US-ASCII; name=pending_list_cleanup_size_v4.patchDownload
*** a/doc/src/sgml/config.sgml
--- b/doc/src/sgml/config.sgml
***************
*** 5907,5912 **** SET XML OPTION { DOCUMENT | CONTENT };
--- 5907,5933 ----
</listitem>
</varlistentry>
+ <varlistentry id="guc-pending-list-cleanup-size" xreflabel="pending_list_cleanup_size">
+ <term><varname>pending_list_cleanup_size</varname> (<type>integer</type>)
+ <indexterm>
+ <primary><varname>pending_list_cleanup_size</> configuration parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ Sets the maximum size of the GIN pending list which is used
+ when <literal>FASTUPDATE</> is enabled. If the list grows
+ larger than this maximum size, it is cleaned up by moving
+ the entries in it to the main GIN data structure in bulk.
+ The default is four megabytes (<literal>4MB</>). This setting
+ can be overridden for individual GIN indexes by changing
+ storage parameters.
+ See <xref linkend="gin-fast-update"> and <xref linkend="gin-tips">
+ for more information.
+ </para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
</sect2>
<sect2 id="runtime-config-client-format">
*** a/doc/src/sgml/gin.sgml
--- b/doc/src/sgml/gin.sgml
***************
*** 728,735 ****
from the indexed item). As of <productname>PostgreSQL</productname> 8.4,
<acronym>GIN</> is capable of postponing much of this work by inserting
new tuples into a temporary, unsorted list of pending entries.
! When the table is vacuumed, or if the pending list becomes too large
! (larger than <xref linkend="guc-work-mem">), the entries are moved to the
main <acronym>GIN</acronym> data structure using the same bulk insert
techniques used during initial index creation. This greatly improves
<acronym>GIN</acronym> index update speed, even counting the additional
--- 728,735 ----
from the indexed item). As of <productname>PostgreSQL</productname> 8.4,
<acronym>GIN</> is capable of postponing much of this work by inserting
new tuples into a temporary, unsorted list of pending entries.
! When the table is vacuumed, or if the pending list becomes larger than
! <xref linkend="guc-pending-list-cleanup-size">, the entries are moved to the
main <acronym>GIN</acronym> data structure using the same bulk insert
techniques used during initial index creation. This greatly improves
<acronym>GIN</acronym> index update speed, even counting the additional
***************
*** 812,829 ****
</varlistentry>
<varlistentry>
! <term><xref linkend="guc-work-mem"></term>
<listitem>
<para>
During a series of insertions into an existing <acronym>GIN</acronym>
index that has <literal>FASTUPDATE</> enabled, the system will clean up
the pending-entry list whenever the list grows larger than
! <varname>work_mem</>. To avoid fluctuations in observed response time,
! it's desirable to have pending-list cleanup occur in the background
! (i.e., via autovacuum). Foreground cleanup operations can be avoided by
! increasing <varname>work_mem</> or making autovacuum more aggressive.
! However, enlarging <varname>work_mem</> means that if a foreground
! cleanup does occur, it will take even longer.
</para>
</listitem>
</varlistentry>
--- 812,837 ----
</varlistentry>
<varlistentry>
! <term><xref linkend="guc-pending-list-cleanup-size"></term>
<listitem>
<para>
During a series of insertions into an existing <acronym>GIN</acronym>
index that has <literal>FASTUPDATE</> enabled, the system will clean up
the pending-entry list whenever the list grows larger than
! <literal>pending_list_cleanup_size</>. To avoid fluctuations in observed
! response time, it's desirable to have pending-list cleanup occur in the
! background (i.e., via autovacuum). Foreground cleanup operations
! can be avoided by increasing <literal>pending_list_cleanup_size</>
! or making autovacuum more aggressive.
! However, enlarging the threshold of the cleanup operation means that
! if a foreground cleanup does occur, it will take even longer.
! </para>
! <para>
! <literal>pending_list_cleanup_size</> can be overridden for individual
! GIN indexes by changing storage parameters, and which allows each
! GIN index to have its own cleanup threshold.
! For example, it's possible to increase the threshold only for the GIN
! index which can be updated heavily, and decrease it otherwise.
</para>
</listitem>
</varlistentry>
*** a/doc/src/sgml/ref/create_index.sgml
--- b/doc/src/sgml/ref/create_index.sgml
***************
*** 356,361 **** CREATE [ UNIQUE ] INDEX [ CONCURRENTLY ] [ <replaceable class="parameter">name</
--- 356,372 ----
</listitem>
</varlistentry>
</variablelist>
+ <variablelist>
+ <varlistentry>
+ <term><literal>PENDING_LIST_CLEANUP_SIZE</></term>
+ <listitem>
+ <para>
+ Custom <xref linkend="guc-pending-list-cleanup-size"> parameter.
+ This value is specified in kilobytes.
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
</refsect2>
<refsect2 id="SQL-CREATEINDEX-CONCURRENTLY">
*** a/src/backend/access/common/reloptions.c
--- b/src/backend/access/common/reloptions.c
***************
*** 209,214 **** static relopt_int intRelOpts[] =
--- 209,222 ----
RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
}, -1, 0, 2000000000
},
+ {
+ {
+ "pending_list_cleanup_size",
+ "Maximum size of the pending list for this GIN index, in kilobytes.",
+ RELOPT_KIND_GIN
+ },
+ -1, 0, MAX_KILOBYTES
+ },
/* list terminator */
{{NULL}}
*** a/src/backend/access/gin/ginfast.c
--- b/src/backend/access/gin/ginfast.c
***************
*** 24,29 ****
--- 24,31 ----
#include "utils/memutils.h"
#include "utils/rel.h"
+ /* GUC parameter */
+ int pending_list_cleanup_size = 0;
#define GIN_PAGE_FREESIZE \
( BLCKSZ - MAXALIGN(SizeOfPageHeaderData) - MAXALIGN(sizeof(GinPageOpaqueData)) )
***************
*** 227,232 **** ginHeapTupleFastInsert(GinState *ginstate, GinTupleCollector *collector)
--- 229,235 ----
ginxlogUpdateMeta data;
bool separateList = false;
bool needCleanup = false;
+ int cleanupSize;
if (collector->ntuples == 0)
return;
***************
*** 421,431 **** ginHeapTupleFastInsert(GinState *ginstate, GinTupleCollector *collector)
* ginInsertCleanup could take significant amount of time, so we prefer to
* call it when it can do all the work in a single collection cycle. In
* non-vacuum mode, it shouldn't require maintenance_work_mem, so fire it
! * while pending list is still small enough to fit into work_mem.
*
* ginInsertCleanup() should not be called inside our CRIT_SECTION.
*/
! if (metadata->nPendingPages * GIN_PAGE_FREESIZE > work_mem * 1024L)
needCleanup = true;
UnlockReleaseBuffer(metabuffer);
--- 424,436 ----
* ginInsertCleanup could take significant amount of time, so we prefer to
* call it when it can do all the work in a single collection cycle. In
* non-vacuum mode, it shouldn't require maintenance_work_mem, so fire it
! * while pending list is still small enough to fit into
! * pending_list_cleanup_size.
*
* ginInsertCleanup() should not be called inside our CRIT_SECTION.
*/
! cleanupSize = GinGetPendingListCleanupSize(index);
! if (metadata->nPendingPages * GIN_PAGE_FREESIZE > cleanupSize * 1024L)
needCleanup = true;
UnlockReleaseBuffer(metabuffer);
*** a/src/backend/access/gin/ginutil.c
--- b/src/backend/access/gin/ginutil.c
***************
*** 524,530 **** ginoptions(PG_FUNCTION_ARGS)
GinOptions *rdopts;
int numoptions;
static const relopt_parse_elt tab[] = {
! {"fastupdate", RELOPT_TYPE_BOOL, offsetof(GinOptions, useFastUpdate)}
};
options = parseRelOptions(reloptions, validate, RELOPT_KIND_GIN,
--- 524,532 ----
GinOptions *rdopts;
int numoptions;
static const relopt_parse_elt tab[] = {
! {"fastupdate", RELOPT_TYPE_BOOL, offsetof(GinOptions, useFastUpdate)},
! {"pending_list_cleanup_size", RELOPT_TYPE_INT, offsetof(GinOptions,
! pendingListCleanupSize)}
};
options = parseRelOptions(reloptions, validate, RELOPT_KIND_GIN,
*** a/src/backend/utils/misc/guc.c
--- b/src/backend/utils/misc/guc.c
***************
*** 96,109 ****
#define CONFIG_EXEC_PARAMS_NEW "global/config_exec_params.new"
#endif
- /* upper limit for GUC variables measured in kilobytes of memory */
- /* note that various places assume the byte size fits in a "long" variable */
- #if SIZEOF_SIZE_T > 4 && SIZEOF_LONG > 4
- #define MAX_KILOBYTES INT_MAX
- #else
- #define MAX_KILOBYTES (INT_MAX / 1024)
- #endif
-
#define KB_PER_MB (1024)
#define KB_PER_GB (1024*1024)
#define KB_PER_TB (1024*1024*1024)
--- 96,101 ----
***************
*** 2567,2572 **** static struct config_int ConfigureNamesInt[] =
--- 2559,2575 ----
NULL, NULL, NULL
},
+ {
+ {"pending_list_cleanup_size", PGC_USERSET, CLIENT_CONN_STATEMENT,
+ gettext_noop("Sets the maximum size of the pending list for GIN index."),
+ NULL,
+ GUC_UNIT_KB
+ },
+ &pending_list_cleanup_size,
+ 4096, 0, MAX_KILOBYTES,
+ 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
***************
*** 519,524 ****
--- 519,525 ----
#bytea_output = 'hex' # hex, escape
#xmlbinary = 'base64'
#xmloption = 'content'
+ #pending_list_cleanup_size = 4MB
# - Locale and Formatting -
*** a/src/bin/psql/tab-complete.c
--- b/src/bin/psql/tab-complete.c
***************
*** 1172,1178 **** psql_completion(const char *text, int start, int end)
pg_strcasecmp(prev_wd, "(") == 0)
{
static const char *const list_INDEXOPTIONS[] =
! {"fillfactor", "fastupdate", NULL};
COMPLETE_WITH_LIST(list_INDEXOPTIONS);
}
--- 1172,1178 ----
pg_strcasecmp(prev_wd, "(") == 0)
{
static const char *const list_INDEXOPTIONS[] =
! {"fillfactor", "fastupdate", "pending_list_cleanup_size", NULL};
COMPLETE_WITH_LIST(list_INDEXOPTIONS);
}
*** a/src/include/access/gin.h
--- b/src/include/access/gin.h
***************
*** 66,71 **** typedef char GinTernaryValue;
--- 66,72 ----
/* GUC parameter */
extern PGDLLIMPORT int GinFuzzySearchLimit;
+ extern int pending_list_cleanup_size;
/* ginutil.c */
extern void ginGetStats(Relation index, GinStatsData *stats);
*** a/src/include/access/gin_private.h
--- b/src/include/access/gin_private.h
***************
*** 314,325 **** typedef struct GinOptions
--- 314,331 ----
{
int32 vl_len_; /* varlena header (do not touch directly!) */
bool useFastUpdate; /* use fast updates? */
+ int pendingListCleanupSize; /* maximum size of pending list */
} GinOptions;
#define GIN_DEFAULT_USE_FASTUPDATE true
#define GinGetUseFastUpdate(relation) \
((relation)->rd_options ? \
((GinOptions *) (relation)->rd_options)->useFastUpdate : GIN_DEFAULT_USE_FASTUPDATE)
+ #define GinGetPendingListCleanupSize(relation) \
+ ((relation)->rd_options && \
+ ((GinOptions *) (relation)->rd_options)->pendingListCleanupSize != -1 ? \
+ ((GinOptions *) (relation)->rd_options)->pendingListCleanupSize : \
+ pending_list_cleanup_size)
/* Macros for buffer lock/unlock operations */
*** a/src/include/utils/guc.h
--- b/src/include/utils/guc.h
***************
*** 18,23 ****
--- 18,31 ----
#include "utils/array.h"
+ /* upper limit for GUC variables measured in kilobytes of memory */
+ /* note that various places assume the byte size fits in a "long" variable */
+ #if SIZEOF_SIZE_T > 4 && SIZEOF_LONG > 4
+ #define MAX_KILOBYTES INT_MAX
+ #else
+ #define MAX_KILOBYTES (INT_MAX / 1024)
+ #endif
+
/*
* Certain options can only be set at certain times. The rules are
* like this:
*** a/src/test/regress/expected/create_index.out
--- b/src/test/regress/expected/create_index.out
***************
*** 2235,2240 **** SELECT COUNT(*) FROM array_gin_test WHERE a @> '{2}';
--- 2235,2253 ----
DROP TABLE array_gin_test;
--
+ -- Test GIN index's reloptions
+ --
+ CREATE INDEX gin_relopts_test ON array_index_op_test USING gin (i)
+ WITH (FASTUPDATE=on, PENDING_LIST_CLEANUP_SIZE=32);
+ \d+ gin_relopts_test
+ Index "public.gin_relopts_test"
+ Column | Type | Definition | Storage
+ --------+---------+------------+---------
+ i | integer | i | plain
+ gin, for table "public.array_index_op_test"
+ Options: fastupdate=on, pending_list_cleanup_size=32
+
+ --
-- HASH
--
CREATE INDEX hash_i4_index ON hash_i4_heap USING hash (random int4_ops);
*** a/src/test/regress/sql/create_index.sql
--- b/src/test/regress/sql/create_index.sql
***************
*** 651,656 **** SELECT COUNT(*) FROM array_gin_test WHERE a @> '{2}';
--- 651,663 ----
DROP TABLE array_gin_test;
--
+ -- Test GIN index's reloptions
+ --
+ CREATE INDEX gin_relopts_test ON array_index_op_test USING gin (i)
+ WITH (FASTUPDATE=on, PENDING_LIST_CLEANUP_SIZE=32);
+ \d+ gin_relopts_test
+
+ --
-- HASH
--
CREATE INDEX hash_i4_index ON hash_i4_heap USING hash (random int4_ops);
(2014/10/08 22:51), Fujii Masao wrote:
On Wed, Sep 24, 2014 at 11:10 AM, Etsuro Fujita
<fujita.etsuro@lab.ntt.co.jp> wrote:
On Wed, Sep 10, 2014 at 10:37 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:Fujii Masao wrote:
On Wed, Sep 10, 2014 at 12:15 PM, Etsuro Fujita
<fujita.etsuro@lab.ntt.co.jp> wrote:
PENDING_LIST_CLEANUP_SIZE and work_mem, for this setting.
Wouldn't it be easy-to-use to have only one parameter,
PENDING_LIST_CLEANUP_SIZE? How about setting PENDING_LIST_CLEANUP_SIZE
to
work_mem as the default value when running the CREATE INDEX command?
So what about introduing pending_list_cleanup_size also as GUC?
That is, users can set the threshold by using either the reloption or
GUC.
Yes, I think having both a GUC and a reloption makes sense -- the GUC
applies to all indexes, and can be tweaked for individual indexes using
the reloption.
OK, I'd vote for your idea of having both the GUC and the reloption. So, I
think the patch needs to be updated. Fujii-san, what plan do you have about
the patch?
Please see the attached patch. In this patch, I introduced the GUC parameter,
pending_list_cleanup_size. I chose 4MB as the default value of the parameter.
But do you have any better idea about that default value?
It seems reasonable to me that the GUC has the same default value as
work_mem. So, +1 for the default value of 4MB.
BTW, I moved the CommitFest entry of this patch to next CF 2014-10.
OK, I'll review the patch in the CF.
Thanks,
Best regards,
Etsuro Fujita
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
(2014/10/09 11:49), Etsuro Fujita wrote:
(2014/10/08 22:51), Fujii Masao wrote:
On Wed, Sep 24, 2014 at 11:10 AM, Etsuro Fujita
<fujita.etsuro@lab.ntt.co.jp> wrote:On Wed, Sep 10, 2014 at 10:37 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:Fujii Masao wrote:
On Wed, Sep 10, 2014 at 12:15 PM, Etsuro Fujita
<fujita.etsuro@lab.ntt.co.jp> wrote:
PENDING_LIST_CLEANUP_SIZE and work_mem, for this setting.
Wouldn't it be easy-to-use to have only one parameter,
PENDING_LIST_CLEANUP_SIZE? How about setting
PENDING_LIST_CLEANUP_SIZE
to
work_mem as the default value when running the CREATE INDEX command?
So what about introduing pending_list_cleanup_size also as GUC?
That is, users can set the threshold by using either the reloption or
GUC.
Yes, I think having both a GUC and a reloption makes sense -- the GUC
applies to all indexes, and can be tweaked for individual indexes
using
the reloption.
OK, I'd vote for your idea of having both the GUC and the reloption.
So, I
think the patch needs to be updated. Fujii-san, what plan do you
have about
the patch?
Please see the attached patch. In this patch, I introduced the GUC
parameter,
pending_list_cleanup_size. I chose 4MB as the default value of the
parameter.
But do you have any better idea about that default value?
It seems reasonable to me that the GUC has the same default value as
work_mem. So, +1 for the default value of 4MB.
BTW, I moved the CommitFest entry of this patch to next CF 2014-10.
OK, I'll review the patch in the CF.
Thank you for updating the patch! Here are my review comments.
* The patch applies cleanly and make and make check run successfully. I
think that the patch is mostly good.
* In src/backend/utils/misc/guc.c:
+ {
+ {"pending_list_cleanup_size", PGC_USERSET, CLIENT_CONN_STATEMENT,
+ gettext_noop("Sets the maximum size of the pending list for GIN
index."),
+ NULL,
+ GUC_UNIT_KB
+ },
+ &pending_list_cleanup_size,
+ 4096, 0, MAX_KILOBYTES,
+ NULL, NULL, NULL
+ },
ISTM it'd be better to use RESOURCES_MEM, not CLIENT_CONN_STATEMENT. No?
Also why not set min to 64, not to 0, in accoradance with that of
work_mem?
* In src/backend/utils/misc/postgresql.conf.sample:
Likewise, why not put this variable in the section of RESOURCE USAGE,
not in that of CLIENT CONNECTION DEFAULTS.
* In src/backend/access/common/reloptions.c:
+ {
+ {
+ "pending_list_cleanup_size",
+ "Maximum size of the pending list for this GIN index, in kilobytes.",
+ RELOPT_KIND_GIN
+ },
+ -1, 0, MAX_KILOBYTES
+ },
As in guc.c, why not set min to 64, not to 0?
* In src/include/access/gin.h:
/* GUC parameter */
extern PGDLLIMPORT int GinFuzzySearchLimit;
+ extern int pending_list_cleanup_size;
The comment should be "GUC parameters".
* In src/backend/access/gin/ginfast.c:
+ /* GUC parameter */
+ int pending_list_cleanup_size = 0;
Do we need to substitute 0 for pending_list_cleanup_size?
* In doc/src/sgml/config.sgml:
+ <varlistentry id="guc-pending-list-cleanup-size"
xreflabel="pending_list_cleanup_size">
+ <term><varname>pending_list_cleanup_size</varname>
(<type>integer</type>)
As in postgresql.conf.sample, ISTM it'd be better to explain this
variable in the section of Resource Consumption (maybe in "Memory"), not
in that of Client Connection Defaults.
* In doc/src/sgml/gin.sgml:
! <literal>pending_list_cleanup_size</>. To avoid fluctuations in
observed
ISTM it'd be better to use <varname> for pending_list_cleanup_size, not
<literal>, here.
* In doc/src/sgml/ref/create_index.sgml:
+ <term><literal>PENDING_LIST_CLEANUP_SIZE</></term>
IMHO, it seems to me better for this variable to be in lowercase in
accordance with the GUC version. Also, I think that the words "GIN
indexes accept a different parameter:" in the section of "Index Storage
Parameters" in this reference page would be "GIN indexes accept
different parameters:".
Sorry for the delay in reviewing the patch.
Best regards,
Etsuro Fujita
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Oct 30, 2014 at 7:30 PM, Etsuro Fujita
<fujita.etsuro@lab.ntt.co.jp> wrote:
(2014/10/09 11:49), Etsuro Fujita wrote:
(2014/10/08 22:51), Fujii Masao wrote:
On Wed, Sep 24, 2014 at 11:10 AM, Etsuro Fujita
<fujita.etsuro@lab.ntt.co.jp> wrote:On Wed, Sep 10, 2014 at 10:37 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:Fujii Masao wrote:
On Wed, Sep 10, 2014 at 12:15 PM, Etsuro Fujita
<fujita.etsuro@lab.ntt.co.jp> wrote:PENDING_LIST_CLEANUP_SIZE and work_mem, for this setting.
Wouldn't it be easy-to-use to have only one parameter,
PENDING_LIST_CLEANUP_SIZE? How about setting
PENDING_LIST_CLEANUP_SIZE
to
work_mem as the default value when running the CREATE INDEX command?So what about introduing pending_list_cleanup_size also as GUC?
That is, users can set the threshold by using either the reloption or
GUC.Yes, I think having both a GUC and a reloption makes sense -- the GUC
applies to all indexes, and can be tweaked for individual indexes
using
the reloption.OK, I'd vote for your idea of having both the GUC and the reloption.
So, I
think the patch needs to be updated. Fujii-san, what plan do you
have about
the patch?Please see the attached patch. In this patch, I introduced the GUC
parameter,
pending_list_cleanup_size. I chose 4MB as the default value of the
parameter.
But do you have any better idea about that default value?It seems reasonable to me that the GUC has the same default value as
work_mem. So, +1 for the default value of 4MB.BTW, I moved the CommitFest entry of this patch to next CF 2014-10.
OK, I'll review the patch in the CF.
Thank you for updating the patch! Here are my review comments.
* The patch applies cleanly and make and make check run successfully. I
think that the patch is mostly good.
Thanks! Attached is the updated version of the patch.
* In src/backend/utils/misc/guc.c: + { + {"pending_list_cleanup_size", PGC_USERSET, CLIENT_CONN_STATEMENT, + gettext_noop("Sets the maximum size of the pending list for GIN index."), + NULL, + GUC_UNIT_KB + }, + &pending_list_cleanup_size, + 4096, 0, MAX_KILOBYTES, + NULL, NULL, NULL + },ISTM it'd be better to use RESOURCES_MEM, not CLIENT_CONN_STATEMENT. No?
Yes if the pending list always exists in the memory. But not, IIUC. Thought?
Also why not set min to 64, not to 0, in accoradance with that of work_mem?
I'm OK to use 64. But I just chose 0 because I could not think of any reasonable
reason why 64k is suitable as the minimum size of the pending list.
IOW, I have no idea about whether it's reasonable to use the min value of
work_mem as the min size of the pending list.
* In src/backend/utils/misc/postgresql.conf.sample:
Likewise, why not put this variable in the section of RESOURCE USAGE, not in
that of CLIENT CONNECTION DEFAULTS.
Same as above.
* In src/backend/access/common/reloptions.c: + { + { + "pending_list_cleanup_size", + "Maximum size of the pending list for this GIN index, in kilobytes.", + RELOPT_KIND_GIN + }, + -1, 0, MAX_KILOBYTES + },As in guc.c, why not set min to 64, not to 0?
Same as above.
* In src/include/access/gin.h:
/* GUC parameter */
extern PGDLLIMPORT int GinFuzzySearchLimit;
+ extern int pending_list_cleanup_size;The comment should be "GUC parameters".
Yes, fixed.
* In src/backend/access/gin/ginfast.c: + /* GUC parameter */ + int pending_list_cleanup_size = 0;Do we need to substitute 0 for pending_list_cleanup_size?
Same as above.
* In doc/src/sgml/config.sgml: + <varlistentry id="guc-pending-list-cleanup-size" xreflabel="pending_list_cleanup_size"> + <term><varname>pending_list_cleanup_size</varname> (<type>integer</type>)As in postgresql.conf.sample, ISTM it'd be better to explain this variable
in the section of Resource Consumption (maybe in "Memory"), not in that of
Client Connection Defaults.
Same as above.
* In doc/src/sgml/gin.sgml:
! <literal>pending_list_cleanup_size</>. To avoid fluctuations in
observedISTM it'd be better to use <varname> for pending_list_cleanup_size, not
<literal>, here.
Yes, fixed.
* In doc/src/sgml/ref/create_index.sgml:
+ <term><literal>PENDING_LIST_CLEANUP_SIZE</></term>IMHO, it seems to me better for this variable to be in lowercase in
accordance with the GUC version.
Using lowercase only for pending_list_cleanup_size and uppercase for
other options
looks strange to me. What about using lowercase for all the storage options?
I changed the document in that way.
Also, I think that the words "GIN indexes
accept a different parameter:" in the section of "Index Storage Parameters"
in this reference page would be "GIN indexes accept different parameters:".
Yes, fixed.
Sorry for the delay in reviewing the patch.
No problem. Thanks!
Regards,
--
Fujii Masao
Attachments:
pending_list_cleanup_size_v5.patchtext/x-patch; charset=US-ASCII; name=pending_list_cleanup_size_v5.patchDownload
*** a/doc/src/sgml/config.sgml
--- b/doc/src/sgml/config.sgml
***************
*** 5911,5916 **** SET XML OPTION { DOCUMENT | CONTENT };
--- 5911,5937 ----
</listitem>
</varlistentry>
+ <varlistentry id="guc-pending-list-cleanup-size" xreflabel="pending_list_cleanup_size">
+ <term><varname>pending_list_cleanup_size</varname> (<type>integer</type>)
+ <indexterm>
+ <primary><varname>pending_list_cleanup_size</> configuration parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ Sets the maximum size of the GIN pending list which is used
+ when <literal>fastupdate</> is enabled. If the list grows
+ larger than this maximum size, it is cleaned up by moving
+ the entries in it to the main GIN data structure in bulk.
+ The default is four megabytes (<literal>4MB</>). This setting
+ can be overridden for individual GIN indexes by changing
+ storage parameters.
+ See <xref linkend="gin-fast-update"> and <xref linkend="gin-tips">
+ for more information.
+ </para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
</sect2>
<sect2 id="runtime-config-client-format">
*** a/doc/src/sgml/gin.sgml
--- b/doc/src/sgml/gin.sgml
***************
*** 728,735 ****
from the indexed item). As of <productname>PostgreSQL</productname> 8.4,
<acronym>GIN</> is capable of postponing much of this work by inserting
new tuples into a temporary, unsorted list of pending entries.
! When the table is vacuumed, or if the pending list becomes too large
! (larger than <xref linkend="guc-work-mem">), the entries are moved to the
main <acronym>GIN</acronym> data structure using the same bulk insert
techniques used during initial index creation. This greatly improves
<acronym>GIN</acronym> index update speed, even counting the additional
--- 728,735 ----
from the indexed item). As of <productname>PostgreSQL</productname> 8.4,
<acronym>GIN</> is capable of postponing much of this work by inserting
new tuples into a temporary, unsorted list of pending entries.
! When the table is vacuumed, or if the pending list becomes larger than
! <xref linkend="guc-pending-list-cleanup-size">, the entries are moved to the
main <acronym>GIN</acronym> data structure using the same bulk insert
techniques used during initial index creation. This greatly improves
<acronym>GIN</acronym> index update speed, even counting the additional
***************
*** 750,756 ****
<para>
If consistent response time is more important than update speed,
use of pending entries can be disabled by turning off the
! <literal>FASTUPDATE</literal> storage parameter for a
<acronym>GIN</acronym> index. See <xref linkend="sql-createindex">
for details.
</para>
--- 750,756 ----
<para>
If consistent response time is more important than update speed,
use of pending entries can be disabled by turning off the
! <literal>fastupdate</literal> storage parameter for a
<acronym>GIN</acronym> index. See <xref linkend="sql-createindex">
for details.
</para>
***************
*** 812,829 ****
</varlistentry>
<varlistentry>
! <term><xref linkend="guc-work-mem"></term>
<listitem>
<para>
During a series of insertions into an existing <acronym>GIN</acronym>
! index that has <literal>FASTUPDATE</> enabled, the system will clean up
the pending-entry list whenever the list grows larger than
! <varname>work_mem</>. To avoid fluctuations in observed response time,
! it's desirable to have pending-list cleanup occur in the background
! (i.e., via autovacuum). Foreground cleanup operations can be avoided by
! increasing <varname>work_mem</> or making autovacuum more aggressive.
! However, enlarging <varname>work_mem</> means that if a foreground
! cleanup does occur, it will take even longer.
</para>
</listitem>
</varlistentry>
--- 812,837 ----
</varlistentry>
<varlistentry>
! <term><xref linkend="guc-pending-list-cleanup-size"></term>
<listitem>
<para>
During a series of insertions into an existing <acronym>GIN</acronym>
! index that has <literal>fastupdate</> enabled, the system will clean up
the pending-entry list whenever the list grows larger than
! <varname>pending_list_cleanup_size</>. To avoid fluctuations in observed
! response time, it's desirable to have pending-list cleanup occur in the
! background (i.e., via autovacuum). Foreground cleanup operations
! can be avoided by increasing <varname>pending_list_cleanup_size</>
! or making autovacuum more aggressive.
! However, enlarging the threshold of the cleanup operation means that
! if a foreground cleanup does occur, it will take even longer.
! </para>
! <para>
! <varname>pending_list_cleanup_size</> can be overridden for individual
! GIN indexes by changing storage parameters, and which allows each
! GIN index to have its own cleanup threshold.
! For example, it's possible to increase the threshold only for the GIN
! index which can be updated heavily, and decrease it otherwise.
</para>
</listitem>
</varlistentry>
*** a/doc/src/sgml/gist.sgml
--- b/doc/src/sgml/gist.sgml
***************
*** 861,867 **** my_distance(PG_FUNCTION_ARGS)
<para>
By default, a GiST index build switches to the buffering method when the
index size reaches <xref linkend="guc-effective-cache-size">. It can
! be manually turned on or off by the <literal>BUFFERING</literal> parameter
to the CREATE INDEX command. The default behavior is good for most cases,
but turning buffering off might speed up the build somewhat if the input
data is ordered.
--- 861,867 ----
<para>
By default, a GiST index build switches to the buffering method when the
index size reaches <xref linkend="guc-effective-cache-size">. It can
! be manually turned on or off by the <literal>buffering</literal> parameter
to the CREATE INDEX command. The default behavior is good for most cases,
but turning buffering off might speed up the build somewhat if the input
data is ordered.
*** a/doc/src/sgml/ref/cluster.sgml
--- b/doc/src/sgml/ref/cluster.sgml
***************
*** 46,52 **** CLUSTER [VERBOSE]
not clustered. That is, no attempt is made to store new or
updated rows according to their index order. (If one wishes, one can
periodically recluster by issuing the command again. Also, setting
! the table's <literal>FILLFACTOR</literal> storage parameter to less than
100% can aid in preserving cluster ordering during updates, since updated
rows are kept on the same page if enough space is available there.)
</para>
--- 46,52 ----
not clustered. That is, no attempt is made to store new or
updated rows according to their index order. (If one wishes, one can
periodically recluster by issuing the command again. Also, setting
! the table's <literal>fillfactor</literal> storage parameter to less than
100% can aid in preserving cluster ordering during updates, since updated
rows are kept on the same page if enough space is available there.)
</para>
*** a/doc/src/sgml/ref/create_index.sgml
--- b/doc/src/sgml/ref/create_index.sgml
***************
*** 287,293 **** CREATE [ UNIQUE ] INDEX [ CONCURRENTLY ] [ <replaceable class="parameter">name</
<variablelist>
<varlistentry>
! <term><literal>FILLFACTOR</></term>
<listitem>
<para>
The fillfactor for an index is a percentage that determines how full
--- 287,293 ----
<variablelist>
<varlistentry>
! <term><literal>fillfactor</></term>
<listitem>
<para>
The fillfactor for an index is a percentage that determines how full
***************
*** 314,320 **** CREATE [ UNIQUE ] INDEX [ CONCURRENTLY ] [ <replaceable class="parameter">name</
<variablelist>
<varlistentry>
! <term><literal>BUFFERING</></term>
<listitem>
<para>
Determines whether the buffering build technique described in
--- 314,320 ----
<variablelist>
<varlistentry>
! <term><literal>buffering</></term>
<listitem>
<para>
Determines whether the buffering build technique described in
***************
*** 328,339 **** CREATE [ UNIQUE ] INDEX [ CONCURRENTLY ] [ <replaceable class="parameter">name</
</variablelist>
<para>
! GIN indexes accept a different parameter:
</para>
<variablelist>
<varlistentry>
! <term><literal>FASTUPDATE</></term>
<listitem>
<para>
This setting controls usage of the fast update technique described in
--- 328,339 ----
</variablelist>
<para>
! GIN indexes accept different parameters:
</para>
<variablelist>
<varlistentry>
! <term><literal>fastupdate</></term>
<listitem>
<para>
This setting controls usage of the fast update technique described in
***************
*** 346,352 **** CREATE [ UNIQUE ] INDEX [ CONCURRENTLY ] [ <replaceable class="parameter">name</
<note>
<para>
! Turning <literal>FASTUPDATE</> off via <command>ALTER INDEX</> prevents
future insertions from going into the list of pending index entries,
but does not in itself flush previous entries. You might want to
<command>VACUUM</> the table afterward to ensure the pending list is
--- 346,352 ----
<note>
<para>
! Turning <literal>fastupdate</> off via <command>ALTER INDEX</> prevents
future insertions from going into the list of pending index entries,
but does not in itself flush previous entries. You might want to
<command>VACUUM</> the table afterward to ensure the pending list is
***************
*** 356,361 **** CREATE [ UNIQUE ] INDEX [ CONCURRENTLY ] [ <replaceable class="parameter">name</
--- 356,372 ----
</listitem>
</varlistentry>
</variablelist>
+ <variablelist>
+ <varlistentry>
+ <term><literal>PENDING_LIST_CLEANUP_SIZE</></term>
+ <listitem>
+ <para>
+ Custom <xref linkend="guc-pending-list-cleanup-size"> parameter.
+ This value is specified in kilobytes.
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
</refsect2>
<refsect2 id="SQL-CREATEINDEX-CONCURRENTLY">
*** a/src/backend/access/common/reloptions.c
--- b/src/backend/access/common/reloptions.c
***************
*** 209,214 **** static relopt_int intRelOpts[] =
--- 209,222 ----
RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
}, -1, 0, 2000000000
},
+ {
+ {
+ "pending_list_cleanup_size",
+ "Maximum size of the pending list for this GIN index, in kilobytes.",
+ RELOPT_KIND_GIN
+ },
+ -1, 0, MAX_KILOBYTES
+ },
/* list terminator */
{{NULL}}
*** a/src/backend/access/gin/ginfast.c
--- b/src/backend/access/gin/ginfast.c
***************
*** 24,29 ****
--- 24,31 ----
#include "utils/memutils.h"
#include "utils/rel.h"
+ /* GUC parameter */
+ int pending_list_cleanup_size = 0;
#define GIN_PAGE_FREESIZE \
( BLCKSZ - MAXALIGN(SizeOfPageHeaderData) - MAXALIGN(sizeof(GinPageOpaqueData)) )
***************
*** 227,232 **** ginHeapTupleFastInsert(GinState *ginstate, GinTupleCollector *collector)
--- 229,235 ----
ginxlogUpdateMeta data;
bool separateList = false;
bool needCleanup = false;
+ int cleanupSize;
if (collector->ntuples == 0)
return;
***************
*** 421,431 **** ginHeapTupleFastInsert(GinState *ginstate, GinTupleCollector *collector)
* ginInsertCleanup could take significant amount of time, so we prefer to
* call it when it can do all the work in a single collection cycle. In
* non-vacuum mode, it shouldn't require maintenance_work_mem, so fire it
! * while pending list is still small enough to fit into work_mem.
*
* ginInsertCleanup() should not be called inside our CRIT_SECTION.
*/
! if (metadata->nPendingPages * GIN_PAGE_FREESIZE > work_mem * 1024L)
needCleanup = true;
UnlockReleaseBuffer(metabuffer);
--- 424,436 ----
* ginInsertCleanup could take significant amount of time, so we prefer to
* call it when it can do all the work in a single collection cycle. In
* non-vacuum mode, it shouldn't require maintenance_work_mem, so fire it
! * while pending list is still small enough to fit into
! * pending_list_cleanup_size.
*
* ginInsertCleanup() should not be called inside our CRIT_SECTION.
*/
! cleanupSize = GinGetPendingListCleanupSize(index);
! if (metadata->nPendingPages * GIN_PAGE_FREESIZE > cleanupSize * 1024L)
needCleanup = true;
UnlockReleaseBuffer(metabuffer);
*** a/src/backend/access/gin/ginutil.c
--- b/src/backend/access/gin/ginutil.c
***************
*** 524,530 **** ginoptions(PG_FUNCTION_ARGS)
GinOptions *rdopts;
int numoptions;
static const relopt_parse_elt tab[] = {
! {"fastupdate", RELOPT_TYPE_BOOL, offsetof(GinOptions, useFastUpdate)}
};
options = parseRelOptions(reloptions, validate, RELOPT_KIND_GIN,
--- 524,532 ----
GinOptions *rdopts;
int numoptions;
static const relopt_parse_elt tab[] = {
! {"fastupdate", RELOPT_TYPE_BOOL, offsetof(GinOptions, useFastUpdate)},
! {"pending_list_cleanup_size", RELOPT_TYPE_INT, offsetof(GinOptions,
! pendingListCleanupSize)}
};
options = parseRelOptions(reloptions, validate, RELOPT_KIND_GIN,
*** a/src/backend/utils/misc/guc.c
--- b/src/backend/utils/misc/guc.c
***************
*** 96,109 ****
#define CONFIG_EXEC_PARAMS_NEW "global/config_exec_params.new"
#endif
- /* upper limit for GUC variables measured in kilobytes of memory */
- /* note that various places assume the byte size fits in a "long" variable */
- #if SIZEOF_SIZE_T > 4 && SIZEOF_LONG > 4
- #define MAX_KILOBYTES INT_MAX
- #else
- #define MAX_KILOBYTES (INT_MAX / 1024)
- #endif
-
#define KB_PER_MB (1024)
#define KB_PER_GB (1024*1024)
#define KB_PER_TB (1024*1024*1024)
--- 96,101 ----
***************
*** 2563,2568 **** static struct config_int ConfigureNamesInt[] =
--- 2555,2571 ----
NULL, NULL, NULL
},
+ {
+ {"pending_list_cleanup_size", PGC_USERSET, CLIENT_CONN_STATEMENT,
+ gettext_noop("Sets the maximum size of the pending list for GIN index."),
+ NULL,
+ GUC_UNIT_KB
+ },
+ &pending_list_cleanup_size,
+ 4096, 0, MAX_KILOBYTES,
+ 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
***************
*** 519,524 ****
--- 519,525 ----
#bytea_output = 'hex' # hex, escape
#xmlbinary = 'base64'
#xmloption = 'content'
+ #pending_list_cleanup_size = 4MB
# - Locale and Formatting -
*** a/src/bin/psql/tab-complete.c
--- b/src/bin/psql/tab-complete.c
***************
*** 1172,1178 **** psql_completion(const char *text, int start, int end)
pg_strcasecmp(prev_wd, "(") == 0)
{
static const char *const list_INDEXOPTIONS[] =
! {"fillfactor", "fastupdate", NULL};
COMPLETE_WITH_LIST(list_INDEXOPTIONS);
}
--- 1172,1178 ----
pg_strcasecmp(prev_wd, "(") == 0)
{
static const char *const list_INDEXOPTIONS[] =
! {"fillfactor", "fastupdate", "pending_list_cleanup_size", NULL};
COMPLETE_WITH_LIST(list_INDEXOPTIONS);
}
*** a/src/include/access/gin.h
--- b/src/include/access/gin.h
***************
*** 64,71 **** typedef char GinTernaryValue;
#define GinTernaryValueGetDatum(X) ((Datum)(X))
#define PG_RETURN_GIN_TERNARY_VALUE(x) return GinTernaryValueGetDatum(x)
! /* GUC parameter */
extern PGDLLIMPORT int GinFuzzySearchLimit;
/* ginutil.c */
extern void ginGetStats(Relation index, GinStatsData *stats);
--- 64,72 ----
#define GinTernaryValueGetDatum(X) ((Datum)(X))
#define PG_RETURN_GIN_TERNARY_VALUE(x) return GinTernaryValueGetDatum(x)
! /* GUC parameters */
extern PGDLLIMPORT int GinFuzzySearchLimit;
+ extern int pending_list_cleanup_size;
/* ginutil.c */
extern void ginGetStats(Relation index, GinStatsData *stats);
*** a/src/include/access/gin_private.h
--- b/src/include/access/gin_private.h
***************
*** 314,325 **** typedef struct GinOptions
--- 314,331 ----
{
int32 vl_len_; /* varlena header (do not touch directly!) */
bool useFastUpdate; /* use fast updates? */
+ int pendingListCleanupSize; /* maximum size of pending list */
} GinOptions;
#define GIN_DEFAULT_USE_FASTUPDATE true
#define GinGetUseFastUpdate(relation) \
((relation)->rd_options ? \
((GinOptions *) (relation)->rd_options)->useFastUpdate : GIN_DEFAULT_USE_FASTUPDATE)
+ #define GinGetPendingListCleanupSize(relation) \
+ ((relation)->rd_options && \
+ ((GinOptions *) (relation)->rd_options)->pendingListCleanupSize != -1 ? \
+ ((GinOptions *) (relation)->rd_options)->pendingListCleanupSize : \
+ pending_list_cleanup_size)
/* Macros for buffer lock/unlock operations */
*** a/src/include/utils/guc.h
--- b/src/include/utils/guc.h
***************
*** 18,23 ****
--- 18,31 ----
#include "utils/array.h"
+ /* upper limit for GUC variables measured in kilobytes of memory */
+ /* note that various places assume the byte size fits in a "long" variable */
+ #if SIZEOF_SIZE_T > 4 && SIZEOF_LONG > 4
+ #define MAX_KILOBYTES INT_MAX
+ #else
+ #define MAX_KILOBYTES (INT_MAX / 1024)
+ #endif
+
/*
* Certain options can only be set at certain times. The rules are
* like this:
*** a/src/test/regress/expected/create_index.out
--- b/src/test/regress/expected/create_index.out
***************
*** 2235,2240 **** SELECT COUNT(*) FROM array_gin_test WHERE a @> '{2}';
--- 2235,2253 ----
DROP TABLE array_gin_test;
--
+ -- Test GIN index's reloptions
+ --
+ CREATE INDEX gin_relopts_test ON array_index_op_test USING gin (i)
+ WITH (FASTUPDATE=on, PENDING_LIST_CLEANUP_SIZE=32);
+ \d+ gin_relopts_test
+ Index "public.gin_relopts_test"
+ Column | Type | Definition | Storage
+ --------+---------+------------+---------
+ i | integer | i | plain
+ gin, for table "public.array_index_op_test"
+ Options: fastupdate=on, pending_list_cleanup_size=32
+
+ --
-- HASH
--
CREATE INDEX hash_i4_index ON hash_i4_heap USING hash (random int4_ops);
*** a/src/test/regress/sql/create_index.sql
--- b/src/test/regress/sql/create_index.sql
***************
*** 651,656 **** SELECT COUNT(*) FROM array_gin_test WHERE a @> '{2}';
--- 651,663 ----
DROP TABLE array_gin_test;
--
+ -- Test GIN index's reloptions
+ --
+ CREATE INDEX gin_relopts_test ON array_index_op_test USING gin (i)
+ WITH (FASTUPDATE=on, PENDING_LIST_CLEANUP_SIZE=32);
+ \d+ gin_relopts_test
+
+ --
-- HASH
--
CREATE INDEX hash_i4_index ON hash_i4_heap USING hash (random int4_ops);
(2014/10/30 21:30), Fujii Masao wrote:
On Thu, Oct 30, 2014 at 7:30 PM, Etsuro Fujita
<fujita.etsuro@lab.ntt.co.jp> wrote:
Here are my review comments.
* The patch applies cleanly and make and make check run successfully. I
think that the patch is mostly good.Thanks! Attached is the updated version of the patch.
Thank you for updating the patch!
* In src/backend/utils/misc/guc.c: + { + {"pending_list_cleanup_size", PGC_USERSET, CLIENT_CONN_STATEMENT, + gettext_noop("Sets the maximum size of the pending list for GIN index."), + NULL, + GUC_UNIT_KB + }, + &pending_list_cleanup_size, + 4096, 0, MAX_KILOBYTES, + NULL, NULL, NULL + },ISTM it'd be better to use RESOURCES_MEM, not CLIENT_CONN_STATEMENT. No?
Yes if the pending list always exists in the memory. But not, IIUC. Thought?
Exactly. But I think we can expect that in many cases, since I think
that the users would often set the GUC to a small value to the extent
that most of the pending list pages would be cached by shared buffer, to
maintain *search* performance.
I'd like to hear the opinions of others about the category for the GUC.
Also why not set min to 64, not to 0, in accoradance with that of work_mem?
I'm OK to use 64. But I just chose 0 because I could not think of any reasonable
reason why 64k is suitable as the minimum size of the pending list.
IOW, I have no idea about whether it's reasonable to use the min value of
work_mem as the min size of the pending list.
IIUC, I think that min = 0 disables fast update, so ISTM that it'd be
appropriate to set min to some positive value. And ISTM that the idea
of using the min value of work_mem is not so bad.
* In doc/src/sgml/ref/create_index.sgml:
+ <term><literal>PENDING_LIST_CLEANUP_SIZE</></term>IMHO, it seems to me better for this variable to be in lowercase in
accordance with the GUC version.Using lowercase only for pending_list_cleanup_size and uppercase for
other options
looks strange to me. What about using lowercase for all the storage options?
+1
I changed the document in that way.
*** 356,361 **** CREATE [ UNIQUE ] INDEX [ CONCURRENTLY ] [ <replaceable
class="parameter">name</
--- 356,372 ----
</listitem>
</varlistentry>
</variablelist>
+ <variablelist>
+ <varlistentry>
+ <term><literal>PENDING_LIST_CLEANUP_SIZE</></term>
The above is still in uppercse.
Thanks,
Best regards,
Etsuro Fujita
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Tue, Nov 4, 2014 at 12:04 PM, Etsuro Fujita
<fujita.etsuro@lab.ntt.co.jp> wrote:
IIUC, I think that min = 0 disables fast update, so ISTM that it'd be
appropriate to set min to some positive value. And ISTM that the idea of
using the min value of work_mem is not so bad.
OK. I changed the min value to 64kB.
*** 356,361 **** CREATE [ UNIQUE ] INDEX [ CONCURRENTLY ] [ <replaceable class="parameter">name</ --- 356,372 ---- </listitem> </varlistentry> </variablelist> + <variablelist> + <varlistentry> + <term><literal>PENDING_LIST_CLEANUP_SIZE</></term>The above is still in uppercse.
Fixed.
Attached is the updated version of the patch. Thanks for the review!
Regards,
--
Fujii Masao
Attachments:
pending_list_cleanup_size_v6.patchtext/x-patch; charset=US-ASCII; name=pending_list_cleanup_size_v6.patchDownload
*** a/doc/src/sgml/config.sgml
--- b/doc/src/sgml/config.sgml
***************
*** 5911,5916 **** SET XML OPTION { DOCUMENT | CONTENT };
--- 5911,5937 ----
</listitem>
</varlistentry>
+ <varlistentry id="guc-pending-list-cleanup-size" xreflabel="pending_list_cleanup_size">
+ <term><varname>pending_list_cleanup_size</varname> (<type>integer</type>)
+ <indexterm>
+ <primary><varname>pending_list_cleanup_size</> configuration parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ Sets the maximum size of the GIN pending list which is used
+ when <literal>fastupdate</> is enabled. If the list grows
+ larger than this maximum size, it is cleaned up by moving
+ the entries in it to the main GIN data structure in bulk.
+ The default is four megabytes (<literal>4MB</>). This setting
+ can be overridden for individual GIN indexes by changing
+ storage parameters.
+ See <xref linkend="gin-fast-update"> and <xref linkend="gin-tips">
+ for more information.
+ </para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
</sect2>
<sect2 id="runtime-config-client-format">
*** a/doc/src/sgml/gin.sgml
--- b/doc/src/sgml/gin.sgml
***************
*** 728,735 ****
from the indexed item). As of <productname>PostgreSQL</productname> 8.4,
<acronym>GIN</> is capable of postponing much of this work by inserting
new tuples into a temporary, unsorted list of pending entries.
! When the table is vacuumed, or if the pending list becomes too large
! (larger than <xref linkend="guc-work-mem">), the entries are moved to the
main <acronym>GIN</acronym> data structure using the same bulk insert
techniques used during initial index creation. This greatly improves
<acronym>GIN</acronym> index update speed, even counting the additional
--- 728,735 ----
from the indexed item). As of <productname>PostgreSQL</productname> 8.4,
<acronym>GIN</> is capable of postponing much of this work by inserting
new tuples into a temporary, unsorted list of pending entries.
! When the table is vacuumed, or if the pending list becomes larger than
! <xref linkend="guc-pending-list-cleanup-size">, the entries are moved to the
main <acronym>GIN</acronym> data structure using the same bulk insert
techniques used during initial index creation. This greatly improves
<acronym>GIN</acronym> index update speed, even counting the additional
***************
*** 750,756 ****
<para>
If consistent response time is more important than update speed,
use of pending entries can be disabled by turning off the
! <literal>FASTUPDATE</literal> storage parameter for a
<acronym>GIN</acronym> index. See <xref linkend="sql-createindex">
for details.
</para>
--- 750,756 ----
<para>
If consistent response time is more important than update speed,
use of pending entries can be disabled by turning off the
! <literal>fastupdate</literal> storage parameter for a
<acronym>GIN</acronym> index. See <xref linkend="sql-createindex">
for details.
</para>
***************
*** 812,829 ****
</varlistentry>
<varlistentry>
! <term><xref linkend="guc-work-mem"></term>
<listitem>
<para>
During a series of insertions into an existing <acronym>GIN</acronym>
! index that has <literal>FASTUPDATE</> enabled, the system will clean up
the pending-entry list whenever the list grows larger than
! <varname>work_mem</>. To avoid fluctuations in observed response time,
! it's desirable to have pending-list cleanup occur in the background
! (i.e., via autovacuum). Foreground cleanup operations can be avoided by
! increasing <varname>work_mem</> or making autovacuum more aggressive.
! However, enlarging <varname>work_mem</> means that if a foreground
! cleanup does occur, it will take even longer.
</para>
</listitem>
</varlistentry>
--- 812,837 ----
</varlistentry>
<varlistentry>
! <term><xref linkend="guc-pending-list-cleanup-size"></term>
<listitem>
<para>
During a series of insertions into an existing <acronym>GIN</acronym>
! index that has <literal>fastupdate</> enabled, the system will clean up
the pending-entry list whenever the list grows larger than
! <varname>pending_list_cleanup_size</>. To avoid fluctuations in observed
! response time, it's desirable to have pending-list cleanup occur in the
! background (i.e., via autovacuum). Foreground cleanup operations
! can be avoided by increasing <varname>pending_list_cleanup_size</>
! or making autovacuum more aggressive.
! However, enlarging the threshold of the cleanup operation means that
! if a foreground cleanup does occur, it will take even longer.
! </para>
! <para>
! <varname>pending_list_cleanup_size</> can be overridden for individual
! GIN indexes by changing storage parameters, and which allows each
! GIN index to have its own cleanup threshold.
! For example, it's possible to increase the threshold only for the GIN
! index which can be updated heavily, and decrease it otherwise.
</para>
</listitem>
</varlistentry>
*** a/doc/src/sgml/gist.sgml
--- b/doc/src/sgml/gist.sgml
***************
*** 861,867 **** my_distance(PG_FUNCTION_ARGS)
<para>
By default, a GiST index build switches to the buffering method when the
index size reaches <xref linkend="guc-effective-cache-size">. It can
! be manually turned on or off by the <literal>BUFFERING</literal> parameter
to the CREATE INDEX command. The default behavior is good for most cases,
but turning buffering off might speed up the build somewhat if the input
data is ordered.
--- 861,867 ----
<para>
By default, a GiST index build switches to the buffering method when the
index size reaches <xref linkend="guc-effective-cache-size">. It can
! be manually turned on or off by the <literal>buffering</literal> parameter
to the CREATE INDEX command. The default behavior is good for most cases,
but turning buffering off might speed up the build somewhat if the input
data is ordered.
*** a/doc/src/sgml/ref/cluster.sgml
--- b/doc/src/sgml/ref/cluster.sgml
***************
*** 46,52 **** CLUSTER [VERBOSE]
not clustered. That is, no attempt is made to store new or
updated rows according to their index order. (If one wishes, one can
periodically recluster by issuing the command again. Also, setting
! the table's <literal>FILLFACTOR</literal> storage parameter to less than
100% can aid in preserving cluster ordering during updates, since updated
rows are kept on the same page if enough space is available there.)
</para>
--- 46,52 ----
not clustered. That is, no attempt is made to store new or
updated rows according to their index order. (If one wishes, one can
periodically recluster by issuing the command again. Also, setting
! the table's <literal>fillfactor</literal> storage parameter to less than
100% can aid in preserving cluster ordering during updates, since updated
rows are kept on the same page if enough space is available there.)
</para>
*** a/doc/src/sgml/ref/create_index.sgml
--- b/doc/src/sgml/ref/create_index.sgml
***************
*** 299,305 **** CREATE [ UNIQUE ] INDEX [ CONCURRENTLY ] [ [ IF NOT EXISTS ] <replaceable class=
<variablelist>
<varlistentry>
! <term><literal>FILLFACTOR</></term>
<listitem>
<para>
The fillfactor for an index is a percentage that determines how full
--- 299,305 ----
<variablelist>
<varlistentry>
! <term><literal>fillfactor</></term>
<listitem>
<para>
The fillfactor for an index is a percentage that determines how full
***************
*** 326,332 **** CREATE [ UNIQUE ] INDEX [ CONCURRENTLY ] [ [ IF NOT EXISTS ] <replaceable class=
<variablelist>
<varlistentry>
! <term><literal>BUFFERING</></term>
<listitem>
<para>
Determines whether the buffering build technique described in
--- 326,332 ----
<variablelist>
<varlistentry>
! <term><literal>buffering</></term>
<listitem>
<para>
Determines whether the buffering build technique described in
***************
*** 340,351 **** CREATE [ UNIQUE ] INDEX [ CONCURRENTLY ] [ [ IF NOT EXISTS ] <replaceable class=
</variablelist>
<para>
! GIN indexes accept a different parameter:
</para>
<variablelist>
<varlistentry>
! <term><literal>FASTUPDATE</></term>
<listitem>
<para>
This setting controls usage of the fast update technique described in
--- 340,351 ----
</variablelist>
<para>
! GIN indexes accept different parameters:
</para>
<variablelist>
<varlistentry>
! <term><literal>fastupdate</></term>
<listitem>
<para>
This setting controls usage of the fast update technique described in
***************
*** 358,364 **** CREATE [ UNIQUE ] INDEX [ CONCURRENTLY ] [ [ IF NOT EXISTS ] <replaceable class=
<note>
<para>
! Turning <literal>FASTUPDATE</> off via <command>ALTER INDEX</> prevents
future insertions from going into the list of pending index entries,
but does not in itself flush previous entries. You might want to
<command>VACUUM</> the table afterward to ensure the pending list is
--- 358,364 ----
<note>
<para>
! Turning <literal>fastupdate</> off via <command>ALTER INDEX</> prevents
future insertions from going into the list of pending index entries,
but does not in itself flush previous entries. You might want to
<command>VACUUM</> the table afterward to ensure the pending list is
***************
*** 368,373 **** CREATE [ UNIQUE ] INDEX [ CONCURRENTLY ] [ [ IF NOT EXISTS ] <replaceable class=
--- 368,384 ----
</listitem>
</varlistentry>
</variablelist>
+ <variablelist>
+ <varlistentry>
+ <term><literal>pending_list_cleanup_size</></term>
+ <listitem>
+ <para>
+ Custom <xref linkend="guc-pending-list-cleanup-size"> parameter.
+ This value is specified in kilobytes.
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
</refsect2>
<refsect2 id="SQL-CREATEINDEX-CONCURRENTLY">
*** a/src/backend/access/common/reloptions.c
--- b/src/backend/access/common/reloptions.c
***************
*** 209,214 **** static relopt_int intRelOpts[] =
--- 209,222 ----
RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
}, -1, 0, 2000000000
},
+ {
+ {
+ "pending_list_cleanup_size",
+ "Maximum size of the pending list for this GIN index, in kilobytes.",
+ RELOPT_KIND_GIN
+ },
+ -1, 64, MAX_KILOBYTES
+ },
/* list terminator */
{{NULL}}
*** a/src/backend/access/gin/ginfast.c
--- b/src/backend/access/gin/ginfast.c
***************
*** 25,30 ****
--- 25,32 ----
#include "utils/memutils.h"
#include "utils/rel.h"
+ /* GUC parameter */
+ int pending_list_cleanup_size = 0;
#define GIN_PAGE_FREESIZE \
( BLCKSZ - MAXALIGN(SizeOfPageHeaderData) - MAXALIGN(sizeof(GinPageOpaqueData)) )
***************
*** 228,233 **** ginHeapTupleFastInsert(GinState *ginstate, GinTupleCollector *collector)
--- 230,236 ----
ginxlogUpdateMeta data;
bool separateList = false;
bool needCleanup = false;
+ int cleanupSize;
if (collector->ntuples == 0)
return;
***************
*** 422,432 **** ginHeapTupleFastInsert(GinState *ginstate, GinTupleCollector *collector)
* ginInsertCleanup could take significant amount of time, so we prefer to
* call it when it can do all the work in a single collection cycle. In
* non-vacuum mode, it shouldn't require maintenance_work_mem, so fire it
! * while pending list is still small enough to fit into work_mem.
*
* ginInsertCleanup() should not be called inside our CRIT_SECTION.
*/
! if (metadata->nPendingPages * GIN_PAGE_FREESIZE > work_mem * 1024L)
needCleanup = true;
UnlockReleaseBuffer(metabuffer);
--- 425,437 ----
* ginInsertCleanup could take significant amount of time, so we prefer to
* call it when it can do all the work in a single collection cycle. In
* non-vacuum mode, it shouldn't require maintenance_work_mem, so fire it
! * while pending list is still small enough to fit into
! * pending_list_cleanup_size.
*
* ginInsertCleanup() should not be called inside our CRIT_SECTION.
*/
! cleanupSize = GinGetPendingListCleanupSize(index);
! if (metadata->nPendingPages * GIN_PAGE_FREESIZE > cleanupSize * 1024L)
needCleanup = true;
UnlockReleaseBuffer(metabuffer);
*** a/src/backend/access/gin/ginutil.c
--- b/src/backend/access/gin/ginutil.c
***************
*** 525,531 **** ginoptions(PG_FUNCTION_ARGS)
GinOptions *rdopts;
int numoptions;
static const relopt_parse_elt tab[] = {
! {"fastupdate", RELOPT_TYPE_BOOL, offsetof(GinOptions, useFastUpdate)}
};
options = parseRelOptions(reloptions, validate, RELOPT_KIND_GIN,
--- 525,533 ----
GinOptions *rdopts;
int numoptions;
static const relopt_parse_elt tab[] = {
! {"fastupdate", RELOPT_TYPE_BOOL, offsetof(GinOptions, useFastUpdate)},
! {"pending_list_cleanup_size", RELOPT_TYPE_INT, offsetof(GinOptions,
! pendingListCleanupSize)}
};
options = parseRelOptions(reloptions, validate, RELOPT_KIND_GIN,
*** a/src/backend/utils/misc/guc.c
--- b/src/backend/utils/misc/guc.c
***************
*** 96,109 ****
#define CONFIG_EXEC_PARAMS_NEW "global/config_exec_params.new"
#endif
- /* upper limit for GUC variables measured in kilobytes of memory */
- /* note that various places assume the byte size fits in a "long" variable */
- #if SIZEOF_SIZE_T > 4 && SIZEOF_LONG > 4
- #define MAX_KILOBYTES INT_MAX
- #else
- #define MAX_KILOBYTES (INT_MAX / 1024)
- #endif
-
#define KB_PER_MB (1024)
#define KB_PER_GB (1024*1024)
#define KB_PER_TB (1024*1024*1024)
--- 96,101 ----
***************
*** 2550,2555 **** static struct config_int ConfigureNamesInt[] =
--- 2542,2558 ----
NULL, NULL, NULL
},
+ {
+ {"pending_list_cleanup_size", PGC_USERSET, CLIENT_CONN_STATEMENT,
+ gettext_noop("Sets the maximum size of the pending list for GIN index."),
+ NULL,
+ GUC_UNIT_KB
+ },
+ &pending_list_cleanup_size,
+ 4096, 64, MAX_KILOBYTES,
+ 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
***************
*** 519,524 ****
--- 519,525 ----
#bytea_output = 'hex' # hex, escape
#xmlbinary = 'base64'
#xmloption = 'content'
+ #pending_list_cleanup_size = 4MB
# - Locale and Formatting -
*** a/src/bin/psql/tab-complete.c
--- b/src/bin/psql/tab-complete.c
***************
*** 1172,1178 **** psql_completion(const char *text, int start, int end)
pg_strcasecmp(prev_wd, "(") == 0)
{
static const char *const list_INDEXOPTIONS[] =
! {"fillfactor", "fastupdate", NULL};
COMPLETE_WITH_LIST(list_INDEXOPTIONS);
}
--- 1172,1178 ----
pg_strcasecmp(prev_wd, "(") == 0)
{
static const char *const list_INDEXOPTIONS[] =
! {"fillfactor", "fastupdate", "pending_list_cleanup_size", NULL};
COMPLETE_WITH_LIST(list_INDEXOPTIONS);
}
*** a/src/include/access/gin.h
--- b/src/include/access/gin.h
***************
*** 65,72 **** typedef char GinTernaryValue;
#define GinTernaryValueGetDatum(X) ((Datum)(X))
#define PG_RETURN_GIN_TERNARY_VALUE(x) return GinTernaryValueGetDatum(x)
! /* GUC parameter */
extern PGDLLIMPORT int GinFuzzySearchLimit;
/* ginutil.c */
extern void ginGetStats(Relation index, GinStatsData *stats);
--- 65,73 ----
#define GinTernaryValueGetDatum(X) ((Datum)(X))
#define PG_RETURN_GIN_TERNARY_VALUE(x) return GinTernaryValueGetDatum(x)
! /* GUC parameters */
extern PGDLLIMPORT int GinFuzzySearchLimit;
+ extern int pending_list_cleanup_size;
/* ginutil.c */
extern void ginGetStats(Relation index, GinStatsData *stats);
*** a/src/include/access/gin_private.h
--- b/src/include/access/gin_private.h
***************
*** 315,326 **** typedef struct GinOptions
--- 315,332 ----
{
int32 vl_len_; /* varlena header (do not touch directly!) */
bool useFastUpdate; /* use fast updates? */
+ int pendingListCleanupSize; /* maximum size of pending list */
} GinOptions;
#define GIN_DEFAULT_USE_FASTUPDATE true
#define GinGetUseFastUpdate(relation) \
((relation)->rd_options ? \
((GinOptions *) (relation)->rd_options)->useFastUpdate : GIN_DEFAULT_USE_FASTUPDATE)
+ #define GinGetPendingListCleanupSize(relation) \
+ ((relation)->rd_options && \
+ ((GinOptions *) (relation)->rd_options)->pendingListCleanupSize != -1 ? \
+ ((GinOptions *) (relation)->rd_options)->pendingListCleanupSize : \
+ pending_list_cleanup_size)
/* Macros for buffer lock/unlock operations */
*** a/src/include/utils/guc.h
--- b/src/include/utils/guc.h
***************
*** 18,23 ****
--- 18,31 ----
#include "utils/array.h"
+ /* upper limit for GUC variables measured in kilobytes of memory */
+ /* note that various places assume the byte size fits in a "long" variable */
+ #if SIZEOF_SIZE_T > 4 && SIZEOF_LONG > 4
+ #define MAX_KILOBYTES INT_MAX
+ #else
+ #define MAX_KILOBYTES (INT_MAX / 1024)
+ #endif
+
/*
* Certain options can only be set at certain times. The rules are
* like this:
*** a/src/test/regress/expected/create_index.out
--- b/src/test/regress/expected/create_index.out
***************
*** 2241,2246 **** SELECT COUNT(*) FROM array_gin_test WHERE a @> '{2}';
--- 2241,2259 ----
DROP TABLE array_gin_test;
--
+ -- Test GIN index's reloptions
+ --
+ CREATE INDEX gin_relopts_test ON array_index_op_test USING gin (i)
+ WITH (FASTUPDATE=on, PENDING_LIST_CLEANUP_SIZE=128);
+ \d+ gin_relopts_test
+ Index "public.gin_relopts_test"
+ Column | Type | Definition | Storage
+ --------+---------+------------+---------
+ i | integer | i | plain
+ gin, for table "public.array_index_op_test"
+ Options: fastupdate=on, pending_list_cleanup_size=128
+
+ --
-- HASH
--
CREATE INDEX hash_i4_index ON hash_i4_heap USING hash (random int4_ops);
*** a/src/test/regress/sql/create_index.sql
--- b/src/test/regress/sql/create_index.sql
***************
*** 655,660 **** SELECT COUNT(*) FROM array_gin_test WHERE a @> '{2}';
--- 655,667 ----
DROP TABLE array_gin_test;
--
+ -- Test GIN index's reloptions
+ --
+ CREATE INDEX gin_relopts_test ON array_index_op_test USING gin (i)
+ WITH (FASTUPDATE=on, PENDING_LIST_CLEANUP_SIZE=128);
+ \d+ gin_relopts_test
+
+ --
-- HASH
--
CREATE INDEX hash_i4_index ON hash_i4_heap USING hash (random int4_ops);
(2014/11/06 23:38), Fujii Masao wrote:
On Tue, Nov 4, 2014 at 12:04 PM, Etsuro Fujita
<fujita.etsuro@lab.ntt.co.jp> wrote:IIUC, I think that min = 0 disables fast update, so ISTM that it'd be
appropriate to set min to some positive value. And ISTM that the idea of
using the min value of work_mem is not so bad.OK. I changed the min value to 64kB.
*** 356,361 **** CREATE [ UNIQUE ] INDEX [ CONCURRENTLY ] [ <replaceable class="parameter">name</ --- 356,372 ---- </listitem> </varlistentry> </variablelist> + <variablelist> + <varlistentry> + <term><literal>PENDING_LIST_CLEANUP_SIZE</></term>The above is still in uppercse.
Fixed.
Attached is the updated version of the patch. Thanks for the review!
Thanks for the updating the patch!
The patch looks good to me except for the following point:
*** a/src/backend/access/gin/ginfast.c
--- b/src/backend/access/gin/ginfast.c
***************
*** 25,30 ****
--- 25,32 ----
#include "utils/memutils.h"
#include "utils/rel.h"
+ /* GUC parameter */
+ int pending_list_cleanup_size = 0;
I think we need to initialize the GUC to boot_val, 4096 in this case.
I asked the opinions of others about the config_group of the GUC. But
there seems no opinions, so I agree with Fujii-san's idea of assigning
the GUC CLIENT_CONN_STATEMENT.
Thanks,
Best regards,
Etsuro Fujita
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Mon, Nov 10, 2014 at 4:15 PM, Etsuro Fujita
<fujita.etsuro@lab.ntt.co.jp> wrote:
(2014/11/06 23:38), Fujii Masao wrote:
On Tue, Nov 4, 2014 at 12:04 PM, Etsuro Fujita
<fujita.etsuro@lab.ntt.co.jp> wrote:IIUC, I think that min = 0 disables fast update, so ISTM that it'd be
appropriate to set min to some positive value. And ISTM that the idea of
using the min value of work_mem is not so bad.OK. I changed the min value to 64kB.
*** 356,361 **** CREATE [ UNIQUE ] INDEX [ CONCURRENTLY ] [ <replaceable class="parameter">name</ --- 356,372 ---- </listitem> </varlistentry> </variablelist> + <variablelist> + <varlistentry> + <term><literal>PENDING_LIST_CLEANUP_SIZE</></term>The above is still in uppercse.
Fixed.
Attached is the updated version of the patch. Thanks for the review!
Thanks for the updating the patch!
The patch looks good to me except for the following point:
Thanks for the review again!
*** a/src/backend/access/gin/ginfast.c --- b/src/backend/access/gin/ginfast.c *************** *** 25,30 **** --- 25,32 ---- #include "utils/memutils.h" #include "utils/rel.h"+ /* GUC parameter */ + int pending_list_cleanup_size = 0;I think we need to initialize the GUC to boot_val, 4096 in this case.
No, IIUC basically the variable for GUC doesn't need to be initialized
to its default value. OTOH, it's also harmless to initialize it to the default.
I like the current code a bit because we don't need to change the initial
value again when we decide to change the default value of GUC.
I have no strong opinion about this, though.
Regards,
--
Fujii Masao
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
(2014/11/11 2:31), Fujii Masao wrote:
On Mon, Nov 10, 2014 at 4:15 PM, Etsuro Fujita
The patch looks good to me except for the following point:
*** a/src/backend/access/gin/ginfast.c --- b/src/backend/access/gin/ginfast.c *************** *** 25,30 **** --- 25,32 ---- #include "utils/memutils.h" #include "utils/rel.h"+ /* GUC parameter */ + int pending_list_cleanup_size = 0;I think we need to initialize the GUC to boot_val, 4096 in this case.
No, IIUC basically the variable for GUC doesn't need to be initialized
to its default value. OTOH, it's also harmless to initialize it to the default.
I like the current code a bit because we don't need to change the initial
value again when we decide to change the default value of GUC.
I have no strong opinion about this, though.
OK, so if there are no objections of others, I'll mark this as "Ready
for Committer".
Thanks,
Best regards,
Etsuro Fujita
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Tue, Nov 11, 2014 at 7:01 PM, Etsuro Fujita
<fujita.etsuro@lab.ntt.co.jp> wrote:
(2014/11/11 2:31), Fujii Masao wrote:
On Mon, Nov 10, 2014 at 4:15 PM, Etsuro Fujita
The patch looks good to me except for the following point:
*** a/src/backend/access/gin/ginfast.c --- b/src/backend/access/gin/ginfast.c *************** *** 25,30 **** --- 25,32 ---- #include "utils/memutils.h" #include "utils/rel.h"+ /* GUC parameter */ + int pending_list_cleanup_size = 0;I think we need to initialize the GUC to boot_val, 4096 in this case.
No, IIUC basically the variable for GUC doesn't need to be initialized
to its default value. OTOH, it's also harmless to initialize it to the
default.
I like the current code a bit because we don't need to change the initial
value again when we decide to change the default value of GUC.
I have no strong opinion about this, though.OK, so if there are no objections of others, I'll mark this as "Ready for
Committer".
I just pushed this. Thanks!
Regards,
--
Fujii Masao
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Tue, Nov 11, 2014 at 7:11 AM, Fujii Masao <masao.fujii@gmail.com> wrote:
OK, so if there are no objections of others, I'll mark this as "Ready for
Committer".I just pushed this. Thanks!
Not to kibitz too much after-the-fact, but wouldn't it be better to
give this a name that has "GIN" in it somewhere?
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Tue, Nov 11, 2014 at 10:14 PM, Robert Haas <robertmhaas@gmail.com> wrote:
On Tue, Nov 11, 2014 at 7:11 AM, Fujii Masao <masao.fujii@gmail.com> wrote:
OK, so if there are no objections of others, I'll mark this as "Ready for
Committer".I just pushed this. Thanks!
Not to kibitz too much after-the-fact, but wouldn't it be better to
give this a name that has "GIN" in it somewhere?
Maybe. gin_pending_list_cleanup_size? gin_pending_list_limit? Better name?
Regards,
--
Fujii Masao
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Fujii Masao <masao.fujii@gmail.com> writes:
On Tue, Nov 11, 2014 at 10:14 PM, Robert Haas <robertmhaas@gmail.com> wrote:
Not to kibitz too much after-the-fact, but wouldn't it be better to
give this a name that has "GIN" in it somewhere?
Maybe. gin_pending_list_cleanup_size? gin_pending_list_limit? Better name?
gin_pending_list_limit sounds good to me.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Nov 12, 2014 at 12:40 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Fujii Masao <masao.fujii@gmail.com> writes:
On Tue, Nov 11, 2014 at 10:14 PM, Robert Haas <robertmhaas@gmail.com> wrote:
Not to kibitz too much after-the-fact, but wouldn't it be better to
give this a name that has "GIN" in it somewhere?Maybe. gin_pending_list_cleanup_size? gin_pending_list_limit? Better name?
gin_pending_list_limit sounds good to me.
OK, barring any objection, I will rename the reloption and GUC to
gin_pending_list_limit.
Regards,
--
Fujii Masao
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Nov 12, 2014 at 9:30 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
On Wed, Nov 12, 2014 at 12:40 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Fujii Masao <masao.fujii@gmail.com> writes:
On Tue, Nov 11, 2014 at 10:14 PM, Robert Haas <robertmhaas@gmail.com> wrote:
Not to kibitz too much after-the-fact, but wouldn't it be better to
give this a name that has "GIN" in it somewhere?Maybe. gin_pending_list_cleanup_size? gin_pending_list_limit? Better name?
gin_pending_list_limit sounds good to me.
OK, barring any objection, I will rename the reloption and GUC to
gin_pending_list_limit.
Done.
Regards,
--
Fujii Masao
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Oct 30, 2014 at 5:30 AM, Fujii Masao <masao.fujii@gmail.com> wrote:
On Thu, Oct 30, 2014 at 7:30 PM, Etsuro Fujita
<fujita.etsuro@lab.ntt.co.jp> wrote:+ { + {"pending_list_cleanup_size", PGC_USERSET, CLIENT_CONN_STATEMENT, + gettext_noop("Sets the maximum size of thepending
list for GIN index."), + NULL, + GUC_UNIT_KB + }, + &pending_list_cleanup_size, + 4096, 0, MAX_KILOBYTES, + NULL, NULL, NULL + },ISTM it'd be better to use RESOURCES_MEM, not CLIENT_CONN_STATEMENT. No?
Yes if the pending list always exists in the memory. But not, IIUC.
Thought?Also why not set min to 64, not to 0, in accoradance with that of
work_mem?
I'm OK to use 64. But I just chose 0 because I could not think of any
reasonable
reason why 64k is suitable as the minimum size of the pending list.
IOW, I have no idea about whether it's reasonable to use the min value of
work_mem as the min size of the pending list.
I know I am late to the party here, but would like to have the minimum be
0, not 64. As long as by zero, it means it doesn't insert anything into
the pending list, rather than inserting and promptly cleaning it up.
The reason for this is that if I am trying to decide what
pending_list_cleanup_size I want to set for the index in the indexes
storage parameters, the way to find that out is try a bunch of different
ones through the guc while the index is still at the default of no
overriding storage parameter. It would be nice to try the fastupdate=off
alternative (i.e. the same as pending_list_cleanup_size=0) without having
to change the index itself and change the syntax used in the testing.
Cheers,
Jeff
On Tue, Aug 11, 2015 at 2:58 AM, Jeff Janes <jeff.janes@gmail.com> wrote:
On Thu, Oct 30, 2014 at 5:30 AM, Fujii Masao <masao.fujii@gmail.com> wrote:
On Thu, Oct 30, 2014 at 7:30 PM, Etsuro Fujita
<fujita.etsuro@lab.ntt.co.jp> wrote:+ { + {"pending_list_cleanup_size", PGC_USERSET, CLIENT_CONN_STATEMENT, + gettext_noop("Sets the maximum size of the pending list for GIN index."), + NULL, + GUC_UNIT_KB + }, + &pending_list_cleanup_size, + 4096, 0, MAX_KILOBYTES, + NULL, NULL, NULL + },ISTM it'd be better to use RESOURCES_MEM, not CLIENT_CONN_STATEMENT. No?
Yes if the pending list always exists in the memory. But not, IIUC.
Thought?Also why not set min to 64, not to 0, in accoradance with that of
work_mem?I'm OK to use 64. But I just chose 0 because I could not think of any
reasonable
reason why 64k is suitable as the minimum size of the pending list.
IOW, I have no idea about whether it's reasonable to use the min value of
work_mem as the min size of the pending list.I know I am late to the party here, but would like to have the minimum be 0,
not 64. As long as by zero, it means it doesn't insert anything into the
pending list, rather than inserting and promptly cleaning it up.The reason for this is that if I am trying to decide what
pending_list_cleanup_size I want to set for the index in the indexes storage
parameters, the way to find that out is try a bunch of different ones
through the guc while the index is still at the default of no overriding
storage parameter. It would be nice to try the fastupdate=off alternative
(i.e. the same as pending_list_cleanup_size=0) without having to change the
index itself and change the syntax used in the testing.
Sounds OK to me. So we should change the minimum values of both
gin_pending_list_limit GUC and storage parameters to 0? Or only GUC?
Regards,
--
Fujii Masao
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers