Re: BUG #17485: Records missing from Primary Key index when doing REINDEX INDEX CONCURRENTLY

Hi,

On 2022-05-25 09:00:47 +0500, Andrey Borodin wrote:
> > On 25 May 2022, at 00:01, Andres Freund <andres(at)anarazel(dot)de> wrote:
> >
> >>> On 24 May 2022, at 23:15, Andres Freund <andres(at)anarazel(dot)de> wrote:
> >>>
> >>> I suspect the problem might be related to pruning done during the validation
> >>> scan. Once PROC_IN_SAFE_IC is set, the backend itself will not preserve tids
> >>> its own snapshot might need. Which will wreak havoc during the validation
> >>> scan.
> >>
> >> I observe that removing PROC_IN_SAFE_IC for index_validate() fixes tests.
> >> But why it's not a problem for index_build() scan?
> >
> > I now suspect it's a problem for both, just more visible for index_validate().

> No, I understood now. index_build() in CIC can be safely replaced by
> index_build_empty().

I'm not convinced that that is true. I think at least for stuff like indexes
used in constraints (unique or exclusion) we need to do something like the
current approach.

But more importantly, there's a difference between not inserting any index
entries, and inserting entries based on a essentially corrupted
snapshot. Using a too old snapshot (which is what we have here) could lead to
all sorts of oddities when following HOT chains etc. Which might end up
inserting *too much* into the index, I think.

> So, can we leave the feature enabled for all the process besides index
> validation? We don't seem to need a valid snapshot in CIC anywhere else. Or
> it's safer to revert whole commit?

I doubt it's safe in the other places either. We can't just do a table scan,
which expects to be passed a valid snapshot, with a broken snapshot, and
expect no problems. At the very least I expect there to be a potential of
spurious warnings / errors.

Greetings,

Andres Freund