Hi Heikki, On Tue, Apr 10, 2018 at 7:07 PM, Heikki Linnakangas <hlinn...@iki.fi> wrote:
> >> > It would seem more straightforward to add a snapshot parameter to > GetNewOidWithIndex(), so that the this one caller could pass SnapshotToast, > while others pass SnapshotDirty. With your patch, you check the index > twice: first with SnapshotDirty, in GetNewOidWithIndex(), and then with > SnapshotToast, in the caller. > Hmm. I actually wrote my first patch exactly like that. I am trying to remember why I discarded that approach. Was that to do with the fact that SnapshotToast can't see all toast tuples either? Yeah, I think so. For example, it won't see tuples with uncommitted-xmin, leading to different issues. Now it's unlikely that we will have a OID conflict where the old tuple has uncommitted-xmin, but not sure if we can completely rule that out. For example, if we did not uncover the redo recovery bug, we could have had a prepared transaction having inserted the old tuple, which now conflicts with new tuple and not detected by SnapshotToast. > > If I'm reading the rewrite case correctly, it's a bit different and quite > special. In the loop with GetNewOidWithIndex(), it needs to check that the > OID is unused in two tables, the old TOAST table, and the new one. You can > only pass one index to GetNewOidWithIndex(), so it needs to check the > second index manually. It's not because of the snapshot issue. Although I > wonder if we should be using SnapshotToast in that GetNewOidWithIndex() > call, too. I.e. if we should be checking both the old and the new toast > table with SnapshotToast. As I said, I am not sure if checking with just SnapshotToast is enough because it can't see "dirty" tuples. > > Agreed. With nextXid, we advance ShmemVariableCache->nextXid if the value > in the online checkpoint record is greater than > ShmemVariableCache->nextXid. But we don't have such a wraparound-aware > concept of "greater than" for OIDs. Ignoring the online checkpoint record's > nextOid value seem fine to me. > Ok. Thanks for checking. Thanks, Pavan -- Pavan Deolasee http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
