On Tue, Aug 3, 2021 at 9:59 AM Pavel Borisov <pashkin.elfe@gmail.com> wrote:
> I've looked at v5 patch. It is completely similar to v2 patch, which I've already tested using the workflow, I've
describedin the comments above. Before the patch I get the errors quite soon, the patch corrects them. Furthermore
I'vetested the same patch under REPEATABLE READ and SERIALIZABLE and detected no flaws. So, now, when we've got
Robert'sexplanation, it seems to me that v2 (aka v5) patch can be committed (leaving possible REPEATABLE READ and
SERIALIZABLEimprovements for the future). I don't really sure it is still possible on 07/21 СF, but I'd change status
ofthe patch to the ready-for-committer. Also I'd like the bugfix to be backported to the previous PG versions.
I agree that the fix should be back-ported, but I'm not keen to commit
anything unless it works for all isolation levels.
The idea I sort of had floating around in my mind is a little
different than what Greg has implemented. I was thinking that we could
just skip SerializeSnapshot and the corresponding shm_toc_allocate()
if !IsolationUsesXactSnapshot(). Then on the restore side we could
just call shm_toc_lookup() with noError = true and skip
RestoreTransactionSnapshot/RestoreSnapshot if it returns NULL.
I don't know why Greg's patch is changing anything related to the
active snapshot (as opposed to the transaction snapshot). Maybe
there's a reason why we need that change, but I don't know what it is.
--
Robert Haas
EDB: http://www.enterprisedb.com