pgsql/src/backend/commands (command.c vacuum.c)

-----Original Message-----
From: Jan Wieck
Sent: Thursday, July 06, 2000 1:18 AM
To: pgsql-committers@postgresql.org
Subject: [COMMITTERS] pgsql/src/backend/commands (command.c vacuum.c)

Date: Wednesday, July 5, 2000 @ 12:17:41
Author: wieck

Update of /home/projects/pgsql/cvsroot/pgsql/src/backend/commands
from hub.org:/tmp/cvs-serv21402/backend/commands

Modified Files:
command.c vacuum.c

----------------------------- Log Message -----------------------------

Changed TOAST relations to have relkind RELKIND_TOASTVALUE.

Special handling of TOAST relations during VACUUM. TOAST relations
are vacuumed while the lock on the master table is still active.

It seems very dangerous to me.
When VACUUM of a master table was finished, the transaction is
in already committed state in many cases.

Regards.
Hiroshi Inoue

"Hiroshi Inoue" <Inoue@tpf.co.jp> writes:

Special handling of TOAST relations during VACUUM. TOAST relations
are vacuumed while the lock on the master table is still active.

It seems very dangerous to me.
When VACUUM of a master table was finished, the transaction is
in already committed state in many cases.

I don't see the problem. If the toast table doesn't get vacuumed,
no real harm is done other than failing to recover space.

regards, tom lane

-----Original Message-----
From: Tom Lane [mailto:tgl@sss.pgh.pa.us]

"Hiroshi Inoue" <Inoue@tpf.co.jp> writes:

Special handling of TOAST relations during VACUUM. TOAST relations
are vacuumed while the lock on the master table is still active.

It seems very dangerous to me.
When VACUUM of a master table was finished, the transaction is
in already committed state in many cases.

I don't see the problem. If the toast table doesn't get vacuumed,
no real harm is done other than failing to recover space.

Hmm,is there any good reason to vacuum toast table in the
transaction which was already internally committed by vacuum
of the master table ? Is it possible under WAL ?

Regards.
Hiroshi Inoue

"Hiroshi Inoue" <Inoue@tpf.co.jp> writes:

Hmm,is there any good reason to vacuum toast table in the
transaction which was already internally committed by vacuum
of the master table ? Is it possible under WAL ?

It had better be possible under WAL, because vacuuming indexes is
done in essentially the same way: we clean the indexes *after* we
commit the master's tuple movements.

Really, the TOAST table is being treated the same way we handle
indexes, and I think that's good.

regards, tom lane

-----Original Message-----
From: Tom Lane [mailto:tgl@sss.pgh.pa.us]

"Hiroshi Inoue" <Inoue@tpf.co.jp> writes:

Hmm,is there any good reason to vacuum toast table in the
transaction which was already internally committed by vacuum
of the master table ? Is it possible under WAL ?

It had better be possible under WAL, because vacuuming indexes is
done in essentially the same way: we clean the indexes *after* we
commit the master's tuple movements.

There's no command other than VACUUM which continues to
access table/index after *commit*. We couldn't process
significant procedures in such an already commiitted state,
could we ?

Really, the TOAST table is being treated the same way we handle
indexes, and I think that's good.

If I recognize correctly,TOAST table is a table not an index and
is little different from ordinary tables. VACUUM now vacuums
2 tables in a transaction for tables with TOAST columns.
^^^^^^^^^^^^^^^^^^
I don't think it's right and my question is simple.
What's wrong with vacuuming master and the toast table in
separate transactions ?

Regrads.
Hiroshi Inoue

"Hiroshi Inoue" <Inoue@tpf.co.jp> writes:

There's no command other than VACUUM which continues to
access table/index after *commit*. We couldn't process
significant procedures in such an already commiitted state,
could we ?

Why not? The intermediate state *is valid*. We just haven't
removed no-longer-referenced index and TOAST entries yet.

What's wrong with vacuuming master and the toast table in
separate transactions ?

You'd have to give up the lock on the master table if there were
a true commit. I don't want to do that ... especially not when
I don't believe there is a problem to fix.

regards, tom lane

Tom Lane wrote:

"Hiroshi Inoue" <Inoue@tpf.co.jp> writes:

There's no command other than VACUUM which continues to
access table/index after *commit*. We couldn't process
significant procedures in such an already commiitted state,
could we ?

Why not? The intermediate state *is valid*. We just haven't
removed no-longer-referenced index and TOAST entries yet.

Do you mean *already committed* state has no problem and
VACUUM is always possible in the state ?
Is VACUUM such a trivial job ?

What's wrong with vacuuming master and the toast table in
separate transactions ?

You'd have to give up the lock on the master table if there were
a true commit. I don't want to do that ... especially not when
I don't believe there is a problem to fix.

Hmmm,is keeping the lock on master table more important than
risking to break consistency ?

Regards.
Hiroshi Inoue

Hiroshi Inoue <Inoue@tpf.co.jp> writes:

Tom Lane wrote:

Why not? The intermediate state *is valid*. We just haven't
removed no-longer-referenced index and TOAST entries yet.

Do you mean *already committed* state has no problem and
VACUUM is always possible in the state ?

Yes. Otherwise VACUUM wouldn't be crash-safe.

Hmmm,is keeping the lock on master table more important than
risking to break consistency ?

I see no consistency risk here. I'd be more worried about potential
risks from dropping the lock too soon.

regards, tom lane

Tom Lane wrote:

Hiroshi Inoue <Inoue@tpf.co.jp> writes:

Tom Lane wrote:

Why not? The intermediate state *is valid*. We just haven't
removed no-longer-referenced index and TOAST entries yet.

Do you mean *already committed* state has no problem and
VACUUM is always possible in the state ?

Yes. Otherwise VACUUM wouldn't be crash-safe.

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.

Hmmm,is keeping the lock on master table more important than
risking to break consistency ?

I see no consistency risk here. I'd be more worried about potential
risks from dropping the lock too soon.

Thers's no potential risk other than deadlock.
If we have to avoid deadlock we could acquire
the lock on master table first. Is there any
problem ?

Regards.
Hiroshi Inoue

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...

regards, tom lane

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

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

"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 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.

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