From: | 高增琦 <pgf00a(at)gmail(dot)com> |
---|---|
To: | Robert Haas <robertmhaas(at)gmail(dot)com> |
Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: buffer locking fix for lazy_scan_heap |
Date: | 2015-08-03 04:03:55 |
Message-ID: | CAFmBtr1aOjcbw2rAnbB0quHPjd6rpzhfMWEPYBBLBHRWA8Lj9w@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi,
sorry for asking this really old commit.
I could not understand why "releases the lock on the buffer" is
a problem since vacuum will lock and check the page bit again before
set the vm. Did I missed something?
Thanks for your help.
On Thu, Apr 19, 2012 at 4:09 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> I discovered when researching the issue of index-only scans and Hot
> Standby that there's a bug (for which I'm responsible) in
> lazy_scan_heap[1]. Actually, the code has been like this for a long
> time, but I needed to change it when I did the crash-safe visibility
> map work, and I didn't. The problem is that lazy_scan_heap() releases
> the lock on the buffer it's whacking around before it sets the
> visibility map bit. Thus, it's possible for the page-level bit to be
> cleared by some other backend before the visibility map bit gets set.
> In previous releases that was arguably OK, since the worst thing that
> could happen is a postponement of vacuuming on that page until the
> next anti-wraparound cycle; but now that we have index-only scans, it
> can cause queries to return wrong answers.
>
> Attached is a patch to fix the problem, which rearranges things so
> that we set the bit in the visibility map before releasing the buffer
> lock. Similar work has already been done for inserts, updates, and
> deletes, which in previous releases would *clear* the visibility map
> bit after releasing the page lock, and no longer done. But the vacuum
> case, which *sets* the bit, was overlooked. Our convention in those
> related cases is that it's acceptable to hold the buffer lock while
> setting the visibility map bit and issuing the WAL log record, but
> there must be no possibility of an I/O to read in the visibility map
> page while the buffer lock is held. This turned out to be pretty
> simple because, as it turns out, lazy_scan_heap() is almost always
> holding a pin on the correct page anyway, so I only had to tweak
> things slightly to guarantee it. As a pleasant side effect, the new
> code is actually quite a bit simpler and more readable than the old
> code, at least IMHO.
>
> While I was at it, I made a couple of minor, related changes which I
> believe to be improvements. First, I arranged things so that, when we
> pause the first vacuum pass to do an index vac cycle, we release any
> buffer pin we're holding on the visibility map page. The fact that we
> haven't done that in the past is probably harmless, but I think
> there's something to be said for not holding onto buffer pins for long
> periods of time when it isn't really necessary. Second, I adjusted
> things so that we print a warning if the visibility map bit is set and
> the page-level bit is clear, since that should never happen in 9.2+.
> It is similar to the existing warning which catches the case where a
> page is marked all-visible despite containing dead tuples.
>
> Comments?
>
> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
> http://archives.postgresql.org/pgsql-hackers/2012-04/msg00682.php
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
>
--
GaoZengqi
pgf00a(at)gmail(dot)com
zengqigao(at)gmail(dot)com
From | Date | Subject | |
---|---|---|---|
Next Message | Amit Kapila | 2015-08-03 04:05:27 | Re: Transactions involving multiple postgres foreign servers |
Previous Message | Michael Paquier | 2015-08-03 04:01:13 | Re: pg_rewind failure by file deletion in source server |