On Tue, Nov 15, 2022 at 6:55 AM Andres Freund <and...@anarazel.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.
improve_checks_snapbuild_1.patch
Description: Binary data