Thread: RE: Is VACUUM still crash-safe?
> > If we move tuples in already committed state, a page with new > > tuple position goes to disk and backend crashes before page with > > old tuple position updated then we'll have two version of tuple > > after restart (new tuple with HEAP_MOVED_IN is valid and there is > > no HEAP_MOVED_OFF in old tuple version). > > That's not good. Perhaps VACUUM still needs to fsync the file before > its internal commit? Ops, sorry - this case is not relevant to 7.1: WAL guarantees that both pages will be updated on restart. Seems we are safe now. Vadim
> Hiroshi Inoue <Inoue@tpf.co.jp> writes: > > When VACUUM for a table starts, the transaction is not > > committed yet of cource. After *commit* VACUUM has handled > > heap/index tuples very carefully to be crash-safe before > > 7.1. Currently another vacuum could be invoked in the > > already committed transaction. There has been no such > > situation before 7.1. Yes,VACUUM isn't crash-safe now. > > Vadim, do you agree with this argument? If so, I think it's > something we need to fix. I don't see what Hiroshi is worried > about, myself, but if there really is an issue here... If we move tuples in already committed state, a page with new tuple position goes to disk and backend crashes before page with old tuple position updated then we'll have two version of tuple after restart (new tuple with HEAP_MOVED_IN is valid and there is no HEAP_MOVED_OFF in old tuple version). I don't know how bad is it for TOAST tables though. Vadim P.S. I had no Inet access on weekends - my home phone line was broken...
"Mikheev, Vadim" <vmikheev@SECTORBASE.COM> writes:
> If we move tuples in already committed state, a page with new
> tuple position goes to disk and backend crashes before page with
> old tuple position updated then we'll have two version of tuple
> after restart (new tuple with HEAP_MOVED_IN is valid and there is
> no HEAP_MOVED_OFF in old tuple version).
That's not good. Perhaps VACUUM still needs to fsync the file before
its internal commit?
> I don't know how bad is it for TOAST tables though.
I still don't see anything here that affects the handling of TOAST
tables, which was Hiroshi's original complaint.
regards, tom lane
"Mikheev, Vadim" wrote: > > > > If we move tuples in already committed state, a page with new > > > tuple position goes to disk and backend crashes before page with > > > old tuple position updated then we'll have two version of tuple > > > after restart (new tuple with HEAP_MOVED_IN is valid and there is > > > no HEAP_MOVED_OFF in old tuple version). > > > > That's not good. Perhaps VACUUM still needs to fsync the file before > > its internal commit? > > Ops, sorry - this case is not relevant to 7.1: WAL guarantees that > both pages will be updated on restart. Seems we are safe now. > First,already committed state isn't a normal state at least without WAL. We must have access to db as less as possible in the state without WAL. AFAIK there has been no proof that we are sufficently safe in the state under WAL. Don't you have to prove it if you dare to do another vacuum in the state ? Second,isn't the following an example that VACUUM isn't crash-safe. VACUUM of a toast table crashed immediately after the movementof a tuple(and before inserting corresponding index tuples).Unfortunatelythe movement of a tuple is directly committed inalready committed state but corresponding index tuplesaren'tinserted. Regards. Hiroshi Inoue
> > Ops, sorry - this case is not relevant to 7.1: WAL guarantees that > > both pages will be updated on restart. Seems we are safe now. > > First,already committed state isn't a normal state at least > without WAL. We must have access to db as less as possible in the > state without WAL. > AFAIK there has been no proof that we are sufficently safe in the > state under WAL. Don't you have to prove it if you dare to do another > vacuum in the state ? > > Second,isn't the following an example that VACUUM isn't crash-safe. > > VACUUM of a toast table crashed immediately after the movement > of a tuple(and before inserting corresponding index tuples). > Unfortunately the movement of a tuple is directly committed in > already committed state but corresponding index tuples aren't > inserted. Now you've won -:) Vadim
Hiroshi Inoue <Inoue@tpf.co.jp> writes:
> VACUUM of a toast table crashed immediately after the movement
> of a tuple(and before inserting corresponding index tuples).
> Unfortunately the movement of a tuple is directly committed in
> already committed state but corresponding index tuples aren't
> inserted.
Ah, *now* I see what you're talking about. You're right, the TOAST
table has to be vacuumed under a separate transaction number.
I still don't like releasing the lock on the master table though.
VACUUM cheats on the commit already, could it start a new transaction
number without releasing the lock?
regards, tom lane
Tom Lane wrote: > > Hiroshi Inoue <Inoue@tpf.co.jp> writes: > > VACUUM of a toast table crashed immediately after the movement > > of a tuple(and before inserting corresponding index tuples). > > Unfortunately the movement of a tuple is directly committed in > > already committed state but corresponding index tuples aren't > > inserted. > > Ah, *now* I see what you're talking about. You're right, the TOAST > table has to be vacuumed under a separate transaction number. > > I still don't like releasing the lock on the master table though. > VACUUM cheats on the commit already, could it start a new transaction > number without releasing the lock? > It is also preferable that we could replace current intermediate *commit* of vacuum by real commit(without releaseing the lock). IIRC,Vadim and I talked about it a little once before. We could avoid releasing the lock at commit time but probably the next StartTransaction() has to change xid-s of LockTable entries. I'm not sure if it's sufficient or not. For example We could hardly keep row-level lock. We could acquire the lock on a row which is already committed. Regards. Hiroshi Inoue