Re: -DCLOBBER_CACHE_ALWAYS shows COPY FREEZE regression problem

From: Simon Riggs <simon(at)2ndquadrant(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: -DCLOBBER_CACHE_ALWAYS shows COPY FREEZE regression problem
Date: 2012-12-07 12:38:37
Message-ID: CA+U5nM+gn7p5191Ye9nxBFWf=3zNEmxPHb_SWXkkQ40vO6+a=Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 7 December 2012 00:13, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>> On a new buildfarm member friarbird
>> <http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=friarbird&dt=2012-12-06%2020%3A55%3A31>,
>> configured with _DCLOBBER_CACHE_ALWAYS:
>
>> BEGIN;
>> TRUNCATE vistest;
>> COPY vistest FROM stdin CSV FREEZE;
>> + NOTICE: FREEZE option specified but pre-conditions not met
>
> The notice is coming out because the relcache is dropping the value of
> rd_newRelfilenodeSubid as a result of cache flushes. The COPY FREEZE
> code is aware of this risk, commenting
>
> * As mentioned in comments in utils/rel.h, the in-same-transaction test
> * is not completely reliable, since in rare cases rd_createSubid or
> * rd_newRelfilenodeSubid can be cleared before the end of the transaction.
> * However this is OK since at worst we will fail to make the optimization.
>
> but I'd say this seriously throws into question whether it should be
> emitting a notice. That seems to tie into the discussion a little bit
> ago about whether the FREEZE option is a command or a hint. Throwing a
> notice seems like a pretty schizophrenic choice: it's not an error but
> you're in the user's face about it anyway. In any case, if the option
> is unreliable (and with this implementation it certainly is), we can
> *not* treat its failure as an error, so it's not a command.

Hmm, yes, its pretty schizophrenic, but that comes from my attempt to
offer something useful to the user in line with Robert's wish for the
hint to not be silently ignored. If I had overridden that and made it
truly silent as I had proposed, the clobber error would not have
happened.

I guess my mind must have been aware of the above, but it regrettably
wasn't consciously there. The above text was there from before, not
from the recent patch.

I'll remove the message now so tests pass, since that was my original
intention, but others may wish to suggest other ways forward.

> TBH I think that COPY FREEZE should not have been committed yet;
> it doesn't seem to be fully baked.

I think so too, in hindsight, but at the time it seemed a simple patch
that followed review and recent on-list discussion. My mistake was
tinkering with the patch before commit, which I then revoked.

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Pavan Deolasee 2012-12-07 12:47:47 Re: Setting visibility map in VACUUM's second phase
Previous Message Michael Paquier 2012-12-07 12:37:06 Re: Support for REINDEX CONCURRENTLY