Horton
Menu

Thread

  1. Re: Uh, I change my mind about commit_delay + commit_siblings (sort of)

    Kevin Grittner <kevin.grittner@wicourts.gov> — 2012-06-02T14:44:27Z

    Tom Lane  wrote:
> Simon Riggs  writes:
>> On 31 May 2012 15:00, Tom Lane  wrote:
>>> If we want to finish the beta cycle in a reasonable time period
>>> and get back to actual development, we have to refrain from
>>> adding more possibly-destabilizing development work to 9.2. And
>>> that is what this is.
> 
>> In what way is it possibly destabilising?
> 
> I'm prepared to believe that it only affects performance, but it
> could be destabilizing to that. It needs proper review and testing,
> and the next CF is the right environment for that to happen.
 
+1
 
This is not a bug fix or even a fix for a performance regression. 
The train has left the station; the next one will be along shortly.
 
-Kevin

  2. Re: Uh, I change my mind about commit_delay + commit_siblings (sort of)

    Robert Haas <robertmhaas@gmail.com> — 2012-06-27T22:01:08Z

    On Sat, Jun 2, 2012 at 10:44 AM, Kevin Grittner
<Kevin.Grittner@wicourts.gov> wrote:
> Tom Lane  wrote:
>> Simon Riggs  writes:
>>> On 31 May 2012 15:00, Tom Lane  wrote:
>>>> If we want to finish the beta cycle in a reasonable time period
>>>> and get back to actual development, we have to refrain from
>>>> adding more possibly-destabilizing development work to 9.2. And
>>>> that is what this is.
>>
>>> In what way is it possibly destabilising?
>>
>> I'm prepared to believe that it only affects performance, but it
>> could be destabilizing to that. It needs proper review and testing,
>> and the next CF is the right environment for that to happen.
>
> +1
>
> This is not a bug fix or even a fix for a performance regression.
> The train has left the station; the next one will be along shortly.

And here it is.  There are a couple of outstanding issues here upon
which it would be helpful to get input from a few more people:

1. Are there any call sites from which this should be disabled, either
because the optimization won't help, or because the caller is holding
a lock that we don't want them sitting on for a moment longer than
necessary?  I think the worst case is that we're doing something like
splitting the root page of a btree, and now nobody can walk the btree
until the flush finishes, and here we are doing an "unnecessary"
sleep.  I am not sure where I can construct a workload where this is a
real as opposed to a theoretical problem, though.  So maybe we should
just not worry about it.  It suffices for this to be an improvement
over the status quo; it doesn't have to be perfect.  Thoughts?

2. Should we rename the GUCs, since this patch will cause them to
control WAL flush in general, as opposed to commit specifically?
Peter Geoghegan and Simon were arguing that we should retitle it to
group_commit_delay rather than just commit_delay, but that doesn't
seem to be much of an improvement in describing what the new behavior
will actually be, and I am doubtful that it is worth creating a naming
incompatibility with previous releases for a cosmetic change.  I
suggested wal_flush_delay, but there's no consensus on that.
Opinions?

Also, I think I see a straightforward, if minor, improvement.   Right
now, the patch has this:

         * Sleep before flush! By adding a delay here, we may give further
         * backends the opportunity to join the backlog of group commit
         * followers; this can significantly improve transaction throughput, at
         * the risk of increasing transaction latency.
         *
         * We do not sleep if enableFsync is not turned on, nor if there are
         * fewer than CommitSiblings other backends with active transactions.
         */
        if (CommitDelay > 0 && enableFsync &&
            MinimumActiveBackends(CommitSiblings))
            pg_usleep(CommitDelay);

        /* Got the lock */
        LogwrtResult = XLogCtl->LogwrtResult;
        if (!XLByteLE(record, LogwrtResult.Flush))
        {
            /* try to write/flush later additions to XLOG as well */
            if (LWLockConditionalAcquire(WALInsertLock, LW_EXCLUSIVE))

The XLByteLE test four lines from the bottom should happen before we
consider whether to sleep, because it's possible that we'll discover
that our flush request has already been satisfied and exit without
doing anything, in which case the sleep is a waste.  The attached
version adjusts the logic accordingly.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

  3. Re: Uh, I change my mind about commit_delay + commit_siblings (sort of)

    Peter Geoghegan <peter@2ndquadrant.com> — 2012-06-28T00:15:56Z

    On 27 June 2012 23:01, Robert Haas <robertmhaas@gmail.com> wrote:
> 1. Are there any call sites from which this should be disabled, either
> because the optimization won't help, or because the caller is holding
> a lock that we don't want them sitting on for a moment longer than
> necessary?  I think the worst case is that we're doing something like
> splitting the root page of a btree, and now nobody can walk the btree
> until the flush finishes, and here we are doing an "unnecessary"
> sleep.  I am not sure where I can construct a workload where this is a
> real as opposed to a theoretical problem, though.  So maybe we should
> just not worry about it.  It suffices for this to be an improvement
> over the status quo; it doesn't have to be perfect.  Thoughts?

I also find it hard to believe this is a concern. As I've said before
though, it hasn't been adequately explained just how you're supposed
to skip the delay if you're in this situation. It's highly unlikely
that you'll be the one doing the delaying anyway. Skipping the delay
iff you're the leader when this might possibly be a problem will only
*conceivably* be effective in a tiny minority of cases, so this seems
to make little sense to me.

Look at the original benchmark - there are latency numbers there too.
The patch actually performs slightly better than baseline in terms of
latency (worst and average cases) as well as throughput. To get back
to bus analogies again, if people start treating buses like Taxis, you
get slightly better minimum latency figures (not included in any
benchmark performed), because one or two people immediately
high-jacked a bus and left on an hour long journey rather than waiting
another 15 minutes for more passengers to come along. That doesn't
seem like it should be something to concern ourselves with. More to
the point, I think that there are "laws of physics" reasons why these
backends that might be particularly adversely affected by a delay (due
to some eventuality like the one you described) cannot *really* avoid
a delay. Of course this isn't an absolute, and certainly won't apply
when there are relatively few clients, when the delay really is
useless, but then we trust the dba to set commit_siblings (and indeed
commit_delay) correctly, and it's hardly that big of a deal, since
it's only typically a fraction of a single fsync() longer at worst -
certainly not a total wait that is longer than the total wait the
backend could reasonably expect to have.

Incidentally, I'm guessing someone is going to come up with a
rule-of-thumb for setting commit_delay that sounds something like
"half the latency of your wal_sync_method as reported by
pg_test_fsync()".

> 2. Should we rename the GUCs, since this patch will cause them to
> control WAL flush in general, as opposed to commit specifically?
> Peter Geoghegan and Simon were arguing that we should retitle it to
> group_commit_delay rather than just commit_delay, but that doesn't
> seem to be much of an improvement in describing what the new behavior
> will actually be, and I am doubtful that it is worth creating a naming
> incompatibility with previous releases for a cosmetic change.  I
> suggested wal_flush_delay, but there's no consensus on that.
> Opinions?

My main problem with the existing name is that every single article on
commit_delay since it was introduce over ten years ago has been on
balance very negative. Greg Smith's book, the docs, my talk at PgCon,
and any other material in existence about commit_delay that I'm aware
of says the same thing.

commit_delay in master is essentially the same now as it was in 2000;
it's just that commit_siblings is a bit smarter, in that it is now
based on number of active transactions specifically.

Now, based on the fact that the doc changes concerning the practical
details of setting commit_delay survived your editorialisations, I
take it that you accept my view that this is going to fundamentally
alter the advice that surrounds commit_delay, from "just don't touch
it" to something like "this is something that you should probably
consider, that may be very compelling in certain situations".

Let's not shoot ourselves in the foot by keeping the old, disreputable
name. You felt that wal_flush_delay more accurately reflected what the
new setting actually does. You were right, and I'd be perfectly happy
to go along with that.

> The XLByteLE test four lines from the bottom should happen before we
> consider whether to sleep, because it's possible that we'll discover
> that our flush request has already been satisfied and exit without
> doing anything, in which case the sleep is a waste.  The attached
> version adjusts the logic accordingly.

Seems like a minor point, unlikely to make any appreciable difference,
but there's no reason to think that it will have any negative effect
compared to what I've proposed either, so that's fine with me.

-- 
Peter Geoghegan       http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and Services

  4. Re: Uh, I change my mind about commit_delay + commit_siblings (sort of)

    Simon Riggs <simon@2ndquadrant.com> — 2012-06-28T08:21:04Z

    On 27 June 2012 23:01, Robert Haas <robertmhaas@gmail.com> wrote:

> 1. Are there any call sites from which this should be disabled, either
> because the optimization won't help, or because the caller is holding
> a lock that we don't want them sitting on for a moment longer than
> necessary?  I think the worst case is that we're doing something like
> splitting the root page of a btree, and now nobody can walk the btree
> until the flush finishes, and here we are doing an "unnecessary"
> sleep.  I am not sure where I can construct a workload where this is a
> real as opposed to a theoretical problem, though.  So maybe we should
> just not worry about it.  It suffices for this to be an improvement
> over the status quo; it doesn't have to be perfect.  Thoughts?

Let's put this in now, without too much fiddling. There is already a
GUC to disable it, so measurements can be made to see if this causes
any problems.

If there are problems, we fix them. We can't second guess everything.

> 2. Should we rename the GUCs, since this patch will cause them to
> control WAL flush in general, as opposed to commit specifically?
> Peter Geoghegan and Simon were arguing that we should retitle it to
> group_commit_delay rather than just commit_delay, but that doesn't
> seem to be much of an improvement in describing what the new behavior
> will actually be, and I am doubtful that it is worth creating a naming
> incompatibility with previous releases for a cosmetic change.  I
> suggested wal_flush_delay, but there's no consensus on that.
> Opinions?

Again, leave the naming of that for later. The idea of a rename came
from yourself, IIRC.

> The XLByteLE test four lines from the bottom should happen before we
> consider whether to sleep, because it's possible that we'll discover
> that our flush request has already been satisfied and exit without
> doing anything, in which case the sleep is a waste.  The attached
> version adjusts the logic accordingly.

I thought there already was a test like that earlier in the flush.

I agree there needs to be one.

-- 
 Simon Riggs                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

  5. Re: Uh, I change my mind about commit_delay + commit_siblings (sort of)

    Robert Haas <robertmhaas@gmail.com> — 2012-06-28T12:18:37Z

    On Thu, Jun 28, 2012 at 4:21 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
> Let's put this in now, without too much fiddling. There is already a
> GUC to disable it, so measurements can be made to see if this causes
> any problems.
>
> If there are problems, we fix them. We can't second guess everything.

Fair enough.

>> 2. Should we rename the GUCs, since this patch will cause them to
>> control WAL flush in general, as opposed to commit specifically?
>> Peter Geoghegan and Simon were arguing that we should retitle it to
>> group_commit_delay rather than just commit_delay, but that doesn't
>> seem to be much of an improvement in describing what the new behavior
>> will actually be, and I am doubtful that it is worth creating a naming
>> incompatibility with previous releases for a cosmetic change.  I
>> suggested wal_flush_delay, but there's no consensus on that.
>> Opinions?
>
> Again, leave the naming of that for later. The idea of a rename came
> from yourself, IIRC.

Deciding on a name is not such a hard thing that leaving it till later
solves any problem.  Let's just make a decision and be done with it.

>> The XLByteLE test four lines from the bottom should happen before we
>> consider whether to sleep, because it's possible that we'll discover
>> that our flush request has already been satisfied and exit without
>> doing anything, in which case the sleep is a waste.  The attached
>> version adjusts the logic accordingly.
>
> I thought there already was a test like that earlier in the flush.
>
> I agree there needs to be one.

There are several of them floating around; the point here is just to
make sure that the sleep is after all of them.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

  6. Re: Uh, I change my mind about commit_delay + commit_siblings (sort of)

    Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> — 2012-06-28T17:57:10Z

    On 28.06.2012 15:18, Robert Haas wrote:
> On Thu, Jun 28, 2012 at 4:21 AM, Simon Riggs<simon@2ndquadrant.com>  wrote:
>>> 2. Should we rename the GUCs, since this patch will cause them to
>>> control WAL flush in general, as opposed to commit specifically?
>>> Peter Geoghegan and Simon were arguing that we should retitle it to
>>> group_commit_delay rather than just commit_delay, but that doesn't
>>> seem to be much of an improvement in describing what the new behavior
>>> will actually be, and I am doubtful that it is worth creating a naming
>>> incompatibility with previous releases for a cosmetic change.  I
>>> suggested wal_flush_delay, but there's no consensus on that.
>>> Opinions?
>>
>> Again, leave the naming of that for later. The idea of a rename came
>> from yourself, IIRC.
>
> Deciding on a name is not such a hard thing that leaving it till later
> solves any problem.  Let's just make a decision and be done with it.

FWIW, I think commit_delay is just fine. In practice, it's mostly 
commits that are affected, anyway. If we try to be more exact, I think 
it's just going to be more confusing to users.

-- 
   Heikki Linnakangas
   EnterpriseDB   http://www.enterprisedb.com

  7. Re: Uh, I change my mind about commit_delay + commit_siblings (sort of)

    Peter Geoghegan <peter@2ndquadrant.com> — 2012-06-28T18:18:57Z

    On 28 June 2012 18:57, Heikki Linnakangas
<heikki.linnakangas@enterprisedb.com> wrote:
> FWIW, I think commit_delay is just fine. In practice, it's mostly commits
> that are affected, anyway. If we try to be more exact, I think it's just
> going to be more confusing to users.

You think it will confuse users less if we start telling them to use
something that we have a very long history of telling them not to use?
The docs say "it is rare that adding delay by increasing this
parameter will actually improve performance". The book PostgreSQL
high-performance says of commit_delay (and commit_siblings) "These are
not effective parameters to tune in most cases. It is extremely
difficult to show any speed-up by adjusting them, and quite easy to
slow every transaction down by tweaking them". Who would be brave
enough to use that in production? Unless you still think these
statements are true, and I don't see why you would, it seems like a
really bad judgement to not change the name.

Is anyone aware of a non-zero commit_delay in the wild today? I
personally am not.

-- 
Peter Geoghegan       http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and Services

  8. Re: Uh, I change my mind about commit_delay + commit_siblings (sort of)

    Kevin Grittner <kevin.grittner@wicourts.gov> — 2012-06-28T18:25:37Z

    Peter Geoghegan <peter@2ndquadrant.com> wrote:
 
> Is anyone aware of a non-zero commit_delay in the wild today? I
> personally am not.
 
http://archives.postgresql.org/pgsql-performance/2011-11/msg00083.php
 
-Kevin

  9. Re: Uh, I change my mind about commit_delay + commit_siblings (sort of)

    Peter Geoghegan <peter@2ndquadrant.com> — 2012-06-28T18:38:35Z

    On 28 June 2012 19:25, Kevin Grittner <Kevin.Grittner@wicourts.gov> wrote:
> Peter Geoghegan <peter@2ndquadrant.com> wrote:
>
>> Is anyone aware of a non-zero commit_delay in the wild today? I
>> personally am not.
>
> http://archives.postgresql.org/pgsql-performance/2011-11/msg00083.php

In that thread, Robert goes on to say to the OP that has set commit_delay:

>On Fri, Nov 4, 2011 at 2:45 PM, Claudio Freire <klaussfreire@gmail.com> wrote:
>> I don't think 1 second can be such a big difference for the bgwriter,
>> but I might be wrong.
>
> Well, the default value is 200 ms.   And I've never before heard of
> anyone tuning it up, except maybe to save on power consumption on a
> system with very low utilization.  Nearly always you want to reduce
> it.
>
>> The wal_writer makes me doubt, though. If logged activity was higher
>> than 8MB/s, then that setting would block it all.
>> I guess I really should lower it.
>
> Here again, you've set it to ten times the default value.  That
> doesn't seem like a good idea.  I would start with the default and
> tune down.

So, let me rephrase my question: Is anyone aware of a non-zero
commit_delay in the wild today with sensible reasoning behind it?

-- 
Peter Geoghegan       http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and Services

  10. Re: Uh, I change my mind about commit_delay + commit_siblings (sort of)

    Robert Haas <robertmhaas@gmail.com> — 2012-06-28T18:55:29Z

    On Thu, Jun 28, 2012 at 2:18 PM, Peter Geoghegan <peter@2ndquadrant.com> wrote:
> You think it will confuse users less if we start telling them to use
> something that we have a very long history of telling them not to use?

I don't buy this line of reasoning at all.  If we're going to rename
the GUC, it should be for accuracy, not PR value.  If we start
renaming something every time we improve it, we're going to go nuts.
We improved lots of things in 9.2; they didn't all get renamed.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

  11. Re: Uh, I change my mind about commit_delay + commit_siblings (sort of)

    Peter Geoghegan <peter@2ndquadrant.com> — 2012-06-28T18:58:15Z

    On 28 June 2012 19:55, Robert Haas <robertmhaas@gmail.com> wrote:
> On Thu, Jun 28, 2012 at 2:18 PM, Peter Geoghegan <peter@2ndquadrant.com> wrote:
>> You think it will confuse users less if we start telling them to use
>> something that we have a very long history of telling them not to use?
>
> I don't buy this line of reasoning at all.  If we're going to rename
> the GUC, it should be for accuracy, not PR value.  If we start
> renaming something every time we improve it, we're going to go nuts.
> We improved lots of things in 9.2; they didn't all get renamed.

That is a false equivalence, and you know it.

Who said anything about PR value? I'm concerned with not confusing users.

-- 
Peter Geoghegan       http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and Services

  12. Re: Uh, I change my mind about commit_delay + commit_siblings (sort of)

    Tom Lane <tgl@sss.pgh.pa.us> — 2012-06-28T19:00:06Z

    Robert Haas <robertmhaas@gmail.com> writes:
> On Thu, Jun 28, 2012 at 2:18 PM, Peter Geoghegan <peter@2ndquadrant.com> wrote:
>> You think it will confuse users less if we start telling them to use
>> something that we have a very long history of telling them not to use?

> I don't buy this line of reasoning at all.  If we're going to rename
> the GUC, it should be for accuracy, not PR value.  If we start
> renaming something every time we improve it, we're going to go nuts.
> We improved lots of things in 9.2; they didn't all get renamed.

See VACUUM FULL for a recent counterexample --- we basically jacked it
up and drove a new implementation underneath, but we didn't change the
name, despite the fact that we were obsoleting a whole lot more folk
knowledge than exists around commit_delay.

Of course, there were application-compatibility reasons not to rename
that command, which wouldn't apply so much to commit_delay.  But still,
we have precedent for expecting that we can fix external documentation
rather than trying to code around whatever it says.

			regards, tom lane

  13. Re: Uh, I change my mind about commit_delay + commit_siblings (sort of)

    Peter Geoghegan <peter@2ndquadrant.com> — 2012-06-28T19:17:53Z

    On 28 June 2012 20:00, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> See VACUUM FULL for a recent counterexample --- we basically jacked it
> up and drove a new implementation underneath, but we didn't change the
> name, despite the fact that we were obsoleting a whole lot more folk
> knowledge than exists around commit_delay.
>
> Of course, there were application-compatibility reasons not to rename
> that command, which wouldn't apply so much to commit_delay.  But still,
> we have precedent for expecting that we can fix external documentation
> rather than trying to code around whatever it says.

I'm sure you're right to some degree. We can rely on some, maybe even
most users to go and learn about these things, or hear about them
somehow. But why should we do it that way, when what I've proposed is
so much simpler, and has no plausible downside?

http://archives.postgresql.org/pgsql-hackers/2005-06/msg01463.php

Here, you say:

""
The issue here is not "is group commit a good idea in the abstract?".
It is "is the commit_delay implementation of the idea worth a dime?"
... and the evidence we have all points to the answer "NO".  We should
not let theoretical arguments blind us to this.
""

What happens when someone reads this, or many other such statements
were made before or since, when they are considering setting
commit_delay now?

The VACUUM FULL implementation is now very close to being nothing more
than a synonym of CLUSTER. The only reason it happened that way was
because it was easier than just deprecating VACUUM FULL. The old
VACUUM FULL actually had something to recommend it. It seems unlikely
that the same thing can be said for commit_delay.

-- 
Peter Geoghegan       http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and Services

  14. Re: Uh, I change my mind about commit_delay + commit_siblings (sort of)

    Robert Haas <robertmhaas@gmail.com> — 2012-06-28T19:55:54Z

    On Thu, Jun 28, 2012 at 2:58 PM, Peter Geoghegan <peter@2ndquadrant.com> wrote:
> On 28 June 2012 19:55, Robert Haas <robertmhaas@gmail.com> wrote:
>> On Thu, Jun 28, 2012 at 2:18 PM, Peter Geoghegan <peter@2ndquadrant.com> wrote:
>>> You think it will confuse users less if we start telling them to use
>>> something that we have a very long history of telling them not to use?
>>
>> I don't buy this line of reasoning at all.  If we're going to rename
>> the GUC, it should be for accuracy, not PR value.  If we start
>> renaming something every time we improve it, we're going to go nuts.
>> We improved lots of things in 9.2; they didn't all get renamed.
>
> That is a false equivalence, and you know it.

A false equivalence between what and what?

> Who said anything about PR value? I'm concerned with not confusing users.

My point here is that we don't normally rename things because they
work better than they used to.  Whether that is called PR value or not
confusing users, we don't normally do it.

We sometimes rename things because they do something *different* than
what they used to do.  But not just because they work better.

Anyway, it seems that no one other than you and I is very excited
about renaming this for whatever reason, so maybe we should leave it
at that.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

  15. Re: Uh, I change my mind about commit_delay + commit_siblings (sort of)

    Peter Geoghegan <peter@2ndquadrant.com> — 2012-06-28T20:32:40Z

    On 28 June 2012 20:55, Robert Haas <robertmhaas@gmail.com> wrote:
>>> I don't buy this line of reasoning at all.  If we're going to rename
>>> the GUC, it should be for accuracy, not PR value.  If we start
>>> renaming something every time we improve it, we're going to go nuts.
>>> We improved lots of things in 9.2; they didn't all get renamed.
>>
>> That is a false equivalence, and you know it.
>
> A false equivalence between what and what?

Changing the name in order to create some buzz about it, or to
highlight an improvement, is quite different to changing the name
because the practical advice surrounding the setting has suddenly
altered, almost diametrically, and after a very long period.

>> Who said anything about PR value? I'm concerned with not confusing users.
>
> My point here is that we don't normally rename things because they
> work better than they used to.  Whether that is called PR value or not
> confusing users, we don't normally do it.

That makes sense. I don't dispute that. We aren't talking about an
incremental improvement here, are we? We're talking about night and
day. That is probably an unprecedented state of affairs, so I don't
see any reason to invoke precedent.

The reason the VACUUM FULL situation isn't at all comparable here is because:

For VACUUM FULL:

User googles "VACUUM FULL", gets something from the 8.* docs, or
something from that era. (This happens more often than not for any
given Postgres term, as you rightly complained about at PgCon).

They see something that says "you should probably use cluster
instead". They do so, and are no worse off for it.

For commit_delay:

User googles commit_delay, and sees something from that same period
that says "you don't want to use this". They naturally assume that
they don't. They lose whatever benefit they might have had from the
new implementation.

> We sometimes rename things because they do something *different* than
> what they used to do.  But not just because they work better.

It does do something different. That's why you proposed changing the name.

> Anyway, it seems that no one other than you and I is very excited
> about renaming this for whatever reason, so maybe we should leave it
> at that.

I think not changing the name is a really bad decision, and I am
personally unhappy about it. I immediately agreed to your proposed
adjustment to the patch without really thinking that it mattered,
because it had no plausible downside, so why not?

That's all I have to say about the matter. If it isn't obvious that
the name should be changed, based on what I've already said, it never
will be.

-- 
Peter Geoghegan       http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and Services

  16. Re: Uh, I change my mind about commit_delay + commit_siblings (sort of)

    Daniel Farina <daniel@heroku.com> — 2012-06-28T21:22:12Z

    On Thu, Jun 28, 2012 at 1:32 PM, Peter Geoghegan <peter@2ndquadrant.com> wrote:
>> Anyway, it seems that no one other than you and I is very excited
>> about renaming this for whatever reason, so maybe we should leave it
>> at that.
>
> I think not changing the name is a really bad decision, and I am
> personally unhappy about it. I immediately agreed to your proposed
> adjustment to the patch without really thinking that it mattered,
> because it had no plausible downside, so why not?
>
> That's all I have to say about the matter. If it isn't obvious that
> the name should be changed, based on what I've already said, it never
> will be.

All in all, I don't think this can be a very productive discussion
unless someone just pitches a equal or better name overall in terms of
conciseness and descriptiveness.  I'd rather optimize for those
attributes.  Old advice is old; that's the nature of the beast.

The one benefit I can think of for name shuffling is avoiding
accidental negation of the optimization when porting Postgres
configuration from older clusters, and even then I don't think the
rename-on-change strategy is a scalable one; a tuning-linting
tool is probably the only scalable option if one is concerned about
making sure that those forward-porting their configurations or
managing multiple postgres versions don't shoot themselves in the
foot.

-- 
fdr

  17. Re: Uh, I change my mind about commit_delay + commit_siblings (sort of)

    Peter Geoghegan <peter@2ndquadrant.com> — 2012-06-28T21:32:09Z

    On 28 June 2012 22:22, Daniel Farina <daniel@heroku.com> wrote:
> All in all, I don't think this can be a very productive discussion
> unless someone just pitches a equal or better name overall in terms of
> conciseness and descriptiveness.  I'd rather optimize for those
> attributes.  Old advice is old; that's the nature of the beast.

Robert suggested wal_flush_delay, which does more accurately describe
what happens now.

Old advice is old, but we frequently go to reasonable lengths to
protect users from making predictable mistakes. The position that we
shouldn't change the name might make sense if there was any downside
to doing so, but there isn't.

-- 
Peter Geoghegan       http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and Services

  18. Re: Uh, I change my mind about commit_delay + commit_siblings (sort of)

    Daniel Farina <daniel@heroku.com> — 2012-06-28T22:01:51Z

    On Thu, Jun 28, 2012 at 2:32 PM, Peter Geoghegan <peter@2ndquadrant.com> wrote:
> On 28 June 2012 22:22, Daniel Farina <daniel@heroku.com> wrote:
>> All in all, I don't think this can be a very productive discussion
>> unless someone just pitches a equal or better name overall in terms of
>> conciseness and descriptiveness.  I'd rather optimize for those
>> attributes.  Old advice is old; that's the nature of the beast.
>
> Robert suggested wal_flush_delay, which does more accurately describe
> what happens now.

Well, I learned something from reading this name, having not followed
the mechanism too closely.  I like it.

-- 
fdr

  19. Re: Uh, I change my mind about commit_delay + commit_siblings (sort of)

    Robert Haas <robertmhaas@gmail.com> — 2012-07-02T14:32:18Z

    On Thu, Jun 28, 2012 at 6:01 PM, Daniel Farina <daniel@heroku.com> wrote:
> On Thu, Jun 28, 2012 at 2:32 PM, Peter Geoghegan <peter@2ndquadrant.com> wrote:
>> On 28 June 2012 22:22, Daniel Farina <daniel@heroku.com> wrote:
>>> All in all, I don't think this can be a very productive discussion
>>> unless someone just pitches a equal or better name overall in terms of
>>> conciseness and descriptiveness.  I'd rather optimize for those
>>> attributes.  Old advice is old; that's the nature of the beast.
>>
>> Robert suggested wal_flush_delay, which does more accurately describe
>> what happens now.
>
> Well, I learned something from reading this name, having not followed
> the mechanism too closely.  I like it.

I've committed this now.  In the absence of a clear consensus to
rename the GUC, I contented myself with a further overhaul of the
documentation, which will hopefully make things clear at least for
people who read the documentation.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

  20. Re: Uh, I change my mind about commit_delay + commit_siblings (sort of)

    Bruce Momjian <bruce@momjian.us> — 2012-07-03T03:28:33Z

    On Thu, Jun 28, 2012 at 08:17:53PM +0100, Peter Geoghegan wrote:
> On 28 June 2012 20:00, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > See VACUUM FULL for a recent counterexample --- we basically jacked it
> > up and drove a new implementation underneath, but we didn't change the
> > name, despite the fact that we were obsoleting a whole lot more folk
> > knowledge than exists around commit_delay.
> >
> > Of course, there were application-compatibility reasons not to rename
> > that command, which wouldn't apply so much to commit_delay.  But still,
> > we have precedent for expecting that we can fix external documentation
> > rather than trying to code around whatever it says.
> 
> I'm sure you're right to some degree. We can rely on some, maybe even
> most users to go and learn about these things, or hear about them
> somehow. But why should we do it that way, when what I've proposed is
> so much simpler, and has no plausible downside?

FYI, the release notes are the big place we should talk about this new,
better behavior.

-- 
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + It's impossible for everything to be true. +