[PATCHES] COPY LOCK for WAL bypass
Following patch implements COPY ... FROM ... LOCK as discussed earlier this year on these threads: http://archives.postgresql.org/pgsql-hackers/2005-06/msg00069.php http://archives.postgresql.org/pgsql-hackers/2005-06/msg00075.php The purpose of the new command is to make an explicit request to run COPY without producing WAL records (i.e. no logging), so as to improve the performance of data loads. (This is the first of a number of COPY performance optimizations, discussed on -hackers). Default COPY is unchanged. LOCK option takes an EXCLUSIVE lock (but perhaps that should be a SHARE ROW EXCLUSIVE?), allowing it to block out CREATE INDEX builds and VACUUM. LOCK option will also cause writing of WAL records to be skipped when XLogArchivingActive() and there are no indexes. The implementation refactors the code used by CTAS for syncing the data to disk once command is complete; COPY and CTAS now both use that code. COPY .. LOCK doesn't write any XLog records as was previously suggested. My train of thought: After some thought, no other heap-based xlog action would leave the table in a consistent state after failure. Nobody wants to see a random single row in the table. I looked into UPDATEing the last row loaded to generate an xlog rec but it would be difficult to do that without being horribly kludgy. I looked at adding a new xlog action, but there is only one slot left for a heap-based xlog action, so that seemed unwise. I wrote this using an RM_XLOG xlrec, but that doesn't actually trigger a commit write (I discover). I've simply set a flag to tell the transaction to record the commit anyway. That was already there in heapam.c anyway, but just for temp relations; I've changed the name of the variable to indicate what it does now, across a number of files. (It is also arguable that I should implement a WAL record that truncates the file back down to the starting size, in the event of a failure. I'm not sure where we were on that; there seem to be comments both in favour and against that idea. I can see the use for that, so I'll be happy to add that also, if we agree there is no danger.) I've added a few lock options onto the copy.sql test script, but not added (yet) a full suite of testing. No docs, yet. Short performance test shown below for 10^6 rows, one col table. 1. Normal COPY 4.5s 11.4s 6.0s 6.1s 2. COPY LOCK 3.0s 2.7s 2.8s 2.7s with postgresql.conf all default apart from: checkpoint_segments=30 This test was an all in cache test. The improvement is substantial, but the numbers above are best case, IMHO: I would expect only 10-40% improvement for larger loads in the general case. Short output shown below, with checkpoint_segments=3, so timings for the standard non-LOCK COPY probably include checkpoint time also. === postgres=# create table ctest as select generate_series(1,100)::integer as col1; SELECT postgres=# copy ctest to '/usr/local/pgsql/ctest.data'; COPY postgres=# truncate ctest; TRUNCATE TABLE Time: 41.343 ms postgres=# copy ctest from '/usr/local/pgsql/ctest.data'; COPY Time: 7111.205 ms postgres=# truncate ctest; TRUNCATE TABLE Time: 23.175 ms postgres=# copy ctest from '/usr/local/pgsql/ctest.data' lock; COPY Time: 2992.482 ms postgres=# truncate ctest; TRUNCATE TABLE Time: 8.306 ms postgres=# copy ctest from '/usr/local/pgsql/ctest.data'; COPY Time: 7433.166 ms Best Regards, Simon Riggs Index: src/backend/access/heap/heapam.c === RCS file: /projects/cvsroot/pgsql/src/backend/access/heap/heapam.c,v retrieving revision 1.205 diff -c -r1.205 heapam.c *** src/backend/access/heap/heapam.c 26 Nov 2005 05:03:06 - 1.205 --- src/backend/access/heap/heapam.c 10 Dec 2005 10:54:58 - *** *** 28,33 --- 28,34 * heap_update - replace a tuple in a relation with another tuple * heap_markpos - mark scan position * heap_restrpos - restore position to marked location + * heap_sync - sync heap, for when no WAL has been written * * NOTES * This file contains the heap_ routines which implement *** *** 49,54 --- 50,56 #include miscadmin.h #include pgstat.h #include storage/procarray.h + #include storage/smgr.h #include utils/inval.h #include utils/relcache.h *** *** 1335,1342 * non-temp relation. Safe usage of this behavior requires that we arrange * that all new tuples go into new pages not containing any tuples from other * transactions, that the relation gets fsync'd before commit, and that the ! * transaction emits at least one WAL record to ensure RecordTransactionCommit ! * will decide to WAL-log the commit. * * use_fsm is passed directly to RelationGetBufferForTuple, which see for * more info. --- 1337,1345 * non-temp relation. Safe usage of this behavior requires that we arrange * that all new tuples go into new pages
[PATCHES] running script on server shutdown (TODO)
I've written a small patch for following TODO item:
«Add GUC variable to run a command on database
panic or smart/fast/immediate shutdown.»
It adds two GUC variables as:
enable_atexit_script_file
atexit_script_file
postmaster will run related script file with passing shutdown type
(like smart, fast or immediate) as parameter to script file
on database shutdown. (Sorry, I'm not so good on PostgreSQL internals,
thus couldn't handle panic situation at the moment.)
I know, it's not the perfect one but I'd be so apprecited to hear
suggestions for fixing lost/wrong parts of it.
Regards.
--
We are the middle children of history, raised by television to believe
that someday we'll be millionaires and movie stars and rock stars, but
we won't. And we're just learning this fact, Tyler said. So don't
fuck with us.
Index: src/backend/postmaster/postmaster.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/postmaster/postmaster.c,v
retrieving revision 1.476
diff -u -r1.476 postmaster.c
--- src/backend/postmaster/postmaster.c 22 Nov 2005 18:17:18 - 1.476
+++ src/backend/postmaster/postmaster.c 10 Dec 2005 18:28:04 -
@@ -254,6 +254,7 @@
static void ConnFree(Port *port);
static void reset_shared(int port);
static void SIGHUP_handler(SIGNAL_ARGS);
+static void RunAtexitScriptFile(const char *);
static void pmdie(SIGNAL_ARGS);
static void reaper(SIGNAL_ARGS);
static void sigusr1_handler(SIGNAL_ARGS);
@@ -1856,6 +1858,21 @@
errno = save_errno;
}
+/*
+ * RunAtexitScriptFile -- run configured atexit script.
+ */
+static void
+RunAtexitScriptFile(const char *shuttype)
+{
+ if (enable_atexit_script_file atexit_script_file)
+ {
+ int l = strlen(atexit_script_file) +
strlen(shuttype) + 2;
+ charbuf[l];
+
+ sprintf(buf, %s %s, atexit_script_file, shuttype);
+ system(buf);
+ }
+}
/*
* pmdie -- signal handler for processing various postmaster signals.
@@ -1885,6 +1902,7 @@
Shutdown = SmartShutdown;
ereport(LOG,
(errmsg(received smart shutdown
request)));
+ RunAtexitScriptFile(smart);
/*
* We won't wait out an autovacuum iteration ...
@@ -1931,6 +1949,7 @@
Shutdown = FastShutdown;
ereport(LOG,
(errmsg(received fast shutdown
request)));
+ RunAtexitScriptFile(fast);
if (DLGetHead(BackendList) || AutoVacPID != 0)
{
@@ -1978,6 +1997,8 @@
*/
ereport(LOG,
(errmsg(received immediate shutdown
request)));
+ RunAtexitScriptFile(immediate);
+
if (StartupPID != 0)
kill(StartupPID, SIGQUIT);
if (BgWriterPID != 0)
Index: src/backend/utils/misc/guc.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/utils/misc/guc.c,v
retrieving revision 1.301
diff -u -r1.301 guc.c
--- src/backend/utils/misc/guc.c22 Nov 2005 18:17:26 - 1.301
+++ src/backend/utils/misc/guc.c10 Dec 2005 18:28:24 -
@@ -145,6 +145,7 @@
/*
* GUC option variables that are exported from this module
*/
+bool enable_atexit_script_file = false;
#ifdef USE_ASSERT_CHECKING
bool assert_enabled = true;
#endif
@@ -181,6 +182,7 @@
char *HbaFileName;
char *IdentFileName;
char *external_pid_file;
+char *atexit_script_file;
inttcp_keepalives_idle;
inttcp_keepalives_interval;
@@ -391,6 +393,14 @@
static struct config_bool ConfigureNamesBool[] =
{
{
+ {enable_atexit_script_file, PGC_POSTMASTER, FILE_LOCATIONS,
+ gettext_noop(Enables usage of atexit script file on
postmaster shutdown.),
+ NULL
+ },
+ enable_atexit_script_file,
+ false, NULL, NULL
+ },
+ {
{enable_seqscan, PGC_USERSET, QUERY_TUNING_METHOD,
gettext_noop(Enables the planner's use of
sequential-scan plans.),
NULL
@@ -2119,6 +2129,16 @@
NULL, assign_canonical_path, NULL
},
+ {
+ {atexit_script_file, PGC_POSTMASTER, FILE_LOCATIONS,
+ gettext_noop(Executes specified atexit script file on
database shutdown.),
+ NULL,
+ GUC_SUPERUSER_ONLY
+ },
+ atexit_script_file,
+ NULL, assign_canonical_path,
Re: [PATCHES] running script on server shutdown (TODO)
Volkan YAZICI [EMAIL PROTECTED] writes: I've written a small patch for following TODO item: «Add GUC variable to run a command on database panic or smart/fast/immediate shutdown.» I'm not sure why this is in TODO; it's a stupid if not outright dangerous idea. Quite aside from any security considerations, the time the script takes is taken away from the time available to shut down the database before the kernel takes matters into its own hands. Besides, what is the point? Whatever you need to do can be incorporated into the initscript you use to start up or shut down the postmaster. I see no need for this functionality to be inside the postmaster. regards, tom lane ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] running script on server shutdown (TODO)
Andrew Dunstan [EMAIL PROTECTED] writes: This is what bothers me about having such an informal TODO list. There is a danger that people will work in items only to have them shot down, which is a great way to turn off developers. And there is also a danger that other people will think that the todo item is likely to be accepted at some stage. I've complained to Bruce about that before --- there are a number of items on TODO that only one person thinks is a good idea. Perhaps some sort of [controversial] marker would be useful to warn people that the item needs more discussion before going off in a corner and trying to implement it. regards, tom lane ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] running script on server shutdown (TODO)
On 12/10/05, Tom Lane [EMAIL PROTECTED] wrote: Andrew Dunstan [EMAIL PROTECTED] writes: This is what bothers me about having such an informal TODO list. There is a danger that people will work in items only to have them shot down, which is a great way to turn off developers. And there is also a danger that other people will think that the todo item is likely to be accepted at some stage. I've complained to Bruce about that before --- there are a number of items on TODO that only one person thinks is a good idea. Perhaps some sort of [controversial] marker would be useful to warn people that the item needs more discussion before going off in a corner and trying to implement it. regards, tom lane Actually some items are marked with a '?' that shows that that item needs discussion... although that it's not clearly stated in no where in the TODO... Maybe be explicit about what the '?' mark means and mark every new item with it until there is concensus a on it -- regards, Jaime Casanova (DBA: DataBase Aniquilator ;) ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] running script on server shutdown (TODO)
Tom Lane wrote: Andrew Dunstan [EMAIL PROTECTED] writes: This is what bothers me about having such an informal TODO list. There is a danger that people will work in items only to have them shot down, which is a great way to turn off developers. And there is also a danger that other people will think that the todo item is likely to be accepted at some stage. I've complained to Bruce about that before --- there are a number of items on TODO that only one person thinks is a good idea. Perhaps some sort of [controversial] marker would be useful to warn people that the item needs more discussion before going off in a corner and trying to implement it. Well, the item was added at the request of Peter Eisentraut and Martijn van Oosterhout and took place on hackers. The TODO addition was posted too: http://archives.postgresql.org/pgsql-hackers/2005-12/msg00333.php Not sure what else can be done to improve this process. I will remove the TODO item. And our developer TODO has: H3 id=item1.41.4) What do I do after choosing an item to work on?/H3 PSend an email to pgsql-hackers with a proposal for what you want to do (assuming your contribution is not trivial). Working in isolation is not advisable because others might be working on the asame TODO item, or you might have misunderstood the TODO item. In the email, discuss both the internal implementation method you plan to use, and any user-visible changes (new syntax, etc). For complex patches, it is important to get community feeback on your proposal before starting work. Failure to do so might mean your patch is rejected./P So I think we are covered there too. -- Bruce Momjian| http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] running script on server shutdown (TODO)
Jaime Casanova wrote: On 12/10/05, Tom Lane [EMAIL PROTECTED] wrote: Andrew Dunstan [EMAIL PROTECTED] writes: This is what bothers me about having such an informal TODO list. There is a danger that people will work in items only to have them shot down, which is a great way to turn off developers. And there is also a danger that other people will think that the todo item is likely to be accepted at some stage. I've complained to Bruce about that before --- there are a number of items on TODO that only one person thinks is a good idea. Perhaps some sort of [controversial] marker would be useful to warn people that the item needs more discussion before going off in a corner and trying to implement it. regards, tom lane Actually some items are marked with a '?' that shows that that item needs discussion... although that it's not clearly stated in no where in the TODO... Maybe be explicit about what the '?' mark means and mark every new item with it until there is concensus a on it Well, I would think a question mark would be pretty clear. The problem here is that no one objected to its addition to the TODO list, so it never got a ?. -- Bruce Momjian| http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
