On Tue, Aug 3, 2021 at 9:59 AM Pavel Borisov <pashkin.e...@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 described in the comments above. > Before the patch I get the errors quite soon, the patch corrects them. > Furthermore I've tested the same patch under REPEATABLE READ and SERIALIZABLE > and detected no flaws. So, now, when we've got Robert's explanation, it seems > to me that v2 (aka v5) patch can be committed (leaving possible REPEATABLE > READ and SERIALIZABLE improvements for the future). I don't really sure it is > still possible on 07/21 СF, but I'd change status of the 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
