pgsql: Avoid marking buffer dirty when VACUUM has no work to do.

Simon Riggs <simon@2ndQuadrant.com> writes:

Avoid marking buffer dirty when VACUUM has no work to do.
When wal_level = 'hot_standby' we touched the last page of the
relation during a VACUUM, even if nothing else had happened.
That would alter the LSN of the last block and set the mtime
of the relation file unnecessarily. Noted by Thom Brown.

This doesn't look right to me --- you have not accounted for the
possibility that btpo_cycleid or BTP_HAS_GARBAGE is changed.

Also, I'm confused about the business of not setting the LSN. Thom
claimed that he was seeing the page not change at all (or at least
md5sum of the file didn't change) despite mtime changing. If we'd
been plastering a new LSN on the page each time, then that should
certainly not have been possible. So I now think maybe we've
mis-analyzed what was happening in his example.

I think this requires more careful analysis.

regards, tom lane

I wrote:

Simon Riggs <simon@2ndQuadrant.com> writes:

Avoid marking buffer dirty when VACUUM has no work to do.
When wal_level = 'hot_standby' we touched the last page of the
relation during a VACUUM, even if nothing else had happened.
That would alter the LSN of the last block and set the mtime
of the relation file unnecessarily. Noted by Thom Brown.

This doesn't look right to me --- you have not accounted for the
possibility that btpo_cycleid or BTP_HAS_GARBAGE is changed.

Also, I'm confused about the business of not setting the LSN. Thom
claimed that he was seeing the page not change at all (or at least
md5sum of the file didn't change) despite mtime changing. If we'd
been plastering a new LSN on the page each time, then that should
certainly not have been possible. So I now think maybe we've
mis-analyzed what was happening in his example.

I think this requires more careful analysis.

Ping? If you don't respond, I'm going to take it on my own authority to
revert this patch, because it's definitely broken as-is, and I don't
think the consequences of not updating the page LSN have been thought
through either.

regards, tom lane

On Tue, Nov 22, 2011 at 2:32 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I wrote:

Simon Riggs <simon@2ndQuadrant.com> writes:

Avoid marking buffer dirty when VACUUM has no work to do.
When wal_level = 'hot_standby' we touched the last page of the
relation during a VACUUM, even if nothing else had happened.
That would alter the LSN of the last block and set the mtime
of the relation file unnecessarily. Noted by Thom Brown.

This doesn't look right to me --- you have not accounted for the
possibility that btpo_cycleid or BTP_HAS_GARBAGE is changed.

Also, I'm confused about the business of not setting the LSN.  Thom
claimed that he was seeing the page not change at all (or at least
md5sum of the file didn't change) despite mtime changing.  If we'd
been plastering a new LSN on the page each time, then that should
certainly not have been possible.  So I now think maybe we've
mis-analyzed what was happening in his example.

I think this requires more careful analysis.

Ping?  If you don't respond, I'm going to take it on my own authority to
revert this patch, because it's definitely broken as-is, and I don't
think the consequences of not updating the page LSN have been thought
through either.

Tom, waiting across a weekend isn't a cause for concern.

I made that change for you, so am happy to revoke for you also.

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