Re: Assertion failure in SnapBuildInitialSnapshot()

On Tue, Nov 15, 2022 at 6:55 AM Andres Freund <andres(at)anarazel(dot)de> wrote:
>
> On 2022-11-10 16:04:40 +0530, Amit Kapila wrote:
> > I don't have any good ideas on how to proceed with this. Any thoughts
> > on this would be helpful?
>
> One thing worth doing might be to convert the assertion path into an elog(),
> mentioning both xids (or add a framework for things like AssertLT(), but that
> seems hard). With the concrete values we could make a better guess at what's
> going wrong.
>
> It'd probably not hurt to just perform this check independent of
> USE_ASSERT_CHECKING - compared to the cost of creating a slot it's neglegible.
>
> That'll obviously only help us whenever we re-encounter the issue, which will
> likely be a while...
>

Agreed.

>
>
> One thing I noticed just now is that we don't assert
> builder->building_full_snapshot==true. I think we should? That didn't use to
> be an option, but now it is... It doesn't look to me like that's the issue,
> but ...
>

Agreed.

The attached patch contains both changes. It seems to me this issue
can happen, if somehow, either slot's effective_xmin increased after
we assign its initial value in CreateInitDecodingContext() or somehow
its value is InvalidTransactionId when we have invoked
SnapBuildInitialSnapshot(). The other possibility is that the
initial_xmin_horizon check in SnapBuildFindSnapshot() doesn't insulate
us from assigning builder->xmin value older than initial_xmin_horizon.
I am not able to see if any of this can be true.

--
With Regards,
Amit Kapila.