Home > Archive > PostgreSQL Hacks > September 2005 > Why does VACUUM FULL bother locking pages?









You are viewing an archived Text-only version of the thread. To view this thread in it's original format and/or if you want to reply to this thread please [click here]

 

Author Why does VACUUM FULL bother locking pages?
Alvaro Herrera

2005-09-16, 1:23 pm

Hackers,

I'm reading the vacuum code and I just noticed that the routines
move_plain_tuple and move_chain_tuple expect the dest and source blocks
to be locked, and unlock them at exit. The only caller of both is
repair_frag, whose only caller in turn is full_vacuum_rel. Same thing
for update_hint_bits.

So, the only callers of both has already acquired appropiate locks at
the relation level -- nobody is going to be modifying the blocks while
they proceed. So why bother locking the pages at all? Is there a
reason or is this an historical accident?

--
Alvaro Herrera -- Valdivia, Chile Architect, www.EnterpriseDB.com
"Puedes vivir solo una vez, pero si lo haces bien, una vez es suficiente"

---------------------------(end of broadcast)---------------------------
TIP 9: In versions below 8.0, the planner will ignore your desire to
choose an index scan if your joining column's datatypes do not
match

Jonah H. Harris

2005-09-16, 8:24 pm

Was it relcache related?

On 9/16/05, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>
> Hackers,
>
> I'm reading the vacuum code and I just noticed that the routines
> move_plain_tuple and move_chain_tuple expect the dest and source blocks
> to be locked, and unlock them at exit. The only caller of both is
> repair_frag, whose only caller in turn is full_vacuum_rel. Same thing
> for update_hint_bits.
>
> So, the only callers of both has already acquired appropiate locks at
> the relation level -- nobody is going to be modifying the blocks while
> they proceed. So why bother locking the pages at all? Is there a
> reason or is this an historical accident?
>
> --
> Alvaro Herrera -- Valdivia, Chile Architect, www.EnterpriseDB.com<http://www.EnterpriseDB.com>
> "Puedes vivir solo una vez, pero si lo haces bien, una vez es suficiente"
>
> ---------------------------(end of broadcast)---------------------------
> TIP 9: In versions below 8.0, the planner will ignore your desire to
> choose an index scan if your joining column's datatypes do not
> match
>




--
Respectfully,

Jonah H. Harris, Database Internals Architect
EnterpriseDB Corporation
http://www.enterprisedb.com/

Alvaro Herrera

2005-09-16, 8:24 pm

On Fri, Sep 16, 2005 at 04:41:39PM -0400, Jonah H. Harris wrote:
> Was it relcache related?


I don't see how -- any user of a relcache entry needs to heap_open() or
relation_open() the table and acquire an appropiate lock. Any such call
would block because of the lock that VACUUM FULL acquires on the relation.
[color=darkred]
> On 9/16/05, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:

--
Alvaro Herrera -- Valdivia, Chile Architect, www.EnterpriseDB.com
"Now I have my system running, not a byte was off the shelf;
It rarely breaks and when it does I fix the code myself.
It's stable, clean and elegant, and lightning fast as well,
And it doesn't cost a nickel, so Bill Gates can go to hell."

---------------------------(end of broadcast)---------------------------
TIP 2: Don't 'kill -9' the postmaster

Jonah H. Harris

2005-09-16, 8:24 pm

I'm probably wrong, but I thought vacuum may invalidate stuff which
semi-required the cache to be flushed. :) I'll go take a look through
as-well but it's hard to imagine this being overlooked for so long.

Sorry Alvaro, I haven't gone out to look at vacuum in awhile so I ain't much
help.


On 9/16/05, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>
> On Fri, Sep 16, 2005 at 04:41:39PM -0400, Jonah H. Harris wrote:
>
> I don't see how -- any user of a relcache entry needs to heap_open() or
> relation_open() the table and acquire an appropiate lock. Any such call
> would block because of the lock that VACUUM FULL acquires on the relation..
>
> blocks
>
> --
> Alvaro Herrera -- Valdivia, Chile Architect, www.EnterpriseDB.com<http://www.EnterpriseDB.com>
> "Now I have my system running, not a byte was off the shelf;
> It rarely breaks and when it does I fix the code myself.
> It's stable, clean and elegant, and lightning fast as well,
> And it doesn't cost a nickel, so Bill Gates can go to hell."
>




--
Respectfully,

Jonah H. Harris, Database Internals Architect
EnterpriseDB Corporation
http://www.enterprisedb.com/

Simon Riggs

2005-09-16, 8:24 pm

> Alvaro Herrera wrote

> The only caller of both is
> repair_frag, whose only caller in turn is full_vacuum_rel.


....bgwriter still needs to access blocks. The WAL system relies on the
locking behaviour for recoverability, see comments in LockBuffer() and
SyncOneBuffer().

....I do think there's lots still to optimise in VACUUM FULL though...

Best Regards, Simon Riggs


---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend

Tom Lane

2005-09-17, 3:24 am

Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> So, the only callers of both has already acquired appropiate locks at
> the relation level -- nobody is going to be modifying the blocks while
> they proceed. So why bother locking the pages at all? Is there a
> reason or is this an historical accident?


No, because operations such as checkpointing and bgwriter will feel free
to write out pages that aren't exclusive-locked; they don't try to get
a lock at the table level. Failing to lock the buffer would risk
allowing an invalid page state to be written to disk --- which, if we
then crashed before writing the WAL record for the vacuum operation,
would represent unrecoverable corruption.

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 5: don't forget to increase your free space map settings

Alvaro Herrera

2005-09-22, 11:24 am

On Fri, Sep 16, 2005 at 11:50:21PM -0700, Simon Riggs wrote:
>
> ...bgwriter still needs to access blocks. The WAL system relies on the
> locking behaviour for recoverability, see comments in LockBuffer() and
> SyncOneBuffer().


Oh, certainly! In this case, may I point out that scan_heap() does not
bother locking pages, mentioning that "we assume that holding exclusive
lock on the relation will keep other backends from looking at the page".
In particular, it calls PageRepairFragmentat
ion which runs with the page
unlocked AFAICT.

Seems like a bug to me.

--
Alvaro Herrera http://www.amazon.com/gp/registry/CTMLCN8V17R4
"Now I have my system running, not a byte was off the shelf;
It rarely breaks and when it does I fix the code myself.
It's stable, clean and elegant, and lightning fast as well,
And it doesn't cost a nickel, so Bill Gates can go to hell."

---------------------------(end of broadcast)---------------------------
TIP 2: Don't 'kill -9' the postmaster

Alvaro Herrera

2005-09-22, 11:24 am

On Thu, Sep 22, 2005 at 10:36:41AM -0400, Alvaro Herrera wrote:
> On Fri, Sep 16, 2005 at 11:50:21PM -0700, Simon Riggs wrote:
>
> Oh, certainly! In this case, may I point out that scan_heap() does not
> bother locking pages, mentioning that "we assume that holding exclusive
> lock on the relation will keep other backends from looking at the page".
> In particular, it calls PageRepairFragmentat
ion which runs with the page
> unlocked AFAICT.


Looking again, PageRepairFragmentat
ion is called on a copy of the page,
not on the page itself, so this is not a problem. The page is only
modified to exchange old Xids for FrozenTransactionId,
or to set some
hint bits, so this really shouldn't be too much of a problem. I still
think it would be better to lock the page beforehand.

--
Alvaro Herrera Architect, http://www.EnterpriseDB.com
"Uno puede defenderse de los ataques; contra los elogios se esta indefenso"

---------------------------(end of broadcast)---------------------------
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo@postgresql
.org so that your
message can get through to the mailing list cleanly

Tom Lane

2005-09-22, 11:24 am

Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> Oh, certainly! In this case, may I point out that scan_heap() does not
> bother locking pages, mentioning that "we assume that holding exclusive
> lock on the relation will keep other backends from looking at the page".
> In particular, it calls PageRepairFragmentat
ion which runs with the page
> unlocked AFAICT.


> Seems like a bug to me.


I agree --- and a pretty silly one considering that there are LockBuffer
calls elsewhere in vacuum.c. Wonder how old that code is ...

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 5: don't forget to increase your free space map settings

Simon Riggs

2005-09-22, 1:24 pm

On Thu, 2005-09-22 at 10:36 -0400, Alvaro Herrera wrote:

> Seems like a bug to me.


Well done. This wins the award for best bug found during beta; shame it
wasn't 8.0 beta!

Just as well we recommend only doing VACUUM FULL when the system is
quiet....

Best Regards, Simon Riggs



---------------------------(end of broadcast)---------------------------
TIP 2: Don't 'kill -9' the postmaster

Tom Lane

2005-09-22, 1:24 pm

Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> Looking again, PageRepairFragmentat
ion is called on a copy of the page,
> not on the page itself, so this is not a problem. The page is only
> modified to exchange old Xids for FrozenTransactionId,
or to set some
> hint bits, so this really shouldn't be too much of a problem. I still
> think it would be better to lock the page beforehand.


Actually, the case that's a bit worrisome is the PageIsNew path: it'd be
possible for a partially-valid page header to be written out. This
wouldn't result in data loss, exactly, since there's nothing on the page
.... but we might have a problem using the page later.

The FrozenTransactionId update case is already presumed to be atomic by
vacuumlazy.c, so I don't feel too bad about it, but it surely needs a
comment at least.

On the whole it seems like we might as well just take the exclusive
buffer lock and not try to be cute.

AFAICT the other routines in vacuum.c all do proper locking when they
are modifying pages, so it's just this one place that is taking a short
cut.

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 4: Have you searched our list archives?

http://archives.postgresql.org

Sponsored Links





Also available: Server administration forum archive | Web Design forum archive | Software forum archive | Hardware reviews archive | Programming forum archive

Copyright 2008 droptable.com