On Sun, Aug 30, 2020 at 2:43 PM Dilip Kumar <dilipbal...@gmail.com> wrote: > > On Sat, Aug 29, 2020 at 5:18 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > > One more comment for which I haven't done anything yet. > > +static void > > +set_schema_sent_in_streamed_txn(RelationSyncEntry *entry, TransactionId > > xid) > > +{ > > + MemoryContext oldctx; > > + > > + oldctx = MemoryContextSwitchTo(CacheMemoryContext); > > + > > + entry->streamed_txns = lappend_int(entry->streamed_txns, xid); > > > Is it a good idea to append xid with lappend_int? Won't we need > > something equivalent for uint32? If so, I think we have a couple of > > options (a) use lcons method and accordingly append the pointer to > > xid, I think we need to allocate memory for xid if we want to use this > > idea or (b) use an array instead. What do you think? > > BTW, OID is internally mapped to uint32, but using lappend_oid might > not look good. So maybe we can provide an option for lappend_uint32? > Using an array is also not a bad idea. Providing lappend_uint32 > option looks more appealing to me. >
I thought about this again and I feel it might be okay to use it for our case as after storing it in T_IntList, we primarily fetch it for comparison with TrasnactionId (uint32), so this shouldn't create any problem. I feel we can just discuss this in a separate thread and check the opinion of others, what do you think? Another comment: +cleanup_rel_sync_cache(TransactionId xid, bool is_commit) +{ + HASH_SEQ_STATUS hash_seq; + RelationSyncEntry *entry; + + Assert(RelationSyncCache != NULL); + + hash_seq_init(&hash_seq, RelationSyncCache); + while ((entry = hash_seq_search(&hash_seq)) != NULL) + { + if (is_commit) + entry->schema_sent = true; How is it correct to set 'entry->schema_sent' for all the entries in RelationSyncCache? Consider a case where due to invalidation in an unrelated transaction we have set the flag schema_sent for a particular relation 'r1' as 'false' and that transaction is executed before the current streamed transaction for which we are performing commit and called this function. It will set the flag for unrelated entry in this case 'r1' which doesn't seem correct to me. Or, if this is correct, it would be a good idea to write some comments about it. -- With Regards, Amit Kapila.