Re: Hot standby and removing VACUUM FULL

VACUUM FULL does a peculiar hack: once it's done moving tuples, and
before it truncates the relation, it calls RecordTransactionCommit to
mark the transaction as committed in clog and WAL, but the transaction
is still kept open in proc array. After it's done with truncating and
other cleanup, normal transaction commit performs
RecordTransactionCommit again as part of normal commit processing.

That causes some headaches for Hot Standby, ie. it needs to know to not
release locks yet when it sees the first commit record. At the moment,
it simply ignores the first commit record, but that's very dangerous. If
failover happens after the standby has seen the truncation record, but
not the 2nd commit record for the transaction, all data moved from the
truncated part will lost.

I'm sure that's fixable, but I believe we're quite committed to removing
VACUUM FULL altogether in this release. If that's going to happen, I can
just remove all the VACUUM FULL related code from the Hot Standby patch
now and not spend any more time on it.

I'm a bit hesitent to do that because that basically leaves me in the
exact situation I said before I wanted to avoid: If I commit Hot Standby
without dealing with VACUUM FULL, and no-one gets around to remove
VACUUM FULL (and more importantly, provide some replacement for it), I
will be forced to do it myself or fix Hot Standby to work with it after
all. I might be involved in the VACUUM FULLectomy anyway, but I don't
want to be pushed against the wall on it.

So I guess what I'm asking is: Does anyone see any show-stoppers in
removing VACUUM FULL, and does anyone want to step up to the plate and
promise to do it before release?

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

Heikki Linnakangas wrote:
> So I guess what I'm asking is: Does anyone see any show-stoppers in
> removing VACUUM FULL
Here's the disclaimers attached to the new VACUUM REPLACE implementation
from Itagaki:

"We still need traditional VACUUM FULL behavior for system catalog
because we cannot change relfilenode for them. Also, VACUUM FULL REPLACE
is not always better than traditional VACUUM FULL; the new version
requires additional disk space and might be slower if we have a few dead
tuples."

That first part seems like it would limit the ability to completely
discard the current behavior.

--
Greg Smith 2ndQuadrant Baltimore, MD
PostgreSQL Training, Services and Support
greg(at)2ndQuadrant(dot)com www.2ndQuadrant.com

Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> writes:
> So I guess what I'm asking is: Does anyone see any show-stoppers in
> removing VACUUM FULL, and does anyone want to step up to the plate and
> promise to do it before release?

I don't see much problem with rejecting VAC FULL in a HS master,
whether or not it gets removed altogether. Why not just do that
rather than write a lot of kluges?

regards, tom lane

Tom Lane wrote:
> Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> writes:
>> So I guess what I'm asking is: Does anyone see any show-stoppers in
>> removing VACUUM FULL, and does anyone want to step up to the plate and
>> promise to do it before release?
>
> I don't see much problem with rejecting VAC FULL in a HS master,
> whether or not it gets removed altogether. Why not just do that
> rather than write a lot of kluges?

Hmm. At the moment, no action is required in the master to allow hot
standby in the slave, except for turning on archiving. The additional
overhead of the extra logging that's needed in the master is small
enough that there has been no need for a switch.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> writes:
> Tom Lane wrote:
>> I don't see much problem with rejecting VAC FULL in a HS master,
>> whether or not it gets removed altogether. Why not just do that
>> rather than write a lot of kluges?

> Hmm. At the moment, no action is required in the master to allow hot
> standby in the slave, except for turning on archiving. The additional
> overhead of the extra logging that's needed in the master is small
> enough that there has been no need for a switch.

There's no equivalent of XLogArchivingActive()? I think there probably
should be. I find it really hard to believe that there won't be any
places where we need to know that we're an HS master. The original
design of WAL archiving didn't think we needed to know we were archiving
WAL, either, and look how many cases there are for that now.

regards, tom lane

Tom Lane wrote:
> Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> writes:
>> Tom Lane wrote:
>>> I don't see much problem with rejecting VAC FULL in a HS master,
>>> whether or not it gets removed altogether. Why not just do that
>>> rather than write a lot of kluges?
>
>> Hmm. At the moment, no action is required in the master to allow hot
>> standby in the slave, except for turning on archiving. The additional
>> overhead of the extra logging that's needed in the master is small
>> enough that there has been no need for a switch.
>
> There's no equivalent of XLogArchivingActive()? I think there probably
> should be. I find it really hard to believe that there won't be any
> places where we need to know that we're an HS master. The original
> design of WAL archiving didn't think we needed to know we were archiving
> WAL, either, and look how many cases there are for that now.

XLogArchivingMode() == false enables us to skip WAL-logging in
operations like CLUSTER or COPY, which is a big optimization. I don't
see anything like that in Hot Standby. There is a few small things that
could be skipped, but nothing noticeable.

It's great from usability point of view that you don't need to enable it
beforehand, especially because HS is also very useful when doing PITR
from a backup; it's too late to turn on the switch at that point. As
soon as we have a good reason to introduce a switch, let's do so, but
not before that.

If we want cop out of VACUUM FULL and HS issues, maybe we could just
kick out all clients from the standby when we see a WAL record from
VACUUM FULL. Not very elegant, but should work.

Anyway, I think I have enough courage now to just rip out the VACUUM
FULL support from HS. If VACUUM FULL is still there when we're ready to
go to beta, we can introduce a "do you want VACUUM FULL or hot standby?"
switch in the master, or some other ugly workaround.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> writes:
> Tom Lane wrote:
>> There's no equivalent of XLogArchivingActive()?

> XLogArchivingMode() == false enables us to skip WAL-logging in
> operations like CLUSTER or COPY, which is a big optimization. I don't
> see anything like that in Hot Standby. There is a few small things that
> could be skipped, but nothing noticeable.

Huh? Surely HS requires XLogArchivingMode as a prerequisite ...

> It's great from usability point of view that you don't need to enable it
> beforehand,

... which also means that I don't understand this statement.

> Anyway, I think I have enough courage now to just rip out the VACUUM
> FULL support from HS. If VACUUM FULL is still there when we're ready to
> go to beta, we can introduce a "do you want VACUUM FULL or hot standby?"
> switch in the master, or some other ugly workaround.

Agreed, we have more than enough worries without worrying about making
that work. (Do we have a list of open issues somewhere, so that this
won't get forgotten?)

regards, tom lane

Tom Lane wrote:
> Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> writes:
>> Tom Lane wrote:
>>> There's no equivalent of XLogArchivingActive()?
>
>> XLogArchivingMode() == false enables us to skip WAL-logging in
>> operations like CLUSTER or COPY, which is a big optimization. I don't
>> see anything like that in Hot Standby. There is a few small things that
>> could be skipped, but nothing noticeable.
>
> Huh? Surely HS requires XLogArchivingMode as a prerequisite ...

Oh, sure! But there's no switch that needs to be enabled in the master
in addition to that.

> (Do we have a list of open issues somewhere, so that this
> won't get forgotten?)

I'm keeping one as I bump into things. I'll post it again soon.
Actually, I should probably put it on the wiki.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> wrote:

> So I guess what I'm asking is: Does anyone see any show-stoppers in
> removing VACUUM FULL, and does anyone want to step up to the plate and
> promise to do it before release?

I'm working on "New VACUUM FULL" patch, that shrinks tables
using CLUSTER-like rewrites.
https://commitfest.postgresql.org/action/patch_view?id=202

What can I do for the Hot Standby issue?
VACUUM FULL is still reserved for system catalogs in my patch
because we cannot modify relfilenodes for the catalog tables.
Do you have solutions for it?

Regards,
---
ITAGAKI Takahiro
NTT Open Source Software Center

Itagaki Takahiro wrote:
> VACUUM FULL is still reserved for system catalogs in my patch
> because we cannot modify relfilenodes for the catalog tables.
> Do you have solutions for it?

Tom had an idea on that:

http://archives.postgresql.org/message-id/19750.1252094460@sss.pgh.pa.us

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

Greg Smith wrote:
> Heikki Linnakangas wrote:
>> So I guess what I'm asking is: Does anyone see any show-stoppers in
>> removing VACUUM FULL
> Here's the disclaimers attached to the new VACUUM REPLACE implementation
> from Itagaki:
>
> "We still need traditional VACUUM FULL behavior for system catalog
> because we cannot change relfilenode for them. Also, VACUUM FULL REPLACE
> is not always better than traditional VACUUM FULL; the new version
> requires additional disk space and might be slower if we have a few dead
> tuples."
>
> That first part seems like it would limit the ability to completely
> discard the current behavior.

For system catalog,s you could still use a utility like the one I
experimented with at
http://archives.postgresql.org/message-id/4AB15065.1000607@enterprisedb.com.
Essentially, do a bunch of dummy UPDATEs on the rows that you want to
move. It can cause serialization errors in concurrent updaters, like any
UPDATE, but I think it would be good enough for the narrow remaining use
case of shrinking system catalogs.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

On Tue, 2009-11-24 at 09:24 +0200, Heikki Linnakangas wrote:
> Greg Smith wrote:
> > Heikki Linnakangas wrote:
> >> So I guess what I'm asking is: Does anyone see any show-stoppers in
> >> removing VACUUM FULL
> > Here's the disclaimers attached to the new VACUUM REPLACE implementation
> > from Itagaki:
> >
> > "We still need traditional VACUUM FULL behavior for system catalog
> > because we cannot change relfilenode for them. Also, VACUUM FULL REPLACE
> > is not always better than traditional VACUUM FULL; the new version
> > requires additional disk space and might be slower if we have a few dead
> > tuples."
> >
> > That first part seems like it would limit the ability to completely
> > discard the current behavior.
>
> For system catalog,s you could still use a utility like the one I
> experimented with at
> http://archives.postgresql.org/message-id/4AB15065.1000607@enterprisedb.com.
> Essentially, do a bunch of dummy UPDATEs on the rows that you want to
> move. It can cause serialization errors in concurrent updaters, like any
> UPDATE, but I think it would be good enough for the narrow remaining use
> case of shrinking system catalogs.

call it VACUUM SHRINK ;)

and you will need to do a REINDEX after using it to get full benefit,
same as ordinary VACUUM or VACUUM FULL.

> --
> Heikki Linnakangas
> EnterpriseDB http://www.enterprisedb.com
>

On Sat, 2009-11-21 at 20:20 +0200, Heikki Linnakangas wrote:

> That causes some headaches for Hot Standby

I say leave HS as it is and we can clean up when we do the VFectomy.

It isn't really a headache, the code works easily enough. I agree its
ugly and it should eventually be removed.

Let's not make this any harder, or get involved with promises that we
may not be able to keep. I'd rather we had HS + SR than HS - VF for
example. VF is ugly but it isn't a priority.

--
Simon Riggs www.2ndQuadrant.com

On Sat, 2009-11-21 at 23:00 +0200, Heikki Linnakangas wrote:
> Tom Lane wrote:
> > Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> writes:
> >> Tom Lane wrote:
> >>> There's no equivalent of XLogArchivingActive()?
> >
> >> XLogArchivingMode() == false enables us to skip WAL-logging in
> >> operations like CLUSTER or COPY, which is a big optimization. I don't
> >> see anything like that in Hot Standby. There is a few small things that
> >> could be skipped, but nothing noticeable.
> >
> > Huh? Surely HS requires XLogArchivingMode as a prerequisite ...
>
> Oh, sure! But there's no switch that needs to be enabled in the master
> in addition to that.

We've tried hard to have it "just work". But I wonder whether we should
have a parameter to allow performance testing on the master? If nobody
finds any issues then we can remove it again, or at least make it a
hidden developer option.

--
Simon Riggs www.2ndQuadrant.com

Simon Riggs <simon(at)2ndQuadrant(dot)com> writes:
> Tom Lane wrote:
>> There's no equivalent of XLogArchivingActive()?

> We've tried hard to have it "just work". But I wonder whether we should
> have a parameter to allow performance testing on the master? If nobody
> finds any issues then we can remove it again, or at least make it a
> hidden developer option.

As long as there's not anything the master actually does differently
then I can't see where there'd be any performance testing to do. What's
bothering me about this is that it seems likely that we'll find places
where the master has to do things differently. I'd rather we made the
status visible; if we get through a release cycle without needing to
check it, we can always take the function out again. But if we don't,
and then find out midway through the 8.5 release cycle that we need to
be able to check it, things could be a bit sticky.

regards, tom lane

On Wed, Nov 25, 2009 at 2:10 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> As long as there's not anything the master actually does differently
> then I can't see where there'd be any performance testing to do.  What's
> bothering me about this is that it seems likely that we'll find places
> where the master has to do things differently.  I'd rather we made the
> status visible; if we get through a release cycle without needing to
> check it, we can always take the function out again.  But if we don't,
> and then find out midway through the 8.5 release cycle that we need to
> be able to check it, things could be a bit sticky.

Well the only thing that's been discussed is having vacuum require a
minimum age before considering a transaction visible to all to reduce
the chance of conflicts on cleanup records. But that would require an
actual tunable, not just a flag. And it's something that could
conceivably be desirable even if you're not running a HS setup (if
someone ever reimplements time travel for example).

So I'm not sure adding a flag before there's an actual need for it is
necessarily going to be helpful. It may turn out to be insufficient
even if we have a flag.

And then there's the question of what the slave should do if the
master was running without the flag. Do we make it throw an error?
Does that mean the master needs to insert information to that effect
in the wal logs? What if you shut down the master switch the flag and
start it up again and you had a standby reading those logs all along.
Will it be able to switch to HS mode now? We won't know until we know
why this flag was necessary and what change in behaviour it might have
caused.

--
greg

Greg Stark <gsstark(at)mit(dot)edu> writes:
> Well the only thing that's been discussed is having vacuum require a
> minimum age before considering a transaction visible to all to reduce
> the chance of conflicts on cleanup records.

[ shrug... ] Call me Cassandra. I am not concerned about what has or
has not been discussed. I am concerned about what effects we are going
to be blindsided by, a few months from now when it is too late to
conveniently add a way to detect that the system is being run as an HS
master. If we design it in, perhaps we won't need it --- but if we
design it out, we will need it. You have heard of Finagle's law, no?

regards, tom lane

On Wed, 2009-11-25 at 03:12 +0000, Greg Stark wrote:
> On Wed, Nov 25, 2009 at 2:10 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > As long as there's not anything the master actually does differently
> > then I can't see where there'd be any performance testing to do. What's
> > bothering me about this is that it seems likely that we'll find places
> > where the master has to do things differently. I'd rather we made the
> > status visible; if we get through a release cycle without needing to
> > check it, we can always take the function out again. But if we don't,
> > and then find out midway through the 8.5 release cycle that we need to
> > be able to check it, things could be a bit sticky.
>
> Well the only thing that's been discussed is having vacuum require a
> minimum age before considering a transaction visible to all to reduce
> the chance of conflicts on cleanup records. But that would require an
> actual tunable, not just a flag. And it's something that could
> conceivably be desirable even if you're not running a HS setup (if
> someone ever reimplements time travel for example).

I will add this also, if it looks simple to do so. Even if we yank it
out later better to have the code for discussion purposes than just a
conceptual bikeshed.

> So I'm not sure adding a flag before there's an actual need for it is
> necessarily going to be helpful. It may turn out to be insufficient
> even if we have a flag.

Same situation as in archiving.

The debate was eventually carried that we should have
archive_mode = on
archive_xxxx = NNNN for additional parameters

> And then there's the question of what the slave should do if the
> master was running without the flag. Do we make it throw an error?

Well, it can't even enter HS mode, so no error needed.

> Does that mean the master needs to insert information to that effect
> in the wal logs? What if you shut down the master switch the flag and
> start it up again and you had a standby reading those logs all along.
> Will it be able to switch to HS mode now? We won't know until we know
> why this flag was necessary and what change in behaviour it might have
> caused.

I'm more comfortable running a new machine when it has an "off" switch.

--
Simon Riggs www.2ndQuadrant.com

On Wed, Nov 25, 2009 at 3:26 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Greg Stark <gsstark(at)mit(dot)edu> writes:
>> Well the only thing that's been discussed is having vacuum require a
>> minimum age before considering a transaction visible to all to reduce
>> the chance of conflicts on cleanup records.
>
> [ shrug... ]  Call me Cassandra.  I am not concerned about what has or
> has not been discussed.  I am concerned about what effects we are going
> to be blindsided by, a few months from now when it is too late to
> conveniently add a way to detect that the system is being run as an HS
> master.  If we design it in, perhaps we won't need it --- but if we
> design it out, we will need it.  You have heard of Finagle's law, no?

Well the point here was that the only inkling of a possible need for
this that we have is going to require more than an on/off switch
anyways. That's likely to be true of any need which arises.

And you didn't answer my questions about the semantics of this switch
will be. That a replica which starts up while reading wal logs
generated by this database will refuse connections even if it's
configured to allow them? How will it determine what the switch was on
the master? The value of the switch at what point in time? The answers
to these questions seem to depend on what the need which triggered the
existence of the switch was.

--
greg

On Wed, 2009-11-25 at 03:12 +0000, Greg Stark wrote:
> On Wed, Nov 25, 2009 at 2:10 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > As long as there's not anything the master actually does differently
> > then I can't see where there'd be any performance testing to do. What's
> > bothering me about this is that it seems likely that we'll find places
> > where the master has to do things differently. I'd rather we made the
> > status visible; if we get through a release cycle without needing to
> > check it, we can always take the function out again. But if we don't,
> > and then find out midway through the 8.5 release cycle that we need to
> > be able to check it, things could be a bit sticky.
>
> Well the only thing that's been discussed is having vacuum require a
> minimum age before considering a transaction visible to all to reduce
> the chance of conflicts on cleanup records. But that would require an
> actual tunable, not just a flag. And it's something that could
> conceivably be desirable even if you're not running a HS setup (if
> someone ever reimplements time travel for example).

I've added "vacuum_delay_cleanup_age = N", default 0 to implement this.
New name please.

This alters the return value of GetOldestXmin() and the setting of
RecentGlobalXmin by the value requested.

I've made this a SIGHUP, since it only has value if it affects all
users.

--
Simon Riggs www.2ndQuadrant.com

On Sat, 2009-11-21 at 20:20 +0200, Heikki Linnakangas wrote:

> VACUUM FULL does a peculiar hack: once it's done moving tuples, and
> before it truncates the relation, it calls RecordTransactionCommit to
> mark the transaction as committed in clog and WAL, but the transaction
> is still kept open in proc array. After it's done with truncating and
> other cleanup, normal transaction commit performs
> RecordTransactionCommit again as part of normal commit processing.
>
> That causes some headaches for Hot Standby, ie. it needs to know to not
> release locks yet when it sees the first commit record. At the moment,
> it simply ignores the first commit record, but that's very dangerous. If
> failover happens after the standby has seen the truncation record, but
> not the 2nd commit record for the transaction, all data moved from the
> truncated part will lost.

...

> So I guess what I'm asking is: Does anyone see any show-stoppers in
> removing VACUUM FULL, and does anyone want to step up to the plate and
> promise to do it before release?

I've just reviewed the VACUUM FULL patch to see if it does all we need
it to do, i.e. does the removal of HS code match the new VF patch.

My answer is it doesn't, we will still have the problem noted above for
catalog tables. So we still have a must-fix issue for HS, though that is
no barrier to the new VF patch.

Requirement is that

* we ignore first commit, since it has an identical xid to second
commit, so requires a special case to avoid breaking other checks

* we musn't truncate until we are certain transaction completes,
otherwise we will have a data loss situation (isolated to this
particular case only)

Proposal:

* (In normal running) Just before we issue the first VFI commit we send
a new type of WAL message to indicate VFI-in-progress, including the xid

* (In HS recovery) When we see first commit record for the VF xid we
ignore it, as we did in the original patch

* (In HS recovery) When we see relation truncate for the xid we ignore
it for now, but defer until after the second commit is processed

It ain't that great, but it means all of the "hack" is isolated to
specific HS-only code, which can be turned off if required.

--
Simon Riggs www.2ndQuadrant.com

Simon Riggs wrote:
> I've just reviewed the VACUUM FULL patch to see if it does all we need
> it to do, i.e. does the removal of HS code match the new VF patch.

Oh, good!

> My answer is it doesn't, we will still have the problem noted above for
> catalog tables. So we still have a must-fix issue for HS, though that is
> no barrier to the new VF patch.

I think the VACUUM FULL patch will have to address that. Either with the
flat-file approach Tom suggested, or by disallowing VACUUM FULL for
catalog tables altogether, requiring you to use the to-be-written online
tool that uses dummy UPDATEs to move tuples.

> Requirement is that
>
> * we ignore first commit, since it has an identical xid to second
> commit, so requires a special case to avoid breaking other checks
>
> * we musn't truncate until we are certain transaction completes,
> otherwise we will have a data loss situation (isolated to this
> particular case only)
>
> Proposal:
>
> * (In normal running) Just before we issue the first VFI commit we send
> a new type of WAL message to indicate VFI-in-progress, including the xid
>
> * (In HS recovery) When we see first commit record for the VF xid we
> ignore it, as we did in the original patch
>
> * (In HS recovery) When we see relation truncate for the xid we ignore
> it for now, but defer until after the second commit is processed
>
> It ain't that great, but it means all of the "hack" is isolated to
> specific HS-only code, which can be turned off if required.

Could you just mark the transaction as committed when you see the 1st
commit record, but leave the XID in the known-assigned list and not
release locks? That would emulate pretty closely what happens in the master.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

On Fri, 2009-12-04 at 11:22 +0200, Heikki Linnakangas wrote:

> Could you just mark the transaction as committed when you see the 1st
> commit record, but leave the XID in the known-assigned list and not
> release locks? That would emulate pretty closely what happens in the master.

OK, works for me. Thanks.

I thought of that before and dismissed it, clearly before my morning
coffee, but it works because we still hold the lock on the table so
nobody can actually read the now visible new rows. Cool.

--
Simon Riggs www.2ndQuadrant.com

On Fri, 2009-12-04 at 11:22 +0200, Heikki Linnakangas wrote:

> > My answer is it doesn't, we will still have the problem noted above for
> > catalog tables. So we still have a must-fix issue for HS, though that is
> > no barrier to the new VF patch.
>
> I think the VACUUM FULL patch will have to address that. Either with the
> flat-file approach Tom suggested, or by disallowing VACUUM FULL for
> catalog tables altogether, requiring you to use the to-be-written online
> tool that uses dummy UPDATEs to move tuples.

I don't see we need either of those, with the solution below.

ISTM premature to remove all traces of VF from code. We may yet need it
for some reason, especially if doing so creates complex dependencies on
important features.

> > Proposal:
> >
> > * (In normal running) Just before we issue the first VFI commit we send
> > a new type of WAL message to indicate VFI-in-progress, including the xid
> >
> > * (In HS recovery) When we see first commit record for the VF xid we
> > ignore it, as we did in the original patch
> >
> > * (In HS recovery) When we see relation truncate for the xid we ignore
> > it for now, but defer until after the second commit is processed
> >
> > It ain't that great, but it means all of the "hack" is isolated to
> > specific HS-only code, which can be turned off if required.
>
> Could you just mark the transaction as committed when you see the 1st
> commit record, but leave the XID in the known-assigned list and not
> release locks? That would emulate pretty closely what happens in the master.

So modified proposal looks like this

1. (In normal running) Provide information to HS so it can identify VF
commit records.
Implement in code either
a) Just before we issue the first VFI commit we send a new type of WAL
message to indicate VFI-in-progress, including the xid.
b) Alter the API of RecordTransactionCommit(), and send the info within
the commit record. This was pretty much how we did that before.
I prefer (a) now because the ugliness is better isolated.

2. (In HS recovery) When we see first commit record for the VF xid we
commit the transaction in clog, yet maintain locks and KnownAssigned
xids

3. (In HS recovery) When we see second commit record for the VF xid we
skip clog updates but then perform remaining parts of commit.

Conceptually this splits a VF commit into two parts.

--
Simon Riggs www.2ndQuadrant.com

Simon Riggs wrote:
> ISTM premature to remove all traces of VF from code. We may yet need it
> for some reason, especially if doing so creates complex dependencies on
> important features.

Well, it's still in the repository.

> So modified proposal looks like this
>
> 1. (In normal running) Provide information to HS so it can identify VF
> commit records.
> Implement in code either
> a) Just before we issue the first VFI commit we send a new type of WAL
> message to indicate VFI-in-progress, including the xid.
> b) Alter the API of RecordTransactionCommit(), and send the info within
> the commit record. This was pretty much how we did that before.
> I prefer (a) now because the ugliness is better isolated.

With a), you need to keep track of the seen VFI-in-progress records,
remember to expire the state at a shutdown record etc. And you have to
deal with the possibility that a checkpoint happens between the
VFI-in-progress record and the commit record; a recovery starting from
the checkpoint/running-xacts record must see both records or it will
release the locks prematurely.

b) seems much simpler to me.

> 2. (In HS recovery) When we see first commit record for the VF xid we
> commit the transaction in clog, yet maintain locks and KnownAssigned
> xids
>
> 3. (In HS recovery) When we see second commit record for the VF xid we
> skip clog updates but then perform remaining parts of commit.

I's harmless to set a clog entry as committed twice, so you can treat
the 2nd commit record the same as a regular commit record.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

On Fri, 2009-12-04 at 13:31 +0200, Heikki Linnakangas wrote:

> b) seems much simpler to me.

OK. Least ugly wins, but she ain't pretty.

> > 2. (In HS recovery) When we see first commit record for the VF xid we
> > commit the transaction in clog, yet maintain locks and KnownAssigned
> > xids
> >
> > 3. (In HS recovery) When we see second commit record for the VF xid we
> > skip clog updates but then perform remaining parts of commit.
>
> I's harmless to set a clog entry as committed twice, so you can treat
> the 2nd commit record the same as a regular commit record.

Yeh, OK, it is harmless and makes code cleaner.

--
Simon Riggs www.2ndQuadrant.com