From: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
---|---|
To: | Andres Freund <andres(at)anarazel(dot)de> |
Cc: | Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>, Stephen Frost <sfrost(at)snowman(dot)net>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Reviewing freeze map code |
Date: | 2016-07-16 11:10:52 |
Message-ID: | CAA4eK1K_-Q8KkFykNn0dAhs2=MM2=Sj6n7oz+HtTsdXgjZKGpg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Sat, Jul 16, 2016 at 7:02 AM, Andres Freund <andres(at)anarazel(dot)de> wrote:
> On 2016-07-13 23:06:07 -0700, Andres Freund wrote:
>> > + /* Clear only the all-frozen bit on visibility map if needed */
>> > + if (PageIsAllVisible(BufferGetPage(buffer)) &&
>> > + VM_ALL_FROZEN(relation, block, &vmbuffer))
>> > + {
>> > + visibilitymap_clear_extended(relation, block, vmbuffer,
>> > + VISIBILITYMAP_ALL_FROZEN);
>> > + }
>> > +
>>
>> FWIW, I don't think it's worth introducing visibilitymap_clear_extended.
>> As this is a 9.6 only patch, i think it's better to change
>> visibilitymap_clear's API.
>
> Besides that easily fixed issue, the code also has the significant issue
> that it's only performing the the visibilitymap processing in the
> BLK_NEEDS_REDO case. But that's not ok, because both in the BLK_RESTORED
> and the BLK_DONE cases the visibilitymap isn't guaranteed (or even
> likely in the former case) to have been updated.
>
> I think we have two choices how to deal with that: First, we can add a
> new flags variable to xl_heap_lock similar to
> xl_heap_insert/update/... and bump page magic,
>
+1 for going in this way. This will keep us consistent with how clear
the visibility info in other places like heap_xlog_update().
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Paquier | 2016-07-16 11:41:34 | Re: visibilitymap_clear()s in vacuumlazy.c aren't WAL logged |
Previous Message | Pavel Stehule | 2016-07-16 08:55:01 | Re: raw output from copy |