pgsql: Revert "commit_delay" change; just add comment that we don't hav

Started by Bruce Momjianover 13 years ago8 messages
#1Bruce Momjian
bruce@momjian.us

Revert "commit_delay" change; just add comment that we don't have
a microsecond specification.

Branch
------
master

Details
-------
http://git.postgresql.org/pg/commitdiff/03bda4535ee119d3dae7226faebed089925ace7e

Modified Files
--------------
src/backend/utils/misc/guc.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

#2Peter Geoghegan
peter@2ndquadrant.com
In reply to: Bruce Momjian (#1)
Re: pgsql: Revert "commit_delay" change; just add comment that we don't hav

On 14 August 2012 21:26, Bruce Momjian <bruce@momjian.us> wrote:

Revert "commit_delay" change; just add comment that we don't have
a microsecond specification.

I think that if we eventually decide to change the name of
commit_delay for 9.3 (you previously suggested that that might be
revisited), it will be reasonable to have the new GUC in units of
milliseconds. After all, we've already been switching to milliseconds
across various statistics views, including pg_stat_statements.

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

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Geoghegan (#2)
Re: [COMMITTERS] pgsql: Revert "commit_delay" change; just add comment that we don't hav

Peter Geoghegan <peter@2ndquadrant.com> writes:

On 14 August 2012 21:26, Bruce Momjian <bruce@momjian.us> wrote:

Revert "commit_delay" change; just add comment that we don't have
a microsecond specification.

I think that if we eventually decide to change the name of
commit_delay for 9.3 (you previously suggested that that might be
revisited), it will be reasonable to have the new GUC in units of
milliseconds.

Well, the reason why it's like that at all is the thought that values
of less than 1 millisecond might be useful. Are we prepared to suppose
that that is not and never will be true?

regards, tom lane

#4Peter Geoghegan
peter@2ndquadrant.com
In reply to: Tom Lane (#3)
Re: [COMMITTERS] pgsql: Revert "commit_delay" change; just add comment that we don't hav

On 15 August 2012 05:15, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Peter Geoghegan <peter@2ndquadrant.com> writes:

On 14 August 2012 21:26, Bruce Momjian <bruce@momjian.us> wrote:

Revert "commit_delay" change; just add comment that we don't have
a microsecond specification.

I think that if we eventually decide to change the name of
commit_delay for 9.3 (you previously suggested that that might be
revisited), it will be reasonable to have the new GUC in units of
milliseconds.

Well, the reason why it's like that at all is the thought that values
of less than 1 millisecond might be useful. Are we prepared to suppose
that that is not and never will be true?

I think that the need for sub-millisecond granularity had more to do
with historic quirks of our implementation. Slight tweaks accidentally
greatly improved throughput, if only for the synthetic benchmark in
question. I personally have not seen any need for a sub-millisecond
granularity when experimenting with the setting on 9.3-devel. However,
I am not sure that sub-millisecond granularity could never be of any
use, in squeezing the last small increase in throughput made possible
by commit_delay. Importantly, feedback as the GUC is tuned is far more
predictable than it was with the prior implementation, so this does
seem quite unimportant.

Why does commit_delay have to be an integer? Can't we devise a way of
manipulating it in units of milliseconds, but have the internal
representation be a double, as with pg_stat_statements' total_time
column? That would allow very fine tuning of the delay. As I've
outlined, I'm not sure that it's worth supporting such fine-tuning
with the new implementation.

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

#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Geoghegan (#4)
Re: [COMMITTERS] pgsql: Revert "commit_delay" change; just add comment that we don't hav

Peter Geoghegan <peter@2ndquadrant.com> writes:

Why does commit_delay have to be an integer? Can't we devise a way of
manipulating it in units of milliseconds, but have the internal
representation be a double, as with pg_stat_statements' total_time
column?

If you wanted to re-implement all the guc.c logic for supporting
unit-ified values such that it would also work with floats, we could
do that. It seems like way more mechanism than the problem is worth
however.

regards, tom lane

#6Peter Geoghegan
peter@2ndquadrant.com
In reply to: Tom Lane (#5)
Re: [COMMITTERS] pgsql: Revert "commit_delay" change; just add comment that we don't hav

On 15 August 2012 14:39, Tom Lane <tgl@sss.pgh.pa.us> wrote:

If you wanted to re-implement all the guc.c logic for supporting
unit-ified values such that it would also work with floats, we could
do that. It seems like way more mechanism than the problem is worth
however.

Fair enough.

I'm not quite comfortable recommending a switch to milliseconds if
that implies a loss of sub-millisecond granularity. I know that
someone is going to point out that in some particularly benchmark,
they can get another relatively modest increase in throughput (perhaps
2%-3%) by splitting the difference between two adjoining millisecond
integer values. In that scenario, I'd be tempted to point out that
that increase is quite unlikely to carry over to real-world benefits,
because the setting is then right on the cusp of where increasing
commit_delay stops helping throughput and starts hurting it. The
improvement is likely to get lost in the noise in the context of a
real-world application, where for example the actually cost of an
fsync is more variable. I'm just not sure that that's the right
attitude.

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

#7Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Geoghegan (#6)
Re: [COMMITTERS] pgsql: Revert "commit_delay" change; just add comment that we don't hav

Peter Geoghegan <peter@2ndquadrant.com> writes:

I'm not quite comfortable recommending a switch to milliseconds if
that implies a loss of sub-millisecond granularity. I know that
someone is going to point out that in some particularly benchmark,
they can get another relatively modest increase in throughput (perhaps
2%-3%) by splitting the difference between two adjoining millisecond
integer values. In that scenario, I'd be tempted to point out that
that increase is quite unlikely to carry over to real-world benefits,
because the setting is then right on the cusp of where increasing
commit_delay stops helping throughput and starts hurting it. The
improvement is likely to get lost in the noise in the context of a
real-world application, where for example the actually cost of an
fsync is more variable. I'm just not sure that that's the right
attitude.

To me it's more about future-proofing. commit_delay is the only
time-interval setting we've got where reasonable values today are in the
single-digit-millisecond range. So it seems to me not hard to infer
that in a few years sub-millisecond values will be important, whether or
not there's any real argument for them today.

regards, tom lane

#8Greg Smith
greg@2ndQuadrant.com
In reply to: Peter Geoghegan (#6)
Re: Re: [COMMITTERS] pgsql: Revert "commit_delay" change; just add comment that we don't hav

On 08/15/2012 11:41 AM, Peter Geoghegan wrote:

I know that someone is going to point out that in some particularly benchmark,
they can get another relatively modest increase in throughput (perhaps
2%-3%) by splitting the difference between two adjoining millisecond
integer values. In that scenario, I'd be tempted to point out that
that increase is quite unlikely to carry over to real-world benefits,
because the setting is then right on the cusp of where increasing
commit_delay stops helping throughput and starts hurting it.

You guessed right on that. I just responded to your survey over on
pgsql-performance with two cases where older versions found optimal
performance with commit_delay in the <=10 usec range. Those are all in
the BBWC case that I don't think you've been testing much of yet.

I recall Jignesh Shah reported his seeing that was from slightly better
chunking of writes to disk, with a small but measurable drop in disk I/O
operations (such as IOPS) relative to TPS. The average throughput was
no different; the number of *operations* was smaller though. Less 8K
I/O requests, more 16K+ ones. Like a lot of these situations, adding
some latency to every transactions can make them batch better. And that
can unexpectedly boost throughput enough that net latency is actually
faster. It's similar to how adding input queue latency with a pooler,
limiting active connections, can actually make latency better by
increasing efficiency.

On higher-end storage you can reach a point where IOPS gets high enough
that the per-operation overhead becomes a problem, on top of the usual
"is there enough write throughput?" question. I suspect this situation
might even be more common now, given IOPS issues like this are commonly
highlighted when people do SSD reviews.

I still don't know that it's a widely popular situation. But this
particular use case has been one of the more persistent ones arguing to
keep the parameter around until now. Making sub-microsecond resolution
on the parameter go away would effectively trash it just when it might
get even more useful than before.

--
Greg Smith 2ndQuadrant US greg@2ndQuadrant.com Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.com