On Mon, Nov 27, 2023 at 11:34 AM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Mon, Nov 27, 2023 at 6:41 AM Tomas Vondra > <tomas.von...@enterprisedb.com> wrote: > > > > While going over 0001, I realized there might be an optimization for > > ReorderBufferSequenceIsTransactional. As coded in 0001, it always > > searches through all top-level transactions, and if there's many of them > > that might be expensive, even if very few of them have any relfilenodes > > in the hash table. It's still linear search, and it needs to happen for > > each sequence change. > > > > But can the relfilenode even be in some other top-level transaction? How > > could it be - our transaction would not see it, and wouldn't be able to > > generate the sequence change. So we should be able to simply check *our* > > transaction (or if it's a subxact, the top-level transaction). Either > > it's there (and it's transactional change), or not (and then it's > > non-transactional change). > > > > I also think the relfilenode should be part of either the current > top-level xact or one of its subxact, so looking at all the top-level > transactions for each change doesn't seem advisable. > > > The 0004 does this. > > > > This of course hinges on when exactly the transactions get created, and > > assignments processed. For example if this would fire before the txn > > gets assigned to the top-level one, this would break. I don't think this > > can happen thanks to the immediate logging of assignments, but I'm too > > tired to think about it now. > > > > This needs some thought because I think we can't guarantee the > association till we reach the point where we can actually decode the > xact. See comments in AssertTXNLsnOrder() [1]. >
I am wondering that instead of building the infrastructure to know whether a particular change is transactional on the decoding side, can't we have some flag in the WAL record to note whether the change is transactional or not? I have discussed this point with my colleague Kuroda-San and we thought that it may be worth exploring whether we can use rd_createSubid/rd_newRelfilelocatorSubid in RelationData to determine if the sequence is created/changed in the current subtransaction and then record that in WAL record. By this, we need to have additional information in the WAL record like XLOG_SEQ_LOG but we can probably do it only with wal_level as logical. One minor point: It'd also + * trigger assert in DecodeSequence. I don't see DecodeSequence() in the patch. Which exact assert/function are you referring to here? -- With Regards, Amit Kapila.
