Re: [HACKERS] ginInsertCleanup called from vacuum could still miss tuples to be deleted

On Thu, Nov 16, 2017 at 11:24 AM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Mon, Nov 13, 2017 at 3:25 AM, Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>> In ginInsertCleanup(), we lock the GIN meta page by LockPage and could
>> wait for the concurrent cleaning up process if stats == NULL. And the
>> source code comment says that this happen is when ginINsertCleanup is
>> called by [auto]vacuum/analyze or gin_clean_pending_list(). I agree
>> with this behavior. However, looking at the callers the stats is NULL
>> only either if pending list exceeds to threshold during insertions or
>> if only analyzing is performed by an autovacum worker or ANALYZE
>> command. So I think we should inVacuum = (stats != NULL) instead.
>
> Argh.  Yeah, that looks wrong.
>
> Instead of relying on this indirect method, how about passing an
> explicit inVacuum argument to that function?  And while we're at it,
> how about renaming inVacuum to forceCleanup?
>

Agreed, that's better. Attached updated patch.
Also I've added this to the next CF so as not to forget.

Regards,

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