Re: TRAP: FailedAssertion("HaveRegisteredOrActiveSnapshot()", File: "toast_internals.c", Line: 670, PID: 19403)

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> I agree that it's a little unclear. In general, I think if we're going
> to blow up and die, doing it closer to the place where the problem is
> happening is for the best. On the other hand, if in most practical
> cases we're going to stumble through and get the right answer anyway,
> then it's maybe not great to break a bunch of accidentally-working
> cases. However, it does strikes me that this principal could easily be
> overdone. init_toast_snapshot() could pick a random snapshot (or take
> a new one) in order to call InitToastSnapshot() and that would often
> work fine. Yet, upon realizing that things are busted, it chooses to
> error out instead. I approve of that choice, and don't think we should
> rule out the idea of making that check more robust.

I'm all for improving robustness, but failing in cases that would have
worked before (even if only accidentally) is not going to be seen by
users as more robust. I think that this late stage of the development
cycle is not the time to be putting in changes that are not actually
going to fix bugs but only call greater attention to the possibility
that a bug exists.

TBH, given where we are in the dev cycle, I thought there was a lot of
sense behind your earlier thought that HaveRegisteredOrActiveSnapshot
should be reverted entirely. I'm okay with keeping it as an assertion-
only check, so that it won't bother end users. I'm not okay with
adding end-user-visible failures, at least not till early in v16.

regards, tom lane