Re: [HACKERS] Reduced power consumption in WAL Writer process
Simon Riggs writes: > On Sun, Jul 17, 2011 at 10:41 PM, Tom Lane wrote: >> Yeah, I agree with putting the actual SetLatch call after we release the >> lock ... but doesn't the calculation need to be done while we're still >> holding it? Otherwise it'd be using potentially-inconsistent values. > The calculation is just a heurustic, so doesn't need to be exactly > accurate. We need the latest values derived while holding the lock, > but we don't need to ensure they change while we make the calc. > The calc needs to say "if we are the ones who make the array more than > half full, then wake up the WALWriter". It might already be awake, it > might be already be more than half full or it might even be less than > half full now - none of those cases are very important. Nonetheless, I think it'll be better to make the calculation and set a local bool variable (to tell whether to do SetLatch later) while we're in the midst of the advance-to-next-WAL-buffer code. Even if it's true that we would generally get an OK answer by reading the variables again after releasing the lock, that's highly unlikely to be a performance win, because of cache line contention effects (ie, having to wrest the line back from the next guy doing WALInsert, and then give it back to him afterwards). Once you release the lock, you should stop touching shared memory, period. 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
Re: [HACKERS] Reduced power consumption in WAL Writer process
On Mon, Jul 18, 2011 at 1:48 AM, Robert Haas wrote: > Might be good to start a new thread for each auxilliary process, or we > may get confused. +1 -- Simon Riggs 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
Re: [HACKERS] Reduced power consumption in WAL Writer process
On Sun, Jul 17, 2011 at 10:41 PM, Tom Lane wrote: > Simon Riggs writes: >> On Fri, Jul 15, 2011 at 6:33 PM, Tom Lane wrote: >>> I'd say send the signal when wal buffers are more than X% full (maybe >>> half). The suggestion to send it when wrapping around at the end of the >>> array is not quite right, because that's an arbitrary condition that's >>> not related to how much work there is for the walwriter to do. It >>> should be cheap to check for this while advancing to a new wal buffer. > >> I think we need to put the calculation and SetLatch() after we release >> WALInsertLock, so as to avoid adding contention. > > Yeah, I agree with putting the actual SetLatch call after we release the > lock ... but doesn't the calculation need to be done while we're still > holding it? Otherwise it'd be using potentially-inconsistent values. The calculation is just a heurustic, so doesn't need to be exactly accurate. We need the latest values derived while holding the lock, but we don't need to ensure they change while we make the calc. The calc needs to say "if we are the ones who make the array more than half full, then wake up the WALWriter". It might already be awake, it might be already be more than half full or it might even be less than half full now - none of those cases are very important. -- Simon Riggs 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
Re: [HACKERS] Reduced power consumption in WAL Writer process
On Sun, Jul 17, 2011 at 8:20 PM, Peter Geoghegan wrote: > This is a bit of a detour, but probably a useful one. Attached is a > patch that replaces a tight PostmasterIsAlive() polling loop in the AV > launcher with a latch, making use of the new WL_POSTMASTER_DEATH > functionality. It's similar to what we've already done for the > archiver. It is relatively straightforward as these auxiliary process > polling loop elimination patches go (certainly compared to what we're > contemplating with the WAL writer), but it raises some questions that > we were lucky to have been able to avoid when I worked on the > archiver. Obviously, this patch isn't finished. Might be good to start a new thread for each auxilliary process, or we may get confused. > We register various generic signal handlers for the AVLauncher, > including StatementCancelHandler(). Of course, signals that are > handled generically have the same potential to invalidate WaitLatch() > timeout as any other type of signal. We should be mindful of this. Right. > ISTM that these generic handlers ought to be handling this > generically, and that there should be a Latch for just this purpose > for each process within PGPROC. We already have this Latch in PGPROC: > > Latch waitLatch; /* allow us to wait for sync rep */ > > Maybe its purpose should be expanded to "current process Latch"? I think that could be a really good idea, though I haven't looked at it closely enough to be sure. > Another concern is, what happens when we receive a signal, generically > handled or otherwise, and have to SetLatch() to avoid time-out > invalidation? Should we just live with a spurious > AutoVacLauncherMain() iteration, or should we do something like check > if the return value of WaitLatch indicates that we woke up due to a > SetLatch() call, which must have been within a singal handler, and > that we should therefore goto just before WaitLatch() and elide the > spurious iteration? Given that we can expect some signals to occur > relatively frequently, spurious iterations could be a real concern. Really? I suspect that it doesn't much matter exactly how many machine language instructions we execute on each wake-up, within reasonable bounds, of course. Maybe some testing is in order? On another note, I might be inclined to write something like: if ((return_value_of_waitlatch & WL_POSTMASTER_DEATH) && !PostmasterIsAlive()) proc_exit(1); ...so as to avoid calling that function unnecessarily on every iteration. > Incidentally, should I worry about the timeout long for WaitLatch() > overflowing? How would that happen? -- 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
Re: [HACKERS] Reduced power consumption in WAL Writer process
This is a bit of a detour, but probably a useful one. Attached is a
patch that replaces a tight PostmasterIsAlive() polling loop in the AV
launcher with a latch, making use of the new WL_POSTMASTER_DEATH
functionality. It's similar to what we've already done for the
archiver. It is relatively straightforward as these auxiliary process
polling loop elimination patches go (certainly compared to what we're
contemplating with the WAL writer), but it raises some questions that
we were lucky to have been able to avoid when I worked on the
archiver. Obviously, this patch isn't finished.
We register various generic signal handlers for the AVLauncher,
including StatementCancelHandler(). Of course, signals that are
handled generically have the same potential to invalidate WaitLatch()
timeout as any other type of signal. We should be mindful of this.
ISTM that these generic handlers ought to be handling this
generically, and that there should be a Latch for just this purpose
for each process within PGPROC. We already have this Latch in PGPROC:
Latch waitLatch; /* allow us to wait for sync rep */
Maybe its purpose should be expanded to "current process Latch"?
Another concern is, what happens when we receive a signal, generically
handled or otherwise, and have to SetLatch() to avoid time-out
invalidation? Should we just live with a spurious
AutoVacLauncherMain() iteration, or should we do something like check
if the return value of WaitLatch indicates that we woke up due to a
SetLatch() call, which must have been within a singal handler, and
that we should therefore goto just before WaitLatch() and elide the
spurious iteration? Given that we can expect some signals to occur
relatively frequently, spurious iterations could be a real concern.
Incidentally, should I worry about the timeout long for WaitLatch()
overflowing?
--
Peter Geoghegan http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and Services
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 2f3fcbf..cf5eb59 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -84,6 +84,7 @@
#include "postmaster/postmaster.h"
#include "storage/bufmgr.h"
#include "storage/ipc.h"
+#include "storage/latch.h"
#include "storage/pmsignal.h"
#include "storage/proc.h"
#include "storage/procsignal.h"
@@ -267,6 +268,10 @@ int AutovacuumLauncherPid = 0;
static pid_t avlauncher_forkexec(void);
static pid_t avworker_forkexec(void);
#endif
+/*
+ * Latch used by signal handlers to wake up the sleep in the main loop.
+ */
+static Latch mainloop_latch;
NON_EXEC_STATIC void AutoVacWorkerMain(int argc, char *argv[]);
NON_EXEC_STATIC void AutoVacLauncherMain(int argc, char *argv[]);
@@ -545,6 +550,8 @@ AutoVacLauncherMain(int argc, char *argv[])
*/
rebuild_database_list(InvalidOid);
+ InitLatch(&mainloop_latch);
+
for (;;)
{
struct timeval nap;
@@ -567,38 +574,20 @@ AutoVacLauncherMain(int argc, char *argv[])
/*
* Sleep for a while according to schedule.
+ * Wait on Latch.
*
- * On some platforms, signals won't interrupt the sleep. To ensure we
- * respond reasonably promptly when someone signals us, break down the
- * sleep into 1-second increments, and check for interrupts after each
- * nap.
+ * We handle wait time invalidation by calling
+ * SetLatch() in signal handlers.
*/
- while (nap.tv_sec > 0 || nap.tv_usec > 0)
- {
- uint32 sleeptime;
-
- if (nap.tv_sec > 0)
- {
-sleeptime = 100;
-nap.tv_sec--;
- }
- else
- {
-sleeptime = nap.tv_usec;
-nap.tv_usec = 0;
- }
- pg_usleep(sleeptime);
-
- /*
- * Emergency bailout if postmaster has died. This is to avoid the
- * necessity for manual cleanup of all postmaster children.
- */
- if (!PostmasterIsAlive())
-proc_exit(1);
-
- if (got_SIGTERM || got_SIGHUP || got_SIGUSR2)
-break;
- }
+ WaitLatch(&mainloop_latch, WL_LATCH_SET | WL_TIMEOUT | WL_POSTMASTER_DEATH, (nap.tv_sec * 100L) + nap.tv_usec);
+ /*
+ * Emergency bailout if postmaster has died. This is to avoid the
+ * necessity for manual cleanup of all postmaster children.
+ *
+ * This happens again here because we may have slept for quite a while.
+ */
+ if (!PostmasterIsAlive())
+ proc_exit(1);
DisableCatchupInterrupt();
@@ -1322,6 +1311,8 @@ static void
avl_sighup_handler(SIGNAL_ARGS)
{
got_SIGHUP = true;
+ /* let the waiting loop iterate */
+ SetLatch(&mainloop_latch);
}
/* SIGUSR2: a worker is up and running, or just finished, or failed to fork */
@@ -1329,6 +1320,8 @@ static void
avl_sigusr2_handler(SIGNAL_ARGS)
{
got_SIGUSR2 = true;
+ /* let the waiting loop iterate */
+ SetLatch(&mainloop_latch);
}
/* SIGTERM: time to die */
@@ -1336,6 +1329,8 @@ static void
avl_sigterm_handler(SIGNAL_ARGS)
{
got_SIGTERM = true;
+ /* let the waiting loop iterate */
+ SetLatch(&mainloop_latch);
Re: [HACKERS] Reduced power consumption in WAL Writer process
Simon Riggs writes: > On Fri, Jul 15, 2011 at 6:33 PM, Tom Lane wrote: >> I'd say send the signal when wal buffers are more than X% full (maybe >> half). The suggestion to send it when wrapping around at the end of the >> array is not quite right, because that's an arbitrary condition that's >> not related to how much work there is for the walwriter to do. It >> should be cheap to check for this while advancing to a new wal buffer. > I think we need to put the calculation and SetLatch() after we release > WALInsertLock, so as to avoid adding contention. Yeah, I agree with putting the actual SetLatch call after we release the lock ... but doesn't the calculation need to be done while we're still holding it? Otherwise it'd be using potentially-inconsistent values. 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
Re: [HACKERS] Reduced power consumption in WAL Writer process
On Fri, Jul 15, 2011 at 6:33 PM, Tom Lane wrote: > Robert Haas writes: >> On Jul 15, 2011, at 8:55 AM, Simon Riggs wrote: >>> The only difference is how bulk write operations are handled. As long >>> as we wake WALWriter before wal_buffers fills then we'll be good. >>> Wakeup once per wal buffer is too much. I agree we should measure this >>> to check how frequently wakeups are required for bulk ops. > >> Yeah. The trick is to get the wake-ups to be frequent enough without >> adding too much latency to the backends that have to perform them. Off-hand, >> I don't have a good feeling for how hard that will be. > > I'd say send the signal when wal buffers are more than X% full (maybe > half). The suggestion to send it when wrapping around at the end of the > array is not quite right, because that's an arbitrary condition that's > not related to how much work there is for the walwriter to do. It > should be cheap to check for this while advancing to a new wal buffer. Yes, I was trying to go too simple. I think we need to put the calculation and SetLatch() after we release WALInsertLock, so as to avoid adding contention. -- Simon Riggs 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
Re: [HACKERS] Reduced power consumption in WAL Writer process
Robert Haas writes: > On Jul 15, 2011, at 8:55 AM, Simon Riggs wrote: >> The only difference is how bulk write operations are handled. As long >> as we wake WALWriter before wal_buffers fills then we'll be good. >> Wakeup once per wal buffer is too much. I agree we should measure this >> to check how frequently wakeups are required for bulk ops. > Yeah. The trick is to get the wake-ups to be frequent enough without adding > too much latency to the backends that have to perform them. Off-hand, I don't > have a good feeling for how hard that will be. I'd say send the signal when wal buffers are more than X% full (maybe half). The suggestion to send it when wrapping around at the end of the array is not quite right, because that's an arbitrary condition that's not related to how much work there is for the walwriter to do. It should be cheap to check for this while advancing to a new wal buffer. 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
Re: [HACKERS] Reduced power consumption in WAL Writer process
On Jul 15, 2011, at 8:55 AM, Simon Riggs wrote: > On Fri, Jul 15, 2011 at 2:29 PM, Robert Haas wrote: > >> If the primary goal here is to reduce power consumption, another option >> would be to keep the regular wake-ups most of the time but have some >> mechanism for putting the process to sleep until wakened when no activity >> happens for a certain period of time - say, 10 cycles. I'm not at all sure >> that's better, but it would be less of a change to the existing behavior. > > Now we have them, latches seem the best approach because they (mostly) > avoid heuristics. That's my feeling as well. > This proposal works same or better for async transactions. Right. I would say probably better. The potential for a reduction in latency here is very appealing. > The only difference is how bulk write operations are handled. As long > as we wake WALWriter before wal_buffers fills then we'll be good. > Wakeup once per wal buffer is too much. I agree we should measure this > to check how frequently wakeups are required for bulk ops. Yeah. The trick is to get the wake-ups to be frequent enough without adding too much latency to the backends that have to perform them. Off-hand, I don't have a good feeling for how hard that will be. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Reduced power consumption in WAL Writer process
Excerpts from Simon Riggs's message of vie jul 15 09:55:40 -0400 2011: > On Fri, Jul 15, 2011 at 2:29 PM, Robert Haas wrote: > > > If the primary goal here is to reduce power consumption, another option > > would be to keep the regular wake-ups most of the time but have some > > mechanism for putting the process to sleep until wakened when no activity > > happens for a certain period of time - say, 10 cycles. I'm not at all sure > > that's better, but it would be less of a change to the existing behavior. > > Now we have them, latches seem the best approach because they (mostly) > avoid heuristics. Yeah, there's no reason for "less of a change" to be a criterion to determine the best way forward. The new tech is clearly a better solution overall, so lets just get rid of the cruft. -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Reduced power consumption in WAL Writer process
On Fri, Jul 15, 2011 at 2:29 PM, Robert Haas wrote: > If the primary goal here is to reduce power consumption, another option > would be to keep the regular wake-ups most of the time but have some > mechanism for putting the process to sleep until wakened when no activity > happens for a certain period of time - say, 10 cycles. I'm not at all sure > that's better, but it would be less of a change to the existing behavior. Now we have them, latches seem the best approach because they (mostly) avoid heuristics. This proposal works same or better for async transactions. The only difference is how bulk write operations are handled. As long as we wake WALWriter before wal_buffers fills then we'll be good. Wakeup once per wal buffer is too much. I agree we should measure this to check how frequently wakeups are required for bulk ops. -- Simon Riggs 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
Re: [HACKERS] Reduced power consumption in WAL Writer process
My instrumentation wasn't that good. I was using powertop 1.13, which apparently goes to great lengths to group processes by various criteria (including process group), but doesn't actually offer the option of seeing that instrumentation per process. I'm using version 1.98 beta 1 as of now, which provides per-process instrumentation - it only works with Kernel versions 2.6.36+ though. The per process breakdown of wake-ups per second is: 248.3 us/s 5.0Process postgres: wal writer process 281.0 us/s 4.9Process postgres: writer process 82.3 us/s1.1Process postgres: autovacuum launcher process 82.3 us/s0.4Process postgres: stats collector process 442.8 us/s 0.15 Process postgres 23.6 us/s0.15 Process postgres -d1 The second column from the left is wake-ups per second. As previously reported and as you see here, there are about 11.5 idle wake-ups per second per cluster. Note that archiving was running when this instrumentation was performed, but the recently-improved archiver wasn't listed at all, presumably because powertop didn't detect any wake-ups in the period of instrumentation, which was 10 or 20 seconds. As you'd expect, the WAL writer is woken up (1 second / wal_writer_delay milliseconds) times per second. On 14 July 2011 11:00, Simon Riggs wrote: >> That was my first though too - but I wonder if that's too aggressive? A >> backend that does for example a large bulk load will cycle through the >> buffers real quick. It seems like a bad idea to wake up walwriter between >> each buffer in that case. Then again, setting a latch that's already set is >> cheap, so maybe it works fine in practice. >> >> Maybe it would be better to do it less frequently, say, every time you >> switch to new WAL segment. Or every 10 buffers or something like that. > > Yes, that roughly what I'm saying. When nextidx == 0 is just after we > wrapped wal_buffers, i.e. we only wake up the WALWriter every > wal_buffers pages. I'll work towards that implementation. -- Peter Geoghegan http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training and Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Reduced power consumption in WAL Writer process
On Jul 14, 2011, at 4:42 AM, Simon Riggs wrote: > On Thu, Jul 14, 2011 at 9:57 AM, Fujii Masao wrote: > >> Currently walwriter might write out the WAL before a transaction commits. >> IOW, walwriter tries to write out the WAL in wal_buffers in every wakeups. >> This might be useful for long transaction which generates lots of WAL >> records before commit. So we should call SetLatch() in XLogInsert() instead >> of RecordTransactionCommit()? Though I'm not sure how much walwriter >> improves the performance of synchronous commit case.. > > Yeh, we did previously have a heuristic to write out the WAL when it > was more than half full. Not sure I want to put exactly that code back > into such a busy code path. > > I suggest that we set latch every time the wal buffers wrap. > > So at the bottom of AdvanceXLInsertBuffer(), if nextidx == 0 then > SetLatch on the WALWriter. > > That's a simple test and we only check it if we're switch WAL buffer page. Seems reasonable at first blush, but I worry that changing the algorithm here could change performance in somewhat unpredictable ways. Some of those might be improvements, but I think some careful measurement would be in order. If the primary goal here is to reduce power consumption, another option would be to keep the regular wake-ups most of the time but have some mechanism for putting the process to sleep until wakened when no activity happens for a certain period of time - say, 10 cycles. I'm not at all sure that's better, but it would be less of a change to the existing behavior. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Reduced power consumption in WAL Writer process
On Wed, Jul 13, 2011 at 10:56 PM, Peter Geoghegan wrote: > Attached is patch for the WAL writer that removes its tight polling > loop (which probably doesn't get hit often in practice, as we just > sleep if wal_writer_delay is under a second), and, at least > potentially, reduces power consumption when idle by using a latch. > > I will break all remaining power consumption work down into > per-auxiliary process patches. I think that this is appropriate - if > we hit a snag on one of the processes, there is no need to have that > hold up everything. > > I've commented that we handle all expected signals, and therefore we > shouldn't worry about having timeout invalidated by signals, just as > with the archiver. Previously, we didn't even worry about Postmaster > death within the tight polling loop, presumably because > wal_writer_delay is typically small enough to avoid that being a > problem. I thought that WL_POSTMASTER_DEATH might be superfluous here, > but then again there is a codepath specifically for the case where > wal_writer_delay exceeds one second, so it is included in this initial > version. > > Comments? ISTM that this in itself isn't enough to reduce power consumption. Currently the only people that use WALWriter are asynchronous commits, so we should include within RecordTransactionCommit() a SetLatch() command for the WALWriter. That way we can have WALWriter sleep until its needed. -- Simon Riggs 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
Re: [HACKERS] Reduced power consumption in WAL Writer process
On Thu, Jul 14, 2011 at 10:53 AM, Heikki Linnakangas wrote: > On 14.07.2011 12:42, Simon Riggs wrote: >> >> On Thu, Jul 14, 2011 at 9:57 AM, Fujii Masao >> wrote: >> >>> Currently walwriter might write out the WAL before a transaction commits. >>> IOW, walwriter tries to write out the WAL in wal_buffers in every >>> wakeups. >>> This might be useful for long transaction which generates lots of WAL >>> records before commit. So we should call SetLatch() in XLogInsert() >>> instead >>> of RecordTransactionCommit()? Though I'm not sure how much walwriter >>> improves the performance of synchronous commit case.. >> >> Yeh, we did previously have a heuristic to write out the WAL when it >> was more than half full. Not sure I want to put exactly that code back >> into such a busy code path. >> >> I suggest that we set latch every time the wal buffers wrap. >> >> So at the bottom of AdvanceXLInsertBuffer(), if nextidx == 0 then >> SetLatch on the WALWriter. >> >> That's a simple test and we only check it if we're switch WAL buffer page. > > That was my first though too - but I wonder if that's too aggressive? A > backend that does for example a large bulk load will cycle through the > buffers real quick. It seems like a bad idea to wake up walwriter between > each buffer in that case. Then again, setting a latch that's already set is > cheap, so maybe it works fine in practice. > > Maybe it would be better to do it less frequently, say, every time you > switch to new WAL segment. Or every 10 buffers or something like that. Yes, that roughly what I'm saying. When nextidx == 0 is just after we wrapped wal_buffers, i.e. we only wake up the WALWriter every wal_buffers pages. -- Simon Riggs 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
Re: [HACKERS] Reduced power consumption in WAL Writer process
On 14.07.2011 12:42, Simon Riggs wrote: On Thu, Jul 14, 2011 at 9:57 AM, Fujii Masao wrote: Currently walwriter might write out the WAL before a transaction commits. IOW, walwriter tries to write out the WAL in wal_buffers in every wakeups. This might be useful for long transaction which generates lots of WAL records before commit. So we should call SetLatch() in XLogInsert() instead of RecordTransactionCommit()? Though I'm not sure how much walwriter improves the performance of synchronous commit case.. Yeh, we did previously have a heuristic to write out the WAL when it was more than half full. Not sure I want to put exactly that code back into such a busy code path. I suggest that we set latch every time the wal buffers wrap. So at the bottom of AdvanceXLInsertBuffer(), if nextidx == 0 then SetLatch on the WALWriter. That's a simple test and we only check it if we're switch WAL buffer page. That was my first though too - but I wonder if that's too aggressive? A backend that does for example a large bulk load will cycle through the buffers real quick. It seems like a bad idea to wake up walwriter between each buffer in that case. Then again, setting a latch that's already set is cheap, so maybe it works fine in practice. Maybe it would be better to do it less frequently, say, every time you switch to new WAL segment. Or every 10 buffers or something like that. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Reduced power consumption in WAL Writer process
On Thu, Jul 14, 2011 at 9:57 AM, Fujii Masao wrote: > Currently walwriter might write out the WAL before a transaction commits. > IOW, walwriter tries to write out the WAL in wal_buffers in every wakeups. > This might be useful for long transaction which generates lots of WAL > records before commit. So we should call SetLatch() in XLogInsert() instead > of RecordTransactionCommit()? Though I'm not sure how much walwriter > improves the performance of synchronous commit case.. Yeh, we did previously have a heuristic to write out the WAL when it was more than half full. Not sure I want to put exactly that code back into such a busy code path. I suggest that we set latch every time the wal buffers wrap. So at the bottom of AdvanceXLInsertBuffer(), if nextidx == 0 then SetLatch on the WALWriter. That's a simple test and we only check it if we're switch WAL buffer page. -- Simon Riggs 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
Re: [HACKERS] Reduced power consumption in WAL Writer process
On Thu, Jul 14, 2011 at 5:39 PM, Simon Riggs wrote: > On Wed, Jul 13, 2011 at 10:56 PM, Peter Geoghegan > wrote: >> Attached is patch for the WAL writer that removes its tight polling >> loop (which probably doesn't get hit often in practice, as we just >> sleep if wal_writer_delay is under a second), and, at least >> potentially, reduces power consumption when idle by using a latch. >> >> I will break all remaining power consumption work down into >> per-auxiliary process patches. I think that this is appropriate - if >> we hit a snag on one of the processes, there is no need to have that >> hold up everything. >> >> I've commented that we handle all expected signals, and therefore we >> shouldn't worry about having timeout invalidated by signals, just as >> with the archiver. Previously, we didn't even worry about Postmaster >> death within the tight polling loop, presumably because >> wal_writer_delay is typically small enough to avoid that being a >> problem. I thought that WL_POSTMASTER_DEATH might be superfluous here, >> but then again there is a codepath specifically for the case where >> wal_writer_delay exceeds one second, so it is included in this initial >> version. >> >> Comments? > > ISTM that this in itself isn't enough to reduce power consumption. > > Currently the only people that use WALWriter are asynchronous commits, > so we should include within RecordTransactionCommit() a SetLatch() > command for the WALWriter. > > That way we can have WALWriter sleep until its needed. +1 Currently walwriter might write out the WAL before a transaction commits. IOW, walwriter tries to write out the WAL in wal_buffers in every wakeups. This might be useful for long transaction which generates lots of WAL records before commit. So we should call SetLatch() in XLogInsert() instead of RecordTransactionCommit()? Though I'm not sure how much walwriter improves the performance of synchronous commit case.. Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Reduced power consumption in WAL Writer process
Attached is patch for the WAL writer that removes its tight polling
loop (which probably doesn't get hit often in practice, as we just
sleep if wal_writer_delay is under a second), and, at least
potentially, reduces power consumption when idle by using a latch.
I will break all remaining power consumption work down into
per-auxiliary process patches. I think that this is appropriate - if
we hit a snag on one of the processes, there is no need to have that
hold up everything.
I've commented that we handle all expected signals, and therefore we
shouldn't worry about having timeout invalidated by signals, just as
with the archiver. Previously, we didn't even worry about Postmaster
death within the tight polling loop, presumably because
wal_writer_delay is typically small enough to avoid that being a
problem. I thought that WL_POSTMASTER_DEATH might be superfluous here,
but then again there is a codepath specifically for the case where
wal_writer_delay exceeds one second, so it is included in this initial
version.
Comments?
--
Peter Geoghegan http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and Services
diff --git a/src/backend/postmaster/walwriter.c b/src/backend/postmaster/walwriter.c
index 1411677..2027dd7 100644
--- a/src/backend/postmaster/walwriter.c
+++ b/src/backend/postmaster/walwriter.c
@@ -52,6 +52,7 @@
#include "storage/bufmgr.h"
#include "storage/fd.h"
#include "storage/ipc.h"
+#include "storage/latch.h"
#include "storage/lwlock.h"
#include "storage/pmsignal.h"
#include "storage/smgr.h"
@@ -79,7 +80,11 @@ static void WalShutdownHandler(SIGNAL_ARGS);
/*
- * Main entry point for walwriter process
+ * Latch used by signal handlers to wake up the sleep in the main loop.
+ */
+static Latch mainloop_latch;
+
+/* Main entry point for walwriter process
*
* This is invoked from BootstrapMain, which has already created the basic
* execution environment, but not enabled signals yet.
@@ -217,12 +222,16 @@ WalWriterMain(void)
PG_SETMASK(&UnBlockSig);
/*
+ * Initialize latch used in loop below
+ */
+ InitLatch(&mainloop_latch);
+
+ /*
* Loop forever
*/
for (;;)
{
- long udelay;
-
+ ResetLatch(&mainloop_latch);
/*
* Emergency bailout if postmaster has died. This is to avoid the
* necessity for manual cleanup of all postmaster children.
@@ -249,20 +258,8 @@ WalWriterMain(void)
*/
XLogBackgroundFlush();
- /*
- * Delay until time to do something more, but fall out of delay
- * reasonably quickly if signaled.
- */
- udelay = WalWriterDelay * 1000L;
- while (udelay > 99L)
- {
- if (got_SIGHUP || shutdown_requested)
-break;
- pg_usleep(100L);
- udelay -= 100L;
- }
- if (!(got_SIGHUP || shutdown_requested))
- pg_usleep(udelay);
+ /* We handle all expected signals, so WalWriterDelay timeout won't be invalidated */
+ WaitLatch(&mainloop_latch, WL_LATCH_SET | WL_TIMEOUT | WL_POSTMASTER_DEATH, WalWriterDelay * 1000L);
}
}
@@ -309,6 +306,8 @@ static void
WalSigHupHandler(SIGNAL_ARGS)
{
got_SIGHUP = true;
+ /* let the waiting loop iterate */
+ SetLatch(&mainloop_latch);
}
/* SIGTERM: set flag to exit normally */
@@ -316,4 +315,6 @@ static void
WalShutdownHandler(SIGNAL_ARGS)
{
shutdown_requested = true;
+ /* let the waiting loop iterate */
+ SetLatch(&mainloop_latch);
}
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
