Thread: -DCLOBBER_CACHE_ALWAYS shows COPY FREEZE regression problem
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
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
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
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
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
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