Thread: -DCLOBBER_CACHE_ALWAYS shows COPY FREEZE regression problem

-DCLOBBER_CACHE_ALWAYS shows COPY FREEZE regression problem

From
Andrew Dunstan
Date:
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-conditionsnot met
 


and similar.

cheers

andrew




Re: -DCLOBBER_CACHE_ALWAYS shows COPY FREEZE regression problem

From
Simon Riggs
Date:
On 6 December 2012 23:28, Andrew Dunstan <andrew@dunslane.net> wrote:
> 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

I don't understand why that build option would produce different
output for simple logic.

I'll look again in the morning.

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



Re: -DCLOBBER_CACHE_ALWAYS shows COPY FREEZE regression problem

From
Andres Freund
Date:
On 2012-12-06 23:59:06 +0000, Simon Riggs wrote:
> On 6 December 2012 23:28, Andrew Dunstan <andrew@dunslane.net> wrote:
> > 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
>
> I don't understand why that build option would produce different
> output for simple logic.
>
> I'll look again in the morning.

It clears the relcache entry as soon as AcceptInvalidationMessages() is
done seems to loose rd_createSubid and rd_newRelfilenodeSubid.

Apparently the magic to preserve those values across cache resets isn't
strong enough for this. Seems bad, because that seems to mean a sinval
overflow leads to this and related optimizations being lost?

Greetings,

Andres Freund

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



Re: -DCLOBBER_CACHE_ALWAYS shows COPY FREEZE regression problem

From
Tom Lane
Date:
Andrew Dunstan <andrew@dunslane.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
rarecases rd_createSubid or    * rd_newRelfilenodeSubid can be cleared before the end of the transaction.    * However
thisis 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.

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



Re: -DCLOBBER_CACHE_ALWAYS shows COPY FREEZE regression problem

From
Simon Riggs
Date:
On 7 December 2012 00:06, Andres Freund <andres@2ndquadrant.com> wrote:

> Apparently the magic to preserve those values across cache resets isn't
> strong enough for this. Seems bad, because that seems to mean a sinval
> overflow leads to this and related optimizations being lost?

Which seems to back up the case for it being a hint, that the system
might not be able to honor.

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



Re: -DCLOBBER_CACHE_ALWAYS shows COPY FREEZE regression problem

From
Simon Riggs
Date:
On 7 December 2012 00:13, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Andrew Dunstan <andrew@dunslane.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