On Wed, Dec 11, 2019 at 11:46 PM Robert Haas <robertmh...@gmail.com> wrote: > > On Mon, Dec 2, 2019 at 3:32 AM Dilip Kumar <dilipbal...@gmail.com> wrote: > > I have rebased the patch set on the latest head. > > 0001 looks like a clever approach, but are you sure it doesn't hurt > performance when many small XLOG records are being inserted? I think > XLogRecordAssemble() can get pretty hot in some workloads. > > With regard to 0002, logging a separate WAL record for each > invalidation seems painful; I think most operations that generate > invalidations generate a bunch of them all at once. Perhaps you could > just queue up invalidations as they happen, and then force anything > that's been queued up to be emitted into WAL just before you emit any > WAL record that might need to be decoded. >
I feel we can log the invalidations of the entire command at one go if we log at CommandEndInvalidationMessages. We already have all the invalidations of current command in transInvalInfo->CurrentCmdInvalidMsgs. This can save us the effort of maintaining a new separate list/queue for invalidations and to a good extent, it will ameliorate your concern of logging each invalidation separately. > > 0006 contains lots of XXX comments that look like real issues. I guess > those need to be fixed. Also, why don't we do the thing that the > commit message for 0006 says we could "theoretically" do? I don't > understand why we need the k-way merge at all, > I think we can do what is written in the commit message, but then we need to maintain two paths (one for streaming contexts and other for non-streaming contexts) unless we want to entirely get rid of storing subtransaction changes separately which seems like a more fundamental change. Right now, also to some extent such things are there, but I have already given a comment to minimize it. Having said that, I think we can go either way. I think the original intention was to avoid doing more stuff unless it is really required as this is already a big patchset, but maybe Tomas has a different idea about this. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
