On 2016-04-04 08:08:32 +0100, Simon Riggs wrote: > For that matter, it's also important for hot standby; to deal with > > FATALed transactions which didn't write an abort record. It's kinda > > important to call StandbyReleaseOldLocks for those. And if a standby is > > in STANDBY_SNAPSHOT_READY it's also important to see this kind of > > record. > > > > Those objections apply to all solutions to the problem so far > proposed,
No, because in the alternative proposal we determine that the system indeed has been idle since the last time a WAL record was logged.
Why would that change anything? If something aborts without writing an abort record, the system can easily be idle afterwards. So an idle system means nothing. Your objection applies to *all* cases, not just for this patch and if we revert, we block all cases.
And only a few select records are exempt from being considered idle; every other type of record causes the system to be considered non-idle.
> likely any solution. My understanding was that those issues are considered > the lesser of the two evils. I'm happy to revert this patch, as long as we > agree that it also blocks all further "bug fixes" so far proposed based > upon those arguments.
Yes, please do. > > Additionally, we're now logging more WAL if archive timeout isn't > > configured, which doesn't strike me as a good idea.
(to clarify, I'm comparing a configured XLogArchiveTimeout to none, not the before/after patch state) > That's not true either...
How can XLogArchiveTimeout not imply a behavioural difference here?
The bug appears when we have archive_timeout set; the bug fix applies in that case also.
> Thanks for your comments, but you seem to be misreading the patch with > respect to the if clause and new brackets.
I indeed missed the outer if; which addresses the logical issue. So the problem there isn't fixed at all.
You ignored a good number of messages and just committed a patch with a different approach without any further discussion. So it shouldn't be particularly surprising that people aren't willing to look super careful at what you did.
I haven't ignored any messages, regrettably I read them all. I've committed a much simpler patch, which is what committers do. That patch does exactly the same thing as the patch you prefer, just does it differently; I'm sorry if that annoys you. If you have time for debate, please read the patch and lets have an even-handed and rational debate. If you don't have time to review the patch, lets skip the debate.
--
Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services