Thread: Should commit_delay be PGC_SIGHUP?
I realize that this isn't terribly critical, but I'd like to suggest that commit_delay be made PGC_SIGHUP on 9.3 (it's currently PGC_USERSET). It's not that a poorly chosen commit_delay setting has the potential to adversely affect other backends where the setting *has* been set in those other backends in a suitable way - the same thing can surely be said for work_mem. It just seems to me that commit_delay is now something that's intended to work at the cluster granularity, and as such it seems like almost a misrepresentation to make it PGC_USERSET. The fact is that whichever backend happens to end up becoming the group commit leader from one XLogFlush() call to the next is, for all practical purposes, unpredictable. You cannot reasonably hope to avoid a delay within an important transaction that needs to prioritize keeping its own latency low over total cluster throughput. If you set commit_delay to 0 in your important transaction with this is mind, your chances of becoming the group commit leader and avoiding the delay are slim to almost none. Much more often than not, the important transaction will end up becoming a group commit follower, and it'll still spend a significant fraction of commit_delay (about 1/2, on average) blocking on LWLockAcquireOrWait(). -- Peter Geoghegan
On 20 March 2013 21:50, Peter Geoghegan <pg@heroku.com> wrote: > I realize that this isn't terribly critical, but I'd like to suggest > that commit_delay be made PGC_SIGHUP on 9.3 (it's currently > PGC_USERSET). It's not that a poorly chosen commit_delay setting has > the potential to adversely affect other backends where the setting > *has* been set in those other backends in a suitable way - the same > thing can surely be said for work_mem. It just seems to me that > commit_delay is now something that's intended to work at the cluster > granularity, and as such it seems like almost a misrepresentation to > make it PGC_USERSET. > > The fact is that whichever backend happens to end up becoming the > group commit leader from one XLogFlush() call to the next is, for all > practical purposes, unpredictable. You cannot reasonably hope to avoid > a delay within an important transaction that needs to prioritize > keeping its own latency low over total cluster throughput. If you set > commit_delay to 0 in your important transaction with this is mind, > your chances of becoming the group commit leader and avoiding the > delay are slim to almost none. Much more often than not, the important > transaction will end up becoming a group commit follower, and it'll > still spend a significant fraction of commit_delay (about 1/2, on > average) blocking on LWLockAcquireOrWait(). +1 -- Simon Riggs http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
On Wed, Mar 20, 2013 at 5:50 PM, Peter Geoghegan <pg@heroku.com> wrote: > I realize that this isn't terribly critical, but I'd like to suggest > that commit_delay be made PGC_SIGHUP on 9.3 (it's currently > PGC_USERSET). It's not that a poorly chosen commit_delay setting has > the potential to adversely affect other backends where the setting > *has* been set in those other backends in a suitable way - the same > thing can surely be said for work_mem. It just seems to me that > commit_delay is now something that's intended to work at the cluster > granularity, and as such it seems like almost a misrepresentation to > make it PGC_USERSET. > > The fact is that whichever backend happens to end up becoming the > group commit leader from one XLogFlush() call to the next is, for all > practical purposes, unpredictable. You cannot reasonably hope to avoid > a delay within an important transaction that needs to prioritize > keeping its own latency low over total cluster throughput. If you set > commit_delay to 0 in your important transaction with this is mind, > your chances of becoming the group commit leader and avoiding the > delay are slim to almost none. Much more often than not, the important > transaction will end up becoming a group commit follower, and it'll > still spend a significant fraction of commit_delay (about 1/2, on > average) blocking on LWLockAcquireOrWait(). This may be true, but so what? We don't generally restrict changing GUC settings on the grounds that people probably won't wish to do so because it isn't useful. We restrict it in situations where it is not technically possible or is liable to be harmful. I'm of the opinion that we should try to keep as many things PGC_USERSET as we possibly can. It makes life easier for DBAs. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 21 March 2013 18:27, Robert Haas <robertmhaas@gmail.com> wrote: > This may be true, but so what? We don't generally restrict changing > GUC settings on the grounds that people probably won't wish to do so > because it isn't useful. We restrict it in situations where it is not > technically possible or is liable to be harmful. > > I'm of the opinion that we should try to keep as many things > PGC_USERSET as we possibly can. It makes life easier for DBAs. Only one setting will be best for the whole cluster, so neither the user nor the DBA gains if a user sets this to a different value than the one that has been determined to be optimal. Since we wait while holding the lock it is actually harmful to everyone if anybody sets a stupid value and might even be considered a denial of service attack. So there is a very good reason to make this SIGHUP, not just a whim. -- Simon Riggs http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
On Thu, Mar 21, 2013 at 6:27 PM, Robert Haas <robertmhaas@gmail.com> wrote: > This may be true, but so what? We don't generally restrict changing > GUC settings on the grounds that people probably won't wish to do so > because it isn't useful. We restrict it in situations where it is not > technically possible or is liable to be harmful. Sure, but that isn't what I'm concerned about. I'm concerned about people being lulled into a false sense of security about setting commit_delay to 0 locally. If they do that, their actual additional delay at commit time may well be only marginally less than the full commit_delay, and will only rarely actually be 0. -- Peter Geoghegan
On Wed, Mar 20, 2013 at 09:50:55PM +0000, Peter Geoghegan wrote: > The fact is that whichever backend happens to end up becoming the > group commit leader from one XLogFlush() call to the next is, for all > practical purposes, unpredictable. You cannot reasonably hope to avoid > a delay within an important transaction that needs to prioritize > keeping its own latency low over total cluster throughput. If you set > commit_delay to 0 in your important transaction with this is mind, > your chances of becoming the group commit leader and avoiding the > delay are slim to almost none. Much more often than not, the important > transaction will end up becoming a group commit follower, and it'll > still spend a significant fraction of commit_delay (about 1/2, on > average) blocking on LWLockAcquireOrWait(). I acknowledge that "SET commit_delay = 0" does not usefully reduce latency for a transaction running on a system subject to a uniform, high commit rate. It is useful for other things. Suppose you have a low-concurrency system with commit_delay=0 in postgresql.conf, but you occasionally spin up a parallel task that benefits from nonzero commit_delay. Changing commit_delay in the task's sessions is a decent approximation of, and more convenient than, temporarily modifying postgresql.conf. -- Noah Misch EnterpriseDB http://www.enterprisedb.com
Simon Riggs <simon@2ndQuadrant.com> writes:
> Only one setting will be best for the whole cluster, so neither the
> user nor the DBA gains if a user sets this to a different value than
> the one that has been determined to be optimal.
> Since we wait while holding the lock it is actually harmful to
> everyone if anybody sets a stupid value and might even be considered a
> denial of service attack.
> So there is a very good reason to make this SIGHUP, not just a whim.
Hmm. If a malicious user could hurt performance for other sessions with
a bad setting of commit_delay, then USERSET is clearly a bad idea.
But it still seems like it could be SUSET rather than SIGHUP.
regards, tom lane
On 22 March 2013 02:14, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Simon Riggs <simon@2ndQuadrant.com> writes: >> Only one setting will be best for the whole cluster, so neither the >> user nor the DBA gains if a user sets this to a different value than >> the one that has been determined to be optimal. > >> Since we wait while holding the lock it is actually harmful to >> everyone if anybody sets a stupid value and might even be considered a >> denial of service attack. > >> So there is a very good reason to make this SIGHUP, not just a whim. > > Hmm. If a malicious user could hurt performance for other sessions with > a bad setting of commit_delay, then USERSET is clearly a bad idea. > But it still seems like it could be SUSET rather than SIGHUP. Agreed; everybody gets what they want. Committed. -- Simon Riggs http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
On Fri, Mar 22, 2013 at 8:06 AM, Simon Riggs <simon@2ndquadrant.com> wrote: >> Hmm. If a malicious user could hurt performance for other sessions with >> a bad setting of commit_delay, then USERSET is clearly a bad idea. >> But it still seems like it could be SUSET rather than SIGHUP. > > Agreed; everybody gets what they want. Committed. This is fine with me, too, and I agree that it's warranted... but your commit message supposes that this behavior is new in 9.3, and I think it dates to 9.2. I'm not inclined to think the issue is serious enough to back-patch (and risk breaking current installations) but I thought that it worth mentioning.... -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Fri, Mar 22, 2013 at 12:42 PM, Robert Haas <robertmhaas@gmail.com> wrote: > This is fine with me, too, and I agree that it's warranted... but your > commit message supposes that this behavior is new in 9.3, and I think > it dates to 9.2. No, it doesn't. It just missed the deadline for 9.2. I'm happy enough to have the setting be PGC_SUSET, since that more or less conveys that commit_delay isn't something that is sensible to set dynamically. -- Peter Geoghegan