Re: Reviewing freeze map code
| От | Amit Kapila |
|---|---|
| Тема | Re: Reviewing freeze map code |
| Дата | |
| Msg-id | CAA4eK1+hzefucHUgTKVGvRUFmZDutZf=dzKTaRsDeCbdtQ810g@mail.gmail.com обсуждение исходный текст |
| Ответ на | Re: Reviewing freeze map code (Masahiko Sawada <sawada.mshk@gmail.com>) |
| Ответы |
Re: Reviewing freeze map code
|
| Список | pgsql-hackers |
On Tue, Jun 7, 2016 at 10:10 PM, Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> Thank you for implementing the patch.
>
> I've not test it deeply but here are some comments.
> This check tool only checks if the frozen page has live-unfrozen tuple.
> That is, it doesn't care in case where the all-frozen page mistakenly
> has dead-frozen tuple.
Do you mean to say that we should have a check for ItemIdIsDead() and then if item is found to be dead, then add it to array of non_frozen items? If so, earlier I thought we might not need this check as we are already using heap_tuple_needs_eventual_freeze(), but now again looking at it, it seems wise to check for dead items separately as those won't be covered by other check.
>
> + /* Clean up. */
> + if (vmbuffer != InvalidBuffer)
> + ReleaseBuffer(vmbuffer);
>
> I think that we should use BufferIsValid() here.
>
>
> Thank you for implementing the patch.
>
> I've not test it deeply but here are some comments.
> This check tool only checks if the frozen page has live-unfrozen tuple.
> That is, it doesn't care in case where the all-frozen page mistakenly
> has dead-frozen tuple.
>
Do you mean to say that we should have a check for ItemIdIsDead() and then if item is found to be dead, then add it to array of non_frozen items? If so, earlier I thought we might not need this check as we are already using heap_tuple_needs_eventual_freeze(), but now again looking at it, it seems wise to check for dead items separately as those won't be covered by other check.
>
> + /* Clean up. */
> + if (vmbuffer != InvalidBuffer)
> + ReleaseBuffer(vmbuffer);
>
> I think that we should use BufferIsValid() here.
>
We can use BufferIsValid() as well, but I am trying to be consistent with nearby code, refer collect_visibility_data(). We can change at all places together if people prefer that way.
В списке pgsql-hackers по дате отправления: