On 2018-Jun-11, Antonin Houska wrote: > Arseny Sher <a.s...@postgrespro.ru> wrote: > > Please see detailed description of the issues, tests which reproduce > > them and fixes in the attached patch. > > I've played with your tests and gdb for a while, both w/o and with your > patch. I think I can understand both problems. I could not invent simpler way > to fix them. > > One comment about the coding: since you introduce a new transaction list and > it's sorted by LSN, I think you should make the function AssertTXNLsnOrder() > more generic and use it to check the by_base_snapshot_lsn list too. For > example call it after the list insertion and deletion in > ReorderBufferPassBaseSnapshot().
I've been going over this patch also, and I've made a few minor edits of the patch and the existing code as I come to understand what it's all about. > Also I think it makes no harm if you split the diff into two, although both > fixes use the by_base_snapshot_lsn field. Please don't. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
