autovacuum_work_mem

There has recently been considerable discussion around auto-tuning.
Throughout the course of this discussion, I raised the idea of
creating a new GUC to separately control autovacuum's usage of
maintenance_work_mem [1], explaining the rationale in some detail [2].
At the time Magnus seemed to think this a good idea [3].

Attached simple patch adds a new GUC, autovacuum_work_mem, which when
set to a value other than -1 (the default) is preferred by autovacuum
over maintenance_work_mem. All other usage of VACUUM is unaffected.

I won't repeat the rationale for the patch here. Appropriate
documentation changes are included. I don't think it's a problem that
autovacuum_work_mem is kind of similar to vacuum_mem in name.
maintenance_work_mem was last spelt vacuum_mem about 10 years ago.
Enough time has passed that I think it very unlikely that someone
might conflate the two.

I have decided to have the default value of -1 carry, and not have it
automatically take the same value as maintenance_work_mem, because
doing so could create the impression that it needs to be set
explicitly, which of course it does not - this is not the same
situation as exists for wal_buffers. We just check if its set when
going to VACUUM, if VACUUM is requested from a worker process.

It seemed neater to me to create a new flag, so that in principle any
vacuum() code path can request autovacuum_work_mem, rather than having
lazyvacuum.c code call IsAutoVacuumWorkerProcess() for the same
purpose. To date, that's only been done within vacuumlazy.c for things
like logging.

[1] http://www.postgresql.org/message-id/CAM3SWZTr1uu+7KR1ZOuGwcJriw9NVBQdjqyDMRWypEvDFi4a6Q@mail.gmail.com

[2] http://www.postgresql.org/message-id/CAM3SWZTYoD0YCLA-4nRb4S8-UGJyr514aEy+8O6VJQwvbzszGQ@mail.gmail.com

[3] http://www.postgresql.org/message-id/CABUevEzVrd36yeFzYBzad0=r09eqRqNoMwX8r=URikG9DrfUkw@mail.gmail.com

--
Peter Geoghegan

On Sun, Oct 20, 2013 at 2:22 AM, Peter Geoghegan <pg(at)heroku(dot)com> wrote:
> There has recently been considerable discussion around auto-tuning.
> Throughout the course of this discussion, I raised the idea of
> creating a new GUC to separately control autovacuum's usage of
> maintenance_work_mem [1], explaining the rationale in some detail [2].
> At the time Magnus seemed to think this a good idea [3].
>
> Attached simple patch adds a new GUC, autovacuum_work_mem, which when
> set to a value other than -1 (the default) is preferred by autovacuum
> over maintenance_work_mem. All other usage of VACUUM is unaffected.
>
> I won't repeat the rationale for the patch here. Appropriate
> documentation changes are included. I don't think it's a problem that
> autovacuum_work_mem is kind of similar to vacuum_mem in name.
> maintenance_work_mem was last spelt vacuum_mem about 10 years ago.
> Enough time has passed that I think it very unlikely that someone
> might conflate the two.

My rationale for liking the idea is also in those threads :)

And I definitely don't think there is any problem whatsoever in having
a name that's close to something we had 10 years ago. I think it would
be hard enough to make a case for not reusing the *same* name that
long after, but here it's actually different.

> I have decided to have the default value of -1 carry, and not have it
> automatically take the same value as maintenance_work_mem, because
> doing so could create the impression that it needs to be set
> explicitly, which of course it does not - this is not the same
> situation as exists for wal_buffers. We just check if its set when
> going to VACUUM, if VACUUM is requested from a worker process.

It is the same that exists for autovacuum_vacuum_cost_limit and
autovacuum_vacuum_cost_delay, so that makes perfect sense.

> It seemed neater to me to create a new flag, so that in principle any
> vacuum() code path can request autovacuum_work_mem, rather than having
> lazyvacuum.c code call IsAutoVacuumWorkerProcess() for the same
> purpose. To date, that's only been done within vacuumlazy.c for things
> like logging.

Hmm. I'm not entirely sure I agree that that makes it neater :)

We could also look at autovacuum_vacuum_cost_limit etc above, but
those just override what the non-autovac parameters do. But since the
parameter is called maintenance_work_mem in that case, I think that
would make it harder to read.

But I'd suggest just a:
int vac_work_mem = (IsAutoVacuumWorkerProcess() && autovacuum_work_mem
!= -1) ? autovacuum_work_mem : maintenance_work_mem;

and not sending around a boolean flag through a bunch of places when
it really means just the same thing,

Another option would be to just have vac_work_mem actually being the
global,and have it set by the maintenance_work_mem parameter guc, and
then overwritten by the vacuum work mem parameter guc in autovacuum
worker startup. Then you wouldn't put either of those two in the
codepath.

Also, isn't this quite confusing:
+ # Note: autovacuum only prefers autovacuum_work_mem over maintenance_work_mem
+ #autovacuum_work_mem = -1 # min 1MB, or -1 to disable

Where does the "only" come from? And we don't really use the term
"prefers over" anywhere else there. And -1 doesn't actually disable
it. I suggest following the pattern of the other parameters that work
the same way, which would then just be:

#autovacuum_work_mem = -1 # amount of memory for each autovacuum
worker. -1 means use maintenance_work_mem

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

On 10/19/13 8:22 PM, Peter Geoghegan wrote:
> I don't think it's a problem that
> autovacuum_work_mem is kind of similar to vacuum_mem in name.
> maintenance_work_mem was last spelt vacuum_mem about 10 years ago.
> Enough time has passed that I think it very unlikely that someone
> might conflate the two.

What is more confusing, however, is that autovacuum_work_mem looks like
it's work_mem as used by autovacuum, where it's really
maintenance_work_mem as used by autovacuum. So maybe it should be
called autovacuum_maintenance_work_mem.

On Mon, Oct 21, 2013 at 9:36 AM, Peter Eisentraut <peter_e(at)gmx(dot)net> wrote:
> On 10/19/13 8:22 PM, Peter Geoghegan wrote:
>> I don't think it's a problem that
>> autovacuum_work_mem is kind of similar to vacuum_mem in name.
>> maintenance_work_mem was last spelt vacuum_mem about 10 years ago.
>> Enough time has passed that I think it very unlikely that someone
>> might conflate the two.
>
> What is more confusing, however, is that autovacuum_work_mem looks like
> it's work_mem as used by autovacuum, where it's really
> maintenance_work_mem as used by autovacuum. So maybe it should be
> called autovacuum_maintenance_work_mem.

I think I prefer autovacuum_work_mem. I don't think sticking the word
maintenance in there is really adding much in the way of clarity.

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

On Mon, Oct 21, 2013 at 3:42 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Mon, Oct 21, 2013 at 9:36 AM, Peter Eisentraut <peter_e(at)gmx(dot)net> wrote:
>> On 10/19/13 8:22 PM, Peter Geoghegan wrote:
>>> I don't think it's a problem that
>>> autovacuum_work_mem is kind of similar to vacuum_mem in name.
>>> maintenance_work_mem was last spelt vacuum_mem about 10 years ago.
>>> Enough time has passed that I think it very unlikely that someone
>>> might conflate the two.
>>
>> What is more confusing, however, is that autovacuum_work_mem looks like
>> it's work_mem as used by autovacuum, where it's really
>> maintenance_work_mem as used by autovacuum. So maybe it should be
>> called autovacuum_maintenance_work_mem.
>
> I think I prefer autovacuum_work_mem. I don't think sticking the word
> maintenance in there is really adding much in the way of clarity.

+1. If changing at all, then maybe just "autovacuum_mem"? It's not
like there's a different parameter to control a different kind of
memory in autovac, is there?

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

* Magnus Hagander (magnus(at)hagander(dot)net) wrote:
> +1. If changing at all, then maybe just "autovacuum_mem"? It's not
> like there's a different parameter to control a different kind of
> memory in autovac, is there?

+1 on that, for my 2c.

Thanks,

Stephen

On Mon, Oct 21, 2013 at 6:44 AM, Magnus Hagander <magnus(at)hagander(dot)net> wrote:
> +1. If changing at all, then maybe just "autovacuum_mem"?

I would like to stick with autovacuum_work_mem - it reflects the fact
that it's a setting shadowed by maintenance_work_mem, without being
too verbose.

--
Peter Geoghegan

On 19 October 2013 19:22, Peter Geoghegan <pg(at)heroku(dot)com> wrote:

> I won't repeat the rationale for the patch here.

I can't see the problem that this patch is trying to solve. I'm having
trouble understanding when I would use this.

VACUUM uses 6 bytes per dead tuple. And autovacuum regularly removes
dead tuples, limiting their numbers.

In what circumstances will the memory usage from multiple concurrent
VACUUMs become a problem? In those circumstances, reducing
autovacuum_work_mem will cause more passes through indexes, dirtying
more pages and elongating the problem workload.

I agree that multiple concurrent VACUUMs could be a problem but this
doesn't solve that, it just makes things worse.

Freezing doesn't require any memory at all, so wraparound vacuums
won't be controlled by this parameter.

Can we re-state what problem actually is here and discuss how to solve
it. (The reference [2] didn't provide a detailed explanation of the
problem, only the reason why we want a separate parameter).

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

On Sun, Nov 24, 2013 at 9:06 AM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
> VACUUM uses 6 bytes per dead tuple. And autovacuum regularly removes
> dead tuples, limiting their numbers.
>
> In what circumstances will the memory usage from multiple concurrent
> VACUUMs become a problem? In those circumstances, reducing
> autovacuum_work_mem will cause more passes through indexes, dirtying
> more pages and elongating the problem workload.

Yes, of course, but if we presume that the memory for autovacuum
workers to do everything in one pass simply isn't there, it's still
better to do multiple passes. Similarly, it's also sometimes (roughly
speaking) locally suboptimal but globally optimal to do tapesorts in
preference to in-memory quicksorts, even though, as you know, very
frequently tapesort is very considerably slower than quicksort.

Look at the folk wisdom for sizing maintenance_work_mem that is
floating around (for example, take a look at Greg Smith's
recommendations in his book). Setting it within postgresql.conf is
assumed. You can end up with a conservative value because you're
worrying about the worst case. The average case suffers. Especially
since, as you say, autovacuum only uses 6 bytes per tuple, and so
probably isn't all that likely to run out of working memory, making
that worst case (that is, maintenance_work_mem over-allocation by
autovacuum workers) very unlikely.

So on larger Heroku Postgres plans, the generic maintenance_work_mem
is on the low side, and I sometimes have to manually increase it when
I'm doing something like creating a new index. I would like to not
have to do that, and I would like to not require users to be aware of
this issue, especially since external sorting is so much slower.

I am inclined to think that we need an altogether more sophisticated
model, but this is an incremental improvement.

> Can we re-state what problem actually is here and discuss how to solve
> it. (The reference [2] didn't provide a detailed explanation of the
> problem, only the reason why we want a separate parameter).

It's principally a DBA feature, in that it allows the DBA to
separately control the memory used by very routine vacuuming, while
also having a less conservative default value for maintenance
operations that typically are under direct human control. Yes, this is
no better than just having maintenance_work_mem be equal to your
would-be autovacuum_work_mem setting in the first place, and having
everyone remember to set maintenance_work_mem dynamically. However,
sometimes people are ill-informed (more ill-informed than the person
that puts the setting in postgresql.conf), and other times they're
forgetful, and other times still they're using a tool like pg_restore
with no convenient way to dynamically set maintenance_work_mem. So, to
answer your question, yes: it is entirely possible that you or someone
like you may have no use for this.

It's often reasonable to assume that autovacuum workers are the only
processes that can allocate memory bound in size by
maintenance_work_mem that are not under the direct control of a human
performing maintenance. Autovacuum workers are in a sense just
servicing regular application queries (consider how Oracle handles
ROLLBACK garbage collection), and things that service regular
application queries are already bound separately by work_mem.

--
Peter Geoghegan

On Sun, Oct 20, 2013 at 7:21 AM, Magnus Hagander <magnus(at)hagander(dot)net> wrote:
>> It seemed neater to me to create a new flag, so that in principle any
>> vacuum() code path can request autovacuum_work_mem, rather than having
>> lazyvacuum.c code call IsAutoVacuumWorkerProcess() for the same
>> purpose. To date, that's only been done within vacuumlazy.c for things
>> like logging.
>
> Hmm. I'm not entirely sure I agree that that makes it neater :)
>
> We could also look at autovacuum_vacuum_cost_limit etc above, but
> those just override what the non-autovac parameters do. But since the
> parameter is called maintenance_work_mem in that case, I think that
> would make it harder to read.
>
> But I'd suggest just a:
> int vac_work_mem = (IsAutoVacuumWorkerProcess() && autovacuum_work_mem
> != -1) ? autovacuum_work_mem : maintenance_work_mem;
>
> and not sending around a boolean flag through a bunch of places when
> it really means just the same thing,

+1 for that change.

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

On 25 November 2013 21:51, Peter Geoghegan <pg(at)heroku(dot)com> wrote:
> On Sun, Nov 24, 2013 at 9:06 AM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
>> VACUUM uses 6 bytes per dead tuple. And autovacuum regularly removes
>> dead tuples, limiting their numbers.
>>
>> In what circumstances will the memory usage from multiple concurrent
>> VACUUMs become a problem? In those circumstances, reducing
>> autovacuum_work_mem will cause more passes through indexes, dirtying
>> more pages and elongating the problem workload.
>
> Yes, of course, but if we presume that the memory for autovacuum
> workers to do everything in one pass simply isn't there, it's still
> better to do multiple passes.

That isn't clear to me. It seems better to wait until we have the memory.

My feeling is this parameter is a fairly blunt approach to the
problems of memory pressure on autovacuum and other maint tasks. I am
worried that it will not effectively solve the problem. I don't wish
to block the patch; I wish to get to an effective solution to the
problem.

A better aproach to handling memory pressure would be to globally
coordinate workers so that we don't oversubscribe memory, allocating
memory from a global pool.

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

On Wed, Dec 11, 2013 at 9:43 AM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
> On 25 November 2013 21:51, Peter Geoghegan <pg(at)heroku(dot)com> wrote:
>> On Sun, Nov 24, 2013 at 9:06 AM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
>>> VACUUM uses 6 bytes per dead tuple. And autovacuum regularly removes
>>> dead tuples, limiting their numbers.
>>>
>>> In what circumstances will the memory usage from multiple concurrent
>>> VACUUMs become a problem? In those circumstances, reducing
>>> autovacuum_work_mem will cause more passes through indexes, dirtying
>>> more pages and elongating the problem workload.
>>
>> Yes, of course, but if we presume that the memory for autovacuum
>> workers to do everything in one pass simply isn't there, it's still
>> better to do multiple passes.
>
> That isn't clear to me. It seems better to wait until we have the memory.
>
> My feeling is this parameter is a fairly blunt approach to the
> problems of memory pressure on autovacuum and other maint tasks. I am
> worried that it will not effectively solve the problem. I don't wish
> to block the patch; I wish to get to an effective solution to the
> problem.
>
> A better aproach to handling memory pressure would be to globally
> coordinate workers so that we don't oversubscribe memory, allocating
> memory from a global pool.

This is doubtless true, but that project is at least two if not three
orders of magnitude more complex than what's being proposed here, and
I don't think we should make the perfect the enemy of the good.

Right now, maintenance_work_mem controls the amount of memory that
we're willing to use for either a vacuum operation or an index build.
Those things don't have much to do with each other, so it's not hard
for me to imagine that someone might want to configure different
memory usage for one than the other. This patch would allow that, and
I think that's good.

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

On 11 December 2013 14:50, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Wed, Dec 11, 2013 at 9:43 AM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
>> On 25 November 2013 21:51, Peter Geoghegan <pg(at)heroku(dot)com> wrote:
>>> On Sun, Nov 24, 2013 at 9:06 AM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
>>>> VACUUM uses 6 bytes per dead tuple. And autovacuum regularly removes
>>>> dead tuples, limiting their numbers.
>>>>
>>>> In what circumstances will the memory usage from multiple concurrent
>>>> VACUUMs become a problem? In those circumstances, reducing
>>>> autovacuum_work_mem will cause more passes through indexes, dirtying
>>>> more pages and elongating the problem workload.
>>>
>>> Yes, of course, but if we presume that the memory for autovacuum
>>> workers to do everything in one pass simply isn't there, it's still
>>> better to do multiple passes.
>>
>> That isn't clear to me. It seems better to wait until we have the memory.
>>
>> My feeling is this parameter is a fairly blunt approach to the
>> problems of memory pressure on autovacuum and other maint tasks. I am
>> worried that it will not effectively solve the problem. I don't wish
>> to block the patch; I wish to get to an effective solution to the
>> problem.
>>
>> A better aproach to handling memory pressure would be to globally
>> coordinate workers so that we don't oversubscribe memory, allocating
>> memory from a global pool.
>
> This is doubtless true, but that project is at least two if not three
> orders of magnitude more complex than what's being proposed here, and
> I don't think we should make the perfect the enemy of the good.

It looks fairly easy to estimate the memory needed for an auto vacuum,
since we know the scale factor and the tuple estimate. We can then use
the memory estimate to alter the scheduling of work. And/or we can use
actual memory usage and block auto vac workers if they need more
memory than is currently available because of other workers.

We would still benefit from a new parameter in the above sketch, but
it would achieve something useful in practice.

That's about 2-3 days work and I know Peter can hack it. So the
situation is not perfection-sought-blocking-good, this is more like
fairly poor solution being driven through when a better solution is
available within the time and skills available.

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

On Wed, Dec 11, 2013 at 10:41 AM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
> It looks fairly easy to estimate the memory needed for an auto vacuum,
> since we know the scale factor and the tuple estimate. We can then use
> the memory estimate to alter the scheduling of work. And/or we can use
> actual memory usage and block auto vac workers if they need more
> memory than is currently available because of other workers.
>
> We would still benefit from a new parameter in the above sketch, but
> it would achieve something useful in practice.
>
> That's about 2-3 days work and I know Peter can hack it. So the
> situation is not perfection-sought-blocking-good, this is more like
> fairly poor solution being driven through when a better solution is
> available within the time and skills available.

I don't agree with that assessment. Anything that involves changing
the scheduling of autovacuum is a major project that will legitimately
provoke much controversy. Extensive testing will be needed to prove
that the new algorithm doesn't perform worse than the current
algorithm in any important cases. I have my doubts about whether that
can be accomplished in an entire release cycle, let alone 2-3 days.
In contrast, the patch proposed does something that is easy to
understand, clearly safe, and an improvement over what we have now.

Quite apart from the amount of development time required, I think that
the idea that we would use the availability of memory to schedule work
is highly suspect. You haven't given any details on what you think
that algorithm might look like, and I doubt that any simple solution
will do. If running more autovacuum workers drives the machine into
swap, then we shouldn't, but we have no way of calculating what size
memory allocation will cause that to happen. But NOT running
autovacuum workers isn't safe either, because it could cause table
bloat that them drives the machine into swap. We have no way of
knowing whether that will happen either. So I think your contention
that we have the necessary information available to make an
intelligent decision is incorrect.

Regardless, whether or not a more complex change is within Peter's
technical capabilities is utterly irrelevant to whether we should
adopt the proposed patch. Your phrasing seems to imply that you would
not ask such a thing of a less-talented individual, and I think that
is utterly wrong. Peter's technical acumen does not give us the right
to ask him to write a more complex patch as a condition of getting a
simpler one accepted.

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

On 12/11/2013 09:57 AM, Robert Haas wrote:
> I don't agree with that assessment. Anything that involves changing
> the scheduling of autovacuum is a major project that will legitimately
> provoke much controversy. Extensive testing will be needed to prove
> that the new algorithm doesn't perform worse than the current
> algorithm in any important cases. I have my doubts about whether that
> can be accomplished in an entire release cycle, let alone 2-3 days.
> In contrast, the patch proposed does something that is easy to
> understand, clearly safe, and an improvement over what we have now.

+1

There is an inherent tuning and troubleshooting challenge in anything
involving a feedback loop.

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

On Wed, Dec 11, 2013 at 7:41 AM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
> That's about 2-3 days work and I know Peter can hack it. So the
> situation is not perfection-sought-blocking-good, this is more like
> fairly poor solution being driven through when a better solution is
> available within the time and skills available.

I think that that's a very optimistic assessment of the amount of work
required. Even by the rose-tinted standards of software project time
estimation. A ton of data is required to justify fundamental
infrastructural changes like that.

--
Peter Geoghegan

On 11 December 2013 17:57, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

> Extensive testing will be needed to prove
> that the new algorithm doesn't perform worse than the current
> algorithm in any important cases.

Agreed, but the amount of testing seems equivalent in both cases,
assuming we weren't going to skip it for this patch.

Let me repeat the question, so we are clear...
In what circumstances will the memory usage from multiple concurrent
VACUUMs become a problem? In those circumstances, reducing
autovacuum_work_mem will cause more passes through indexes, dirtying
more pages and elongating the problem workload. I agree that multiple
concurrent VACUUMs could be a problem but this
doesn't solve that, it just makes things worse.

The *only* time this parameter would have any effect looks like when
it will make matters worse.

With considerable regret, I don't see how this solves the problem at
hand. We can and should do better.

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

On Wed, Dec 11, 2013 at 2:37 PM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
> On 11 December 2013 17:57, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>> Extensive testing will be needed to prove
>> that the new algorithm doesn't perform worse than the current
>> algorithm in any important cases.
>
> Agreed, but the amount of testing seems equivalent in both cases,
> assuming we weren't going to skip it for this patch.
>
> Let me repeat the question, so we are clear...
> In what circumstances will the memory usage from multiple concurrent
> VACUUMs become a problem? In those circumstances, reducing
> autovacuum_work_mem will cause more passes through indexes, dirtying
> more pages and elongating the problem workload. I agree that multiple
> concurrent VACUUMs could be a problem but this
> doesn't solve that, it just makes things worse.

That's not the problem the patch is designed to solve. It's intended
for the case where you want to allow more or less memory to autovacuum
than you do for index builds. There's no principled reason that
anyone should want those things to be the same. It is not difficult
to imagine situations in which you would want one set to a very
different value than the other. In particular it seems quite likely
to me that the amount of memory appropriate for index builds might be
vastly more than is needed by autovacuum. For example, in a
data-warehousing environment where updates are rare but large index
builds by the system's sole user are frequent, someone might want to
default index builds to 64GB of RAM (especially after Noah's patch to
allow huge allocations for the tuple array while sorting) but only
need 256MB for autovacuum.

In general, I'm reluctant to believe that Peter proposed this patch
just for fun. I assume this is a real-world problem that Heroku
encounters in their environment. If not, well then that's different.

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

On 12/11/2013 11:37 AM, Simon Riggs wrote:> On 11 December 2013 17:57,
Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
>> Extensive testing will be needed to prove
>> that the new algorithm doesn't perform worse than the current
>> algorithm in any important cases.
>
> Agreed, but the amount of testing seems equivalent in both cases,
> assuming we weren't going to skip it for this patch.

No performance testing is required for this patch. The effect of memory
limits on vacuum are already well-known and well-understood.

> With considerable regret, I don't see how this solves the problem at
> hand. We can and should do better.

I strongly disagree. The problem we are dealing with currently is that
two resource limits which should have *always* been independent of each
other are currently conflated into a single GUC variable. This forces
users to remember to set maintenance_work_mem interactively every time
they want to run a manual VACUUM, because the setting in postgresql.conf
is needed to tune autovacuum.

In other words, we are having an issue with *non-atomic data*, and this
patch partially fixes that.

Would it be better to have an admissions-control policy engine for
launching autovacuum which takes into account available RAM, estimated
costs of concurrent vacuums, current CPU activity, and which tables are
in cache? Yes. And if you started on that now, you might have it ready
for 9.5.

And, for that matter, accepting this patch by no means blocks doing
something more sophisticated in the future.

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

Josh Berkus <josh(at)agliodbs(dot)com> writes:
> And, for that matter, accepting this patch by no means blocks doing
> something more sophisticated in the future.

Yeah. I think the only real argument against it is "do we really need
yet another knob?". Since Josh, who's usually the voicer of that
argument, is for this one, I don't have a problem with it.

regards, tom lane

On 12/11/2013 12:40 PM, Tom Lane wrote:
> Josh Berkus <josh(at)agliodbs(dot)com> writes:
>> And, for that matter, accepting this patch by no means blocks doing
>> something more sophisticated in the future.
>
> Yeah. I think the only real argument against it is "do we really need
> yet another knob?". Since Josh, who's usually the voicer of that
> argument, is for this one, I don't have a problem with it.

This passes the "is it a chronic problem not to have a knob for this?" test.

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

On 11 December 2013 19:54, Josh Berkus <josh(at)agliodbs(dot)com> wrote:
> On 12/11/2013 11:37 AM, Simon Riggs wrote:> On 11 December 2013 17:57,
> Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>>
>>> Extensive testing will be needed to prove
>>> that the new algorithm doesn't perform worse than the current
>>> algorithm in any important cases.
>>
>> Agreed, but the amount of testing seems equivalent in both cases,
>> assuming we weren't going to skip it for this patch.
>
> No performance testing is required for this patch. The effect of memory
> limits on vacuum are already well-known and well-understood.

Yes, I wrote the patch that you use to tune autovacuum. Not everybody
agreed with me then either about whether we need it, so I'm used to
people questioning my thinking and am happy people do.

>> With considerable regret, I don't see how this solves the problem at
>> hand. We can and should do better.
>
> I strongly disagree. The problem we are dealing with currently is that
> two resource limits which should have *always* been independent of each
> other are currently conflated into a single GUC variable. This forces
> users to remember to set maintenance_work_mem interactively every time
> they want to run a manual VACUUM, because the setting in postgresql.conf
> is needed to tune autovacuum.

I understand the general argument and it sounds quite cool, I agree. I
am all for user control.

But nobody has given a sensible answer to my questions, other than to
roll out the same general points again. In practice, its a knob that
does not do very much of use for us. At best it is addressing the
symptoms, not the cause. IMHO.

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

On Wed, Dec 11, 2013 at 1:28 PM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
> But nobody has given a sensible answer to my questions, other than to
> roll out the same general points again. In practice, its a knob that
> does not do very much of use for us. At best it is addressing the
> symptoms, not the cause. IMHO.

It's just a usability feature. It lessens the intellectual overhead of
managing maintenance_work_mem. I understand that it isn't very
impressive from a technical point of view. However, in many
environments, it actually will make a significant difference, because
non-autovacuum maintenance operations are very rare compared to
autovacuum workers vacuuming, and therefore I can now afford to be
much less conservative in setting maintenance_work_mem globally on
each of 8 Postgres instances hosted on a single box. These are
precisely the kinds of Postgres instances where users are very
unlikely to have heard of maintenance_work_mem at all. These users are
not even using an admin tool in many cases, preferring to rely on ORM
migrations. Having said that, it's also something I'll find useful on
a day to day basis, because it's a chore to set maintenace_work_mem
manually, and sometimes I forget to do so, particularly when under
pressure to relieve a production performance issues on a random
customer's database.

--
Peter Geoghegan

On 11 December 2013 22:23, Peter Geoghegan <pg(at)heroku(dot)com> wrote:
> On Wed, Dec 11, 2013 at 1:28 PM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
>> But nobody has given a sensible answer to my questions, other than to
>> roll out the same general points again. In practice, its a knob that
>> does not do very much of use for us. At best it is addressing the
>> symptoms, not the cause. IMHO.
>
> It's just a usability feature. It lessens the intellectual overhead of
> managing maintenance_work_mem. I understand that it isn't very
> impressive from a technical point of view. However, in many
> environments, it actually will make a significant difference, because
> non-autovacuum maintenance operations are very rare compared to
> autovacuum workers vacuuming, and therefore I can now afford to be
> much less conservative in setting maintenance_work_mem globally on
> each of 8 Postgres instances hosted on a single box. These are
> precisely the kinds of Postgres instances where users are very
> unlikely to have heard of maintenance_work_mem at all. These users are
> not even using an admin tool in many cases, preferring to rely on ORM
> migrations. Having said that, it's also something I'll find useful on
> a day to day basis, because it's a chore to set maintenace_work_mem
> manually, and sometimes I forget to do so, particularly when under
> pressure to relieve a production performance issues on a random
> customer's database.

My view remains that, yes, we have a problem setting maint work men
usefully, my conclusion is that having two parameters we don't know
how to set won't improve things and doesn't constitute an improvement
in usability.

That being said, I acknowledge everybody else's viewpoints here and
will commit this feature as is.

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

On Wed, Dec 11, 2013 at 10:35:32AM -0800, Josh Berkus wrote:
> On 12/11/2013 09:57 AM, Robert Haas wrote:
> > I don't agree with that assessment. Anything that involves changing
> > the scheduling of autovacuum is a major project that will legitimately
> > provoke much controversy. Extensive testing will be needed to prove
> > that the new algorithm doesn't perform worse than the current
> > algorithm in any important cases. I have my doubts about whether that
> > can be accomplished in an entire release cycle, let alone 2-3 days.
> > In contrast, the patch proposed does something that is easy to
> > understand, clearly safe, and an improvement over what we have now.
>
> +1
>
> There is an inherent tuning and troubleshooting challenge in anything
> involving a feedback loop.

We have avoided feedback loops in the past. I think eventually we are
going to need to tackle them, but it is a big job, and vacuum memory
usage would be at the bottom of my list of feedback loop tasks.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

On 12/13/2013 08:24 PM, Bruce Momjian wrote:
> On Wed, Dec 11, 2013 at 10:35:32AM -0800, Josh Berkus wrote:
>> On 12/11/2013 09:57 AM, Robert Haas wrote:
>>> I don't agree with that assessment. Anything that involves changing
>>> the scheduling of autovacuum is a major project that will legitimately
>>> provoke much controversy. Extensive testing will be needed to prove
>>> that the new algorithm doesn't perform worse than the current
>>> algorithm in any important cases. I have my doubts about whether that
>>> can be accomplished in an entire release cycle, let alone 2-3 days.
>>> In contrast, the patch proposed does something that is easy to
>>> understand, clearly safe, and an improvement over what we have now.
>>
>> +1
>>
>> There is an inherent tuning and troubleshooting challenge in anything
>> involving a feedback loop.
>
> We have avoided feedback loops in the past. I think eventually we are
> going to need to tackle them, but it is a big job, and vacuum memory
> usage would be at the bottom of my list of feedback loop tasks.

I haven't been following this thread in detail, but would it help if we
implemented a scheme to reduce (auto)vacuum's memory usage? Such schemes
have been discussed in the past, packing the list of dead items more
tightly.

I guess you'd still want to have a limit on autovacuum's memory usage. A
much lower limit than you'd want to allow for one-off CREATE INDEX
operations and such.

(Personally, I don't care whether we add this new option or not. And -1
for feedback loops.)

- Heikki

Heikki Linnakangas escribió:

> I haven't been following this thread in detail, but would it help if
> we implemented a scheme to reduce (auto)vacuum's memory usage? Such
> schemes have been discussed in the past, packing the list of dead
> items more tightly.

Well, it would help some, but it wouldn't eliminate the problem
completely. Autovacuum scales its memory usage based on the size of the
table. There will always be a table so gigantic that a maximum
allocated memory is to be expected; and DBAs will need a way to limit
the memory consumption even if we pack dead items more densely.

I was playing with keeping item pointers for each page in a bitmapset.
This was pretty neat and used a lot less memory than currently, except
that I needed to allocate a large chunk of memory and then have
bitmapsets use words within that large allocation space. It turned out
to be too ugly so I abandoned it. With the "varbit encoding" thingy in
the recent GIN patchset, maybe it would be workable.

I think a more dense packing is desired regardless of this patch.
Maybe we can have a generic module for "item pointer arrays" which could
be useful for both autovacuum and GIN.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On 12/13/2013 08:40 PM, Alvaro Herrera wrote:
> Heikki Linnakangas escribió:
>
>> I haven't been following this thread in detail, but would it help if
>> we implemented a scheme to reduce (auto)vacuum's memory usage? Such
>> schemes have been discussed in the past, packing the list of dead
>> items more tightly.
>
> Well, it would help some, but it wouldn't eliminate the problem
> completely. Autovacuum scales its memory usage based on the size of the
> table. There will always be a table so gigantic that a maximum
> allocated memory is to be expected; and DBAs will need a way to limit
> the memory consumption even if we pack dead items more densely.
>
> I was playing with keeping item pointers for each page in a bitmapset.
> This was pretty neat and used a lot less memory than currently, except
> that I needed to allocate a large chunk of memory and then have
> bitmapsets use words within that large allocation space. It turned out
> to be too ugly so I abandoned it. With the "varbit encoding" thingy in
> the recent GIN patchset, maybe it would be workable.

The varbyte encoding is actually a very poor fit for vacuum. Vacuum
needs fast random access into the array when scanning indexes, and the
varbyte encoded item pointer lists used in gin don't allow that.

I couldn't find it in the archives now, but when we last discussed this,
Tom suggested that we divide the large chunk of memory that vacuum
allocates into two parts. The first part grows from the bottom up, and
the second part from top down, until there is no free space in the
middle anymore. For each heap page, there is one entry in the first
part, with the block number, and a pointer to an entry in the second
part. In the second part, there's a list of offset numbers on that page
(or a bitmap).

Another idea: Store only the least significant 20 bits the block number
of each item pointer, and use the remaining 12 bits for the offset
number. So each item pointer is stored as a single 32 bit integer. For
the top 12 bits of the block number, build a separate lookup table of
4096 entries, indexed by the top bits. Each entry in the lookup table
points to the beginning and end index in the main array where the
entries for that page range is stored. That would reduce the memory
usage by about 1/3, which isn't as good as the bitmap method when there
is a lot of dead tuples same pages, but would probably be a smaller patch.

- Heikki

On 16 December 2013 10:12, Heikki Linnakangas <hlinnakangas(at)vmware(dot)com> wrote:
> On 12/13/2013 08:40 PM, Alvaro Herrera wrote:
>>
>> Heikki Linnakangas escribió:
>>
>>> I haven't been following this thread in detail, but would it help if
>>> we implemented a scheme to reduce (auto)vacuum's memory usage? Such
>>> schemes have been discussed in the past, packing the list of dead
>>> items more tightly.
>>
>>
>> Well, it would help some, but it wouldn't eliminate the problem
>> completely. Autovacuum scales its memory usage based on the size of the
>> table. There will always be a table so gigantic that a maximum
>> allocated memory is to be expected; and DBAs will need a way to limit
>> the memory consumption even if we pack dead items more densely.

The problem is allocation of memory, not one of efficient usage once
we have been allocated.

> Another idea: Store only the least significant 20 bits the block number of
> each item pointer, and use the remaining 12 bits for the offset number. So
> each item pointer is stored as a single 32 bit integer. For the top 12 bits
> of the block number, build a separate lookup table of 4096 entries, indexed
> by the top bits. Each entry in the lookup table points to the beginning and
> end index in the main array where the entries for that page range is stored.
> That would reduce the memory usage by about 1/3, which isn't as good as the
> bitmap method when there is a lot of dead tuples same pages, but would
> probably be a smaller patch.

We would do better to just use memory from shared_buffers and then we
wouldn't need a memory allocation or limit.
If we split the allocation into a series of BLCKSZ blocks of memory,
we can use your compression down to 4 bytes/row and then index the
blocks.

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