Обсуждение: Truncating/vacuuming relations on full tablespaces

On 9/4/15 7:04 AM, Thom Brown wrote:
> But shouldn't we not be creating FSM or VM files when truncating?

Maybe, but even then you still need to create a bunch of new files (at 
least one for the table and one for each index), and AFAIK the first 
page in each file will be properly initialized, which means each file 
will be at least BLKSZ.

> ISTM that the vacuum case is one we'd really want to avoid, though, as
> it's trickier to work around the problem.

What might make sense is a special 'free up space NOW' mode that focuses 
only on attempting to truncate the relation, because if you can't 
actually shrink the heap you're not going to make any progress.

But since none of this will help at all in the default case where WAL is 
on the same filesystem as the data, I don't know that it's worth it.
-- 
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com

On Fri, Sep 4, 2015 at 8:04 AM, Thom Brown <thom@linux.com> wrote:
> Currently, when attempting to vacuum a table on a tablespace with no space
> left, we get an error:
>
> postgres=# vacuum test;
> ERROR:  could not extend file
> "pg_tblspc/19605/PG_9.6_201508111/12382/19616_vm": No space left on device
> HINT:  Check free disk space.
>
> This is because it first tries to grow the visibility map file.
>
> We also get a similar problem when attempting to truncate with restart
> identity:
>
> postgres=# truncate table test restart identity;
> ERROR:  canceling autovacuum task
> CONTEXT:  automatic vacuum of table "postgres.public.test"
> ERROR:  could not extend file "base/12382/16391": No space left on device
> HINT:  Check free disk space.
> STATEMENT:  truncate table test restart identity;
>
> I guess a workaround for the 2nd case is to truncate without restarting the
> identify, then truncate again with restart identify, or just resetting the
> sequence.  In any case, someone will likely be running this command to free
> up space, and they can't due to lack of space.
>
> But shouldn't we not be creating FSM or VM files when truncating?

That seems like it might possibly be a good thing to avoid, but we're
not doing it in either of those examples.  So, I am confused.  In the
first example, the error is happening during VACUUM, not TRUNCATE, and
it's unclear what else we could do besides error out.  I mean, we
could make it so that VACUUM fails softly rather than emitting a hard
error if unable to grow the visibility map, but that sounds like an
anti-feature.  In the second case, the error is happening during
TRUNCATE, but it's happening on the main fork of the sequence
relation, not any auxiliary fork.  So you've got two examples of
things failing here but neither one matches the problem statement.

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

On 15 January 2016 at 15:21, Robert Haas <robertmhaas@gmail.com> wrote:
> On Fri, Sep 4, 2015 at 8:04 AM, Thom Brown <thom@linux.com> wrote:
>> Currently, when attempting to vacuum a table on a tablespace with no space
>> left, we get an error:
>>
>> postgres=# vacuum test;
>> ERROR:  could not extend file
>> "pg_tblspc/19605/PG_9.6_201508111/12382/19616_vm": No space left on device
>> HINT:  Check free disk space.
>>
>> This is because it first tries to grow the visibility map file.
>>
>> We also get a similar problem when attempting to truncate with restart
>> identity:
>>
>> postgres=# truncate table test restart identity;
>> ERROR:  canceling autovacuum task
>> CONTEXT:  automatic vacuum of table "postgres.public.test"
>> ERROR:  could not extend file "base/12382/16391": No space left on device
>> HINT:  Check free disk space.
>> STATEMENT:  truncate table test restart identity;
>>
>> I guess a workaround for the 2nd case is to truncate without restarting the
>> identify, then truncate again with restart identify, or just resetting the
>> sequence.  In any case, someone will likely be running this command to free
>> up space, and they can't due to lack of space.
>>
>> But shouldn't we not be creating FSM or VM files when truncating?
>
> That seems like it might possibly be a good thing to avoid, but we're
> not doing it in either of those examples.  So, I am confused.

So am I, reading it back I'm not sure why I said that now.

The problem is with attempting to extend some file on a full
tablespace during a vacuum or a truncate.  I guess they are different
but related problems.

Thom

On Fri, Jan 15, 2016 at 11:05 AM, Thom Brown <thom@linux.com> wrote:
> On 15 January 2016 at 15:21, Robert Haas <robertmhaas@gmail.com> wrote:
>> On Fri, Sep 4, 2015 at 8:04 AM, Thom Brown <thom@linux.com> wrote:
>>> Currently, when attempting to vacuum a table on a tablespace with no space
>>> left, we get an error:
>>>
>>> postgres=# vacuum test;
>>> ERROR:  could not extend file
>>> "pg_tblspc/19605/PG_9.6_201508111/12382/19616_vm": No space left on device
>>> HINT:  Check free disk space.
>>>
>>> This is because it first tries to grow the visibility map file.
>>>
>>> We also get a similar problem when attempting to truncate with restart
>>> identity:
>>>
>>> postgres=# truncate table test restart identity;
>>> ERROR:  canceling autovacuum task
>>> CONTEXT:  automatic vacuum of table "postgres.public.test"
>>> ERROR:  could not extend file "base/12382/16391": No space left on device
>>> HINT:  Check free disk space.
>>> STATEMENT:  truncate table test restart identity;
>>>
>>> I guess a workaround for the 2nd case is to truncate without restarting the
>>> identify, then truncate again with restart identify, or just resetting the
>>> sequence.  In any case, someone will likely be running this command to free
>>> up space, and they can't due to lack of space.
>>>
>>> But shouldn't we not be creating FSM or VM files when truncating?
>>
>> That seems like it might possibly be a good thing to avoid, but we're
>> not doing it in either of those examples.  So, I am confused.
>
> So am I, reading it back I'm not sure why I said that now.
>
> The problem is with attempting to extend some file on a full
> tablespace during a vacuum or a truncate.  I guess they are different
> but related problems.

Well, I think that trying to extend a file on a full tablespace during
truncate would be a problem.  However, I can't see any evidence that
we do that, except with RESTART IDENTITY, where it's unavoidable
because you need to recreate the sequence.  On the other hand,
extending a file on a full tablespace during VACUUM does not seem to
me to be a bug.  It is true that it is remotely possible that you
could have a table with empty space at the end which VACUUM would
truncate but for inability to create the FSM or VM, and that would
suck.  On the other hand, suppose you have a table which just happens
to fill the tablespace until it's almost but (you think) not quite
full.  Then you VACUUM the table.  If it just silently failed to build
the visibility map and then all your subsequent vacuums were really
slow but without any user-visible notice that there's a problem, that
would be awful.  So all in all I think the system seems to be behaving
as we would wish, unless there's some other test case that shows us
creating the VM or FSM when it's needless to do so.

Now, I do think it's a somewhat fair complaint that if you have a
tablespace which is totally, 100% full, recovery is difficult.  You
can probably DROP the table, but TRUNCATE might fail, and so might
VACUUM.  You could argue that DROP is usually a good substitute for
TRUNCATE, although there could be dependencies, but DROP is certainly
not a good substitute for VACUUM.  We could address the VACUUM case by
having an optional argument to VACUUM which tells it to skip VM and
FSM maintenance, presumably to be used only in case of emergency.  I'm
not sure if it's worth having for what is presumably  a pretty rare
case, but it seems like it could be done.

We could try to address the TRUNCATE case by adding a flag to
optionally perform a non-transactional TRUNCATE, like we did prior to
7.4, but I wonder how that's ever really safe.  Suppose PostgreSQL
tries to truncate either the table or one of its indexes but then,
when trying to truncate the other, we get an error from the operating
system.  We cannot recover by aborting the transaction, nor can we
complete the operation by going forward.  That mighta been the sort of
thing we didn't worry about much in the early 7 series, but I don't
think it has much chance of passing muster today.

Anybody else want to weigh in with thoughts on any of this?

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

On Fri, Jan 15, 2016 at 11:41 AM, Robert Haas <robertmhaas@gmail.com> wrote:

> Anybody else want to weigh in with thoughts on any of this?

Leaving aside VACUUM issues for a moment, what problems to you see
with an empty table that has no visibility map or free space map
fork?  In other words, for the TRUNCATE case we could either skip
these if there is an error, or not even try to build them at all.
Either seems better than the status quo.

--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Fri, Jan 15, 2016 at 1:24 PM, Kevin Grittner <kgrittn@gmail.com> wrote:
> On Fri, Jan 15, 2016 at 11:41 AM, Robert Haas <robertmhaas@gmail.com> wrote:
>> Anybody else want to weigh in with thoughts on any of this?
>
> Leaving aside VACUUM issues for a moment, what problems to you see
> with an empty table that has no visibility map or free space map
> fork?  In other words, for the TRUNCATE case we could either skip
> these if there is an error, or not even try to build them at all.
> Either seems better than the status quo.

The status quo *is* that we don't try to build them at all.

rhaas=# create table foo (a int, b int);
CREATE TABLE
rhaas=# insert into foo values (1,1);
INSERT 0 1
rhaas=# vacuum analyze foo;
VACUUM
rhaas=# select relfilenode from pg_class where relname = 'foo';relfilenode
-------------      16385
(1 row)

In another window:

-rw-------  1 rhaas  staff   8192 Jan 15 13:45 16385
-rw-------  1 rhaas  staff  24576 Jan 15 13:45 16385_fsm
-rw-------  1 rhaas  staff   8192 Jan 15 13:45 16385_vm

Back to the first window:

rhaas=# truncate foo;
TRUNCATE TABLE
rhaas=# select relfilenode from pg_class where relname = 'foo';relfilenode
-------------      16388
(1 row)

Back to the second window:

[rhaas 16384]$ ls -l 16388*
-rw-------  1 rhaas  staff  0 Jan 15 13:46 16388

There's no full disk involved here or anything.  Just a plain old TRUNCATE.

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

Robert Haas <robertmhaas@gmail.com> writes:
> Now, I do think it's a somewhat fair complaint that if you have a
> tablespace which is totally, 100% full, recovery is difficult.  You
> can probably DROP the table, but TRUNCATE might fail, and so might
> VACUUM.  You could argue that DROP is usually a good substitute for
> TRUNCATE, although there could be dependencies, but DROP is certainly
> not a good substitute for VACUUM.  We could address the VACUUM case by
> having an optional argument to VACUUM which tells it to skip VM and
> FSM maintenance, presumably to be used only in case of emergency.  I'm
> not sure if it's worth having for what is presumably  a pretty rare
> case, but it seems like it could be done.

Presumably the hope would be that VACUUM would truncate off some of the
heap, else there's not much good that's going to happen.  That leaves
me wondering exactly what the invariant is for the maps, and if it's
okay to not touch them during a heap truncation.

I believe that there would be ramifications for some of the index AMs
too.  For example, if left to its own devices GIN would consider VACUUM
to include flushing its pending-list pages, which more than likely will
increase not reduce the total index size.  I'm not sure that it has
any ability to omit that step; can it remove dead entries directly off
the pending pages, or only from the main index?

On the whole this sounds like a lot of work, and a lot of hard-to-test
seldom-exercised code, for a very very narrow corner case.
        regards, tom lane

On Fri, Jan 15, 2016 at 11:16 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

> I believe that there would be ramifications for some of the index AMs
> too.  For example, if left to its own devices GIN would consider VACUUM
> to include flushing its pending-list pages, which more than likely will
> increase not reduce the total index size.  I'm not sure that it has
> any ability to omit that step; can it remove dead entries directly off
> the pending pages, or only from the main index?

It cannot vacuum the pending list directly.  That is why it is a bug
for the vacuum to short-cut out of the pending list cleanup step when
it finds someone else already cleaning it.  For correctness it has to
either clean it itself, or wait until the other process is done (or at
least, done up to the point where the tail was at the time the vacuum
started).

Cheers,

Jeff

On Fri, Jan 15, 2016 at 2:16 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> Now, I do think it's a somewhat fair complaint that if you have a
>> tablespace which is totally, 100% full, recovery is difficult.  You
>> can probably DROP the table, but TRUNCATE might fail, and so might
>> VACUUM.  You could argue that DROP is usually a good substitute for
>> TRUNCATE, although there could be dependencies, but DROP is certainly
>> not a good substitute for VACUUM.  We could address the VACUUM case by
>> having an optional argument to VACUUM which tells it to skip VM and
>> FSM maintenance, presumably to be used only in case of emergency.  I'm
>> not sure if it's worth having for what is presumably  a pretty rare
>> case, but it seems like it could be done.
>
> Presumably the hope would be that VACUUM would truncate off some of the
> heap, else there's not much good that's going to happen.  That leaves
> me wondering exactly what the invariant is for the maps, and if it's
> okay to not touch them during a heap truncation.

No, you're missing the point, or at least I think you are.  Suppose
somebody creates a big table and then deletes all the tuples in the
second half, but VACUUM never runs.  When at last VACUUM does run on
that table, it will try to build the VM and FSM forks as it scans the
table, and will only truncate AFTER that has been done.  If building
the VM and FSM forks fails because there is no freespace, we will
never reach the part of the operation that could create some.

The key point is that both the VM and the FSM are optional.  If
there's no VM, vacuum will have to visit every page in the table and
index-only scans will never be index-only, but everything still works.
If there's no FSM, we'll assume the table has no internal freespace,
so inserts will extend the table.  Those consequences are bad, of
course, so we really want vacuum to succeed in creating the VM and
FSM.  However, when a failure creating the FSM or VM causes us not to
reach the truncation step, then there's no way to shrink the table.
That's not good.

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

Robert Haas <robertmhaas@gmail.com> writes:
> On Fri, Jan 15, 2016 at 2:16 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Presumably the hope would be that VACUUM would truncate off some of the
>> heap, else there's not much good that's going to happen.  That leaves
>> me wondering exactly what the invariant is for the maps, and if it's
>> okay to not touch them during a heap truncation.

> No, you're missing the point, or at least I think you are.  Suppose
> somebody creates a big table and then deletes all the tuples in the
> second half, but VACUUM never runs.  When at last VACUUM does run on
> that table, it will try to build the VM and FSM forks as it scans the
> table, and will only truncate AFTER that has been done.  If building
> the VM and FSM forks fails because there is no freespace, we will
> never reach the part of the operation that could create some.

No, I follow that perfectly.  I think you missed *my* point, which is:
suppose that we do have a full-length VM and/or FSM fork for a relation,
and VACUUM decides to truncate the relation.  Is it okay to not truncate
the VM/FSM?  If it isn't, we're going to have to have very tricky
semantics for any "don't touch the map forks" option, because it will
have to suppress only some of VACUUM's map updates.

If the map invariants are such that leaving garbage in them is
unconditionally safe, then this isn't a problem; but I'm unsure of that.

> The key point is that both the VM and the FSM are optional.

No, the key point is whether it's okay if they *are* there and contain
lies, or self-inconsistent data.

An alternative approach that might avoid such worries is to have a mode
wherein VACUUM always truncates the map forks to nothing, rather than
attempting to update them.
        regards, tom lane

On Mon, Jan 18, 2016 at 2:26 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> On Fri, Jan 15, 2016 at 2:16 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> Presumably the hope would be that VACUUM would truncate off some of the
>>> heap, else there's not much good that's going to happen.  That leaves
>>> me wondering exactly what the invariant is for the maps, and if it's
>>> okay to not touch them during a heap truncation.
>
>> No, you're missing the point, or at least I think you are.  Suppose
>> somebody creates a big table and then deletes all the tuples in the
>> second half, but VACUUM never runs.  When at last VACUUM does run on
>> that table, it will try to build the VM and FSM forks as it scans the
>> table, and will only truncate AFTER that has been done.  If building
>> the VM and FSM forks fails because there is no freespace, we will
>> never reach the part of the operation that could create some.
>
> No, I follow that perfectly.  I think you missed *my* point, which is:
> suppose that we do have a full-length VM and/or FSM fork for a relation,
> and VACUUM decides to truncate the relation.  Is it okay to not truncate
> the VM/FSM?  If it isn't, we're going to have to have very tricky
> semantics for any "don't touch the map forks" option, because it will
> have to suppress only some of VACUUM's map updates.

Oh, I see.  I think it's probably not a good idea to skip truncating
those maps, but perhaps the option could be defined as making no new
entries, rather than ignoring them altogether, so that they never
grow.  It seems that both of those are defined in such a way that if
page Y follows page X in the heap, the entries for page Y in the
relation fork will follow the one for page X.  So truncating them
should be OK; it's just growing them that we need to avoid.

> An alternative approach that might avoid such worries is to have a mode
> wherein VACUUM always truncates the map forks to nothing, rather than
> attempting to update them.

That could work, too, but might be stronger medicine than needed.

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

On Wed, Apr 6, 2016 at 3:32 AM, Asif Naeem <anaeem.it@gmail.com> wrote:
>> Oh, I see.  I think it's probably not a good idea to skip truncating
>> those maps, but perhaps the option could be defined as making no new
>> entries, rather than ignoring them altogether, so that they never
>> grow.  It seems that both of those are defined in such a way that if
>> page Y follows page X in the heap, the entries for page Y in the
>> relation fork will follow the one for page X.  So truncating them
>> should be OK; it's just growing them that we need to avoid.
>
> Thank you Robert. PFA basic patch, it introduces EMERGENCY option to VACUUM
> that forces to avoid extend any entries in the VM or FSM. It seems working
> fine in simple test scenarios e.g.
>
>> postgres=# create table test1 as (select generate_series(1,100000));
>> SELECT 100000
>> postgres=# vacuum  EMERGENCY test1;
>> VACUUM
>> postgres=# select pg_relation_filepath('test1');
>>  pg_relation_filepath
>> ----------------------
>>  base/13250/16384
>> (1 row)
>> [asif@centos66 inst_96]$ find . | grep base/13250/16384
>> ./data/base/13250/16384
>> postgres=# vacuum test1;
>> VACUUM
>> [asif@centos66 inst_96]$ find . | grep base/13250/16384
>> ./data/base/13250/16384
>> ./data/base/13250/16384_fsm
>> ./data/base/13250/16384_vm
>
>
> Please do let me know if I missed something or more information is required.
> Thanks.

This is too late for 9.6 at this point and certainly requires
discussion anyway, so please add it to the next CommitFest.  But in
the meantime, here are a few quick comments:

You should only support EMERGENCY using the new-style, parenthesized
options syntax.  That way, you won't need to make EMERGENCY a keyword.
We don't want to do that, and we especially don't want it to be
partially reserved, as you have done here.

Passing the EMERGENCY flag around in a global variable is probably not
a good idea; use parameter lists.  That's what they are for.  Also,
calling the variable that decides whether EMERGENCY was set
Extend_VM_FSM is confusing.

I see why you changed the calling convention for visibilitymap_pin()
and RecordPageWithFreeSpace(), but that's awfully invasive.  I wonder
if there's a better way to do that, like maybe having vacuumlazy.c ask
the VM and FSM for their length in pages and then not trying to use
those functions for block numbers that are too large.

Don't forget to update the documentation.

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

On 4/6/16 11:06 AM, Robert Haas wrote:
> This is too late for 9.6 at this point and certainly requires
> discussion anyway, so please add it to the next CommitFest.

If the goal here is to free up space via truncation when there's a real 
emergency, perhaps there's some other steps that should be taken as 
well. What immediately comes to mind is scanning the heap backwards and 
stopping on the first page we can't truncate.

There might be some other non-critical things we could skip in emergency 
mode, with an eye towards making it as fast as possible.

BTW, if someone really wanted to put some effort into this, it would be 
possible to better handle filling up a single filesystem that has both 
data and WAL by slowly shrinking the VM/FSM to make room in the WAL for 
vacuum records. ISTM that's a much more common problem people run into 
than filling up a separate tablespace.
-- 
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com

<div dir="ltr">Thank you Robert. Sure, I will add the updated patch on the next CommitFest with all the suggested
changes.<br/></div><div class="gmail_extra"><br /><div class="gmail_quote">On Wed, Apr 6, 2016 at 9:06 PM, Robert Haas
<spandir="ltr"><<a href="mailto:robertmhaas@gmail.com" target="_blank">robertmhaas@gmail.com</a>></span>
wrote:<br/><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div
class="HOEnZb"><divclass="h5">On Wed, Apr 6, 2016 at 3:32 AM, Asif Naeem <<a
href="mailto:anaeem.it@gmail.com">anaeem.it@gmail.com</a>>wrote:<br /> >> Oh, I see.  I think it's probably
nota good idea to skip truncating<br /> >> those maps, but perhaps the option could be defined as making no
new<br/> >> entries, rather than ignoring them altogether, so that they never<br /> >> grow.  It seems that
bothof those are defined in such a way that if<br /> >> page Y follows page X in the heap, the entries for page Y
inthe<br /> >> relation fork will follow the one for page X.  So truncating them<br /> >> should be OK;
it'sjust growing them that we need to avoid.<br /> ><br /> > Thank you Robert. PFA basic patch, it introduces
EMERGENCYoption to VACUUM<br /> > that forces to avoid extend any entries in the VM or FSM. It seems working<br />
>fine in simple test scenarios e.g.<br /> ><br /> >> postgres=# create table test1 as (select
generate_series(1,100000));<br/> >> SELECT 100000<br /> >> postgres=# vacuum  EMERGENCY test1;<br />
>>VACUUM<br /> >> postgres=# select pg_relation_filepath('test1');<br /> >>  pg_relation_filepath<br
/>>> ----------------------<br /> >>  base/13250/16384<br /> >> (1 row)<br /> >> [asif@centos66
inst_96]$find . | grep base/13250/16384<br /> >> ./data/base/13250/16384<br /> >> postgres=# vacuum
test1;<br/> >> VACUUM<br /> >> [asif@centos66 inst_96]$ find . | grep base/13250/16384<br /> >>
./data/base/13250/16384<br/> >> ./data/base/13250/16384_fsm<br /> >> ./data/base/13250/16384_vm<br />
><br/> ><br /> > Please do let me know if I missed something or more information is required.<br /> >
Thanks.<br/><br /></div></div>This is too late for 9.6 at this point and certainly requires<br /> discussion anyway, so
pleaseadd it to the next CommitFest.  But in<br /> the meantime, here are a few quick comments:<br /><br /> You should
onlysupport EMERGENCY using the new-style, parenthesized<br /> options syntax.  That way, you won't need to make
EMERGENCYa keyword.<br /> We don't want to do that, and we especially don't want it to be<br /> partially reserved, as
youhave done here.<br /><br /> Passing the EMERGENCY flag around in a global variable is probably not<br /> a good
idea;use parameter lists.  That's what they are for.  Also,<br /> calling the variable that decides whether EMERGENCY
wasset<br /> Extend_VM_FSM is confusing.<br /><br /> I see why you changed the calling convention for
visibilitymap_pin()<br/> and RecordPageWithFreeSpace(), but that's awfully invasive.  I wonder<br /> if there's a
betterway to do that, like maybe having vacuumlazy.c ask<br /> the VM and FSM for their length in pages and then not
tryingto use<br /> those functions for block numbers that are too large.<br /><br /> Don't forget to update the
documentation.<br/><div class="HOEnZb"><div class="h5"><br /> --<br /> Robert Haas<br /> EnterpriseDB: <a
href="http://www.enterprisedb.com"rel="noreferrer" target="_blank">http://www.enterprisedb.com</a><br /> The Enterprise
PostgreSQLCompany<br /></div></div></blockquote></div><br /></div> 

On Wed, Apr 6, 2016 at 5:15 PM, Jim Nasby <Jim.Nasby@bluetreble.com> wrote:
> On 4/6/16 11:06 AM, Robert Haas wrote:
>> This is too late for 9.6 at this point and certainly requires
>> discussion anyway, so please add it to the next CommitFest.
>
> If the goal here is to free up space via truncation when there's a real
> emergency, perhaps there's some other steps that should be taken as well.
> What immediately comes to mind is scanning the heap backwards and stopping
> on the first page we can't truncate.
>
> There might be some other non-critical things we could skip in emergency
> mode, with an eye towards making it as fast as possible.

I think this comes down to what you think the remit of a VACUUM
(EMERGENCY) option ought to be.  I think it ought to do just enough to
make VACUUM succeed instead of failing, but you could argue it ought
to focus more specifically on freeing up space and skip everything
else it might normally do.

> BTW, if someone really wanted to put some effort into this, it would be
> possible to better handle filling up a single filesystem that has both data
> and WAL by slowly shrinking the VM/FSM to make room in the WAL for vacuum
> records. ISTM that's a much more common problem people run into than filling
> up a separate tablespace.

Really?  The VM and FSM are extremely tiny; that doesn't seem likely
to work out.

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

On Mon, Jun 20, 2016 at 7:28 AM, Asif Naeem <anaeem.it@gmail.com> wrote:
> Thank you for useful suggestions. PFA patch, I have tried to cover all the
> points mentioned.

Thanks for the new patch.  I think that you have failed to address
this point from my previous review:

# I see why you changed the calling convention for visibilitymap_pin()
# and RecordPageWithFreeSpace(), but that's awfully invasive.  I wonder
# if there's a better way to do that, like maybe having vacuumlazy.c ask
# the VM and FSM for their length in pages and then not trying to use
# those functions for block numbers that are too large.

The patch has gotten a lot smaller, and that's clearly good, but
introducing extended versions of those functions still seems like a
thing we should try to avoid. In particular, there's no way this hunk
is going to be acceptable:

@@ -286,6 +299,10 @@ visibilitymap_set(Relation rel, BlockNumber
heapBlk, Buffer heapBuf,    if (BufferIsValid(heapBuf) && BufferGetBlockNumber(heapBuf) != heapBlk)        elog(ERROR,
"wrongheap buffer passed to visibilitymap_set");
 

+    /* In case of invalid buffer just return */
+    if(vmBuf == InvalidBuffer)
+        return;
+    /* Check that we have the right VM page pinned */    if (!BufferIsValid(vmBuf) || BufferGetBlockNumber(vmBuf) !=
mapBlock)       elog(ERROR, "wrong VM buffer passed to visibilitymap_set");
 

You're going to have to find a different approach there.

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

On Thu, Sep 8, 2016 at 2:46 AM, Haribabu Kommi <kommi.haribabu@gmail.com> wrote:
> Patch needs rebase, it is failing to apply on latest master.
> And also there are some pending comment fix from Robert.

It's been almost three weeks and this hasn't been updated, so I think
it's pretty clear that it should be marked "Returned with Feedback" at
this point.  I'll go do that.  Asif, if you update the patch, you can
resubmit for the next CommitFest.  Please make sure that all review
comments already given are addressed in your next revision so that
reviewers don't waste time giving you the same comments again.

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