Re: New vacuum option to do only freezing

On Thu, Jan 17, 2019 at 1:33 AM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
> On Wed, Jan 16, 2019 at 3:30 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> > As the above comment says, it's possible that the state of an
> > INSERT_IN_PROGRESS tuple could be changed to 'dead' after
> > heap_page_prune(). Since such tuple is not truncated at this point we
> > record it and set it as UNUSED in lazy_vacuum_page(). I think that the
> > DISABLE_INDEX_CLEANUP case is the same; we need to process them after
> > recorded. Am I missing something?
>
> I believe you are. Think about it this way. After the first pass
> over the heap has been completed but before we've done anything to the
> indexes, let alone started the second pass over the heap, somebody
> could kill the vacuum process. Somebody could in fact yank the plug
> out of the wall, stopping the entire server in its tracks. If they do
> that, then lazy_vacuum_page() will never get executed. Yet, the heap
> can't be in any kind of corrupted state at this point, right? We know
> that the system is resilient against crashes, and killing a vacuum or
> even the whole server midway through does not leave the system in any
> kind of bad state. If it's fine for lazy_vacuum_page() to never be
> reached in that case, it must also be fine for it never to be reached
> if we ask for vacuum to stop cleanly before lazy_vacuum_page().
>

Yes, that's right. It doesn't necessarily need to be reached lazy_vacuum_page().

> In the case of the particular comment to which you are referring, that
> comment is part of lazy_scan_heap(), not lazy_vacuum_page(), so I
> don't see how it bears on the question of whether we need to call
> lazy_vacuum_page(). It's true that, at any point in time, an
> in-progress transaction could abort. And if it does then some
> insert-in-progress tuples could become dead. But if that happens,
> then the next vacuum will remove them, just as it will remove any
> tuples that become dead for that reason when vacuum isn't running in
> the first place. You can't use that as a justification for needing a
> second heap pass, because if it were, then you'd also need a THIRD
> heap pass in case a transaction aborts after the second heap pass has
> visited the pages, and a fourth heap pass in case a transaction aborts
> after the third heap pass has visited the pages, etc. etc. forever.
>

The reason why I processed the tuples that became dead after the first
heap pass is that I was not sure the reason why we ignore such tuples
in the second heap pass despite of there already have been the code
doing so which has been used for a long time. I thought we can do that
in the same manner even in DISABLE_INDEX_CLEANUP case. Also, since I
thought that lazy_vacuum_page() is the best place to set them as DEAD
I modified it (In the previous patch I introduced another function
setting them as DEAD aside from lazy_vacuum_page(). But since these
were almost same I merged them).

However, as you mentioned it's true that these tuples will be removed
by the next vacuum even if we ignore. Also these tuples would not be
many and without this change the patch would get more simple. So if
the risk of this change is greater than the benefit, we should not do
that.

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center