Re: Truncating/vacuuming relations on full tablespaces

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.

/* A few variables that don't seem worth passing around as parameters */
static int elevel = -1;
static TransactionId OldestXmin;
static TransactionId FreezeLimit;
static MultiXactId MultiXactCutoff;
static BufferAccessStrategy vac_strategy;

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