From: Tom Lane on
Simon Riggs <simon(a)2ndQuadrant.com> writes:
> Having said that I now realise a few things I didn't before:

> * Approach (2) effects the core of Postgres, even if you don't use Hot
> Standby.
> * I've had to remove 7 sanity checks to get the first few VACUUMs
> working. ISTM that removing various basic checks in the code is not a
> good thing.
> * There are are more special cases than I realised at first: temp,
> shared, with-toast, nailed, shared-and-nailed, pg_class, normal system.

Quite honestly, these statements and the attached patch (which doesn't
even begin to touch the central issue, but does indeed break a lot of
things) demonstrate that *you* are not the guy to implement what was
being discussed. It needs to be done by someone who understands the
core caching code, which apparently you haven't studied in any detail.

I have a feeling that I should go do this...

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Simon Riggs on
On Sun, 2010-01-31 at 14:35 -0500, Tom Lane wrote:
> Simon Riggs <simon(a)2ndQuadrant.com> writes:
> > On Sat, 2010-01-30 at 15:17 -0500, Tom Lane wrote:
> >> Simon Riggs <simon(a)2ndQuadrant.com> writes:
> >>> The options to do this were and still are:
> >>> (1) Add WAL messages for non-transactional relcache invalidations
> >>> (2) Allow system relations to be cluster-ed/vacuum full-ed.
>
> >> Refresh my memory on why (1) lets us avoid fixing (2)?
>
> > (1) allows us to retain VACUUM FULL INPLACE for system relations, thus
> > avoiding the need to do (2). Non-transactional invalidations need to be
> > replayed in recovery for the same reason they exist on the primary.
>
> Well, I would expect that invalidation events need to be transmitted to
> hot-standby slaves no matter what --- backends running queries on an HS
> slave need to hear about inval events just as much as backends on the
> master do. So my take on it is that all inval events will have to have
> associated WAL records when in HS mode, independently of what we choose
> to do about VACUUM.

All transactional invalidations are already handled by HS. It was the
non-transactional invalidations in VACUUM FULL INPLACE that still
require additional handling.

> Anyway, it's still not apparent to me exactly what the connection is
> between VACUUM FULL and Hot Standby. I remember that we said HS didn't
> work with VACUUM FULL (INPLACE) but I don't recall why that is, and the
> links on the open-items pages are not leading me to any useful
> discussion.

Very little really; not enough to force the sort of changes that I am
now seeing will be required in the way catalogs and caches operate.
There was some difficulty around the fact that VFI issues two commits
for the same transaction, but that is now correctly handled in the code
after discussion.

I'm not known as a risk-averse person but (2) is a step too far for me.

--
Simon Riggs www.2ndQuadrant.com


--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Simon Riggs on
On Sun, 2010-01-31 at 14:44 -0500, Tom Lane wrote:
> Simon Riggs <simon(a)2ndQuadrant.com> writes:
> > Having said that I now realise a few things I didn't before:
>
> > * Approach (2) effects the core of Postgres, even if you don't use Hot
> > Standby.
> > * I've had to remove 7 sanity checks to get the first few VACUUMs
> > working. ISTM that removing various basic checks in the code is not a
> > good thing.
> > * There are are more special cases than I realised at first: temp,
> > shared, with-toast, nailed, shared-and-nailed, pg_class, normal system.
>
> Quite honestly, these statements and the attached patch (which doesn't
> even begin to touch the central issue, but does indeed break a lot of
> things) demonstrate that *you* are not the guy to implement what was
> being discussed. It needs to be done by someone who understands the
> core caching code, which apparently you haven't studied in any detail.

I didn't claim the attached patch began to touch the issues. I was very
clear that it covered only the very simplest use case, that was the
point. You may not wish to acknowledge it, but I *have* studied the core
caching code in detail for many months and that is the basis for my
comments. The way I have written the exclusions in vacuum.c shows that I
have identified each of the sub-cases we are required to handle.

There is nothing wrong with your idea of using a mapping file. That is
relatively easy part of the problem.

> I have a feeling that I should go do this...

If you wish, but I still think it is an unnecessary change for this
release, whoever does it. We both know that once you start you won't
stop, but that doesn't make it worthwhile or less risky.

--
Simon Riggs www.2ndQuadrant.com


--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Tom Lane on
Simon Riggs <simon(a)2ndQuadrant.com> writes:
> On Sun, 2010-01-31 at 14:35 -0500, Tom Lane wrote:
>> Anyway, it's still not apparent to me exactly what the connection is
>> between VACUUM FULL and Hot Standby. I remember that we said HS didn't
>> work with VACUUM FULL (INPLACE) but I don't recall why that is, and the

[ sorry, I meant not-INPLACE ]

>> links on the open-items pages are not leading me to any useful
>> discussion.

> Very little really; not enough to force the sort of changes that I am
> now seeing will be required in the way catalogs and caches operate.
> There was some difficulty around the fact that VFI issues two commits
> for the same transaction, but that is now correctly handled in the code
> after discussion.

If the only benefit of getting rid of VACUUM FULL were simplifying
Hot Standby, I'd agree with you. But there are numerous other benefits.
The double-commit hack you mention is something we need to get rid of
for general system stability (because of the risk of PANIC if the vacuum
fails after the first commit). Getting rid of REINDEX-in-place on
shared catalog indexes is another thing that's really safety critical.
Removing V-F related hacks in other places would just be a bonus.

It's something we need to do, so if Hot Standby is forcing our hands,
then let's just do it.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Simon Riggs on
On Sun, 2010-01-31 at 15:14 -0500, Tom Lane wrote:
> Simon Riggs <simon(a)2ndQuadrant.com> writes:
> > On Sun, 2010-01-31 at 14:35 -0500, Tom Lane wrote:
> >> Anyway, it's still not apparent to me exactly what the connection is
> >> between VACUUM FULL and Hot Standby. I remember that we said HS didn't
> >> work with VACUUM FULL (INPLACE) but I don't recall why that is, and the
>
> [ sorry, I meant not-INPLACE ]
>
> >> links on the open-items pages are not leading me to any useful
> >> discussion.
>
> > Very little really; not enough to force the sort of changes that I am
> > now seeing will be required in the way catalogs and caches operate.
> > There was some difficulty around the fact that VFI issues two commits
> > for the same transaction, but that is now correctly handled in the code
> > after discussion.
>
> If the only benefit of getting rid of VACUUM FULL were simplifying
> Hot Standby, I'd agree with you. But there are numerous other benefits.
> The double-commit hack you mention is something we need to get rid of
> for general system stability (because of the risk of PANIC if the vacuum
> fails after the first commit). Getting rid of REINDEX-in-place on
> shared catalog indexes is another thing that's really safety critical.
> Removing V-F related hacks in other places would just be a bonus.
>
> It's something we need to do, so if Hot Standby is forcing our hands,
> then let's just do it.

That's the point: Hot Standby is *not* forcing our hand to do this.

Doing this will not simplify Hot Standby in any significant way. The
code to support VFI with Hot Standby is, after technical review, much,
much simpler than the code to remove VFI.

I'll do a little work towards step (1) just so we can take a more
informed view once you've had a better look at just what (2) involves. I
had already written the code for the Sept release of HS.

--
Simon Riggs www.2ndQuadrant.com


--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers