On Wed, Feb 12, 2025 at 10:41 AM Ajin Cherian <itsa...@gmail.com> wrote: > > On Wed, Jan 29, 2025 at 9:31 AM Peter Smith <smithpb2...@gmail.com> wrote: > > Hi Ajin, > > Some review comments for patch v12-0001. > > ====== > Commit message > > 1. > Track transactions which have snapshot changes with a new flag > RBTXN_HAS_SNAPSHOT_CHANGES > > ~ > > The commit message only says *what* it does, but not *why* this patch > even exists. TBH, I don't understand why this patch needs to be > separated from your patch 0002, because 0001 makes no independent use > of the flag, nor is it separately tested. > > Anyway, if it is going to remain separated then IMO at least the the > message should explain the intended purpose e.g. why the subsequent > patches require this flagged info and how they will use it. > > > Fixed. >
I still can't get from 0001's commit message the reason for tracking the snapshot changes separately. Also, please find my comments for 0002's commit message. > When most changes in a transaction are unfilterable, the overhead of starting a transaction for each record is significant. > Can you tell what is the exact overhead by testing it? IIRC, that was the initial approach. It is better if you can mention in the commit message what was overhead. It will be easier for reviewers. > To reduce this overhead a hash cache of relation file locators is created. Even then a hash search for every record before recording has considerable overhead especially for use cases where most tables in an instance are published. > Again, can you share the link of performance data for this overhead and if you have not published then please share it and also mention it in commit message? > To further reduce this overhead a simple approach is used to suspend filtering for a certain number of changes (100) when an unfilterable change is encountered. In other words, continue filtering changes if the last record was filtered out. If an unfilterable change is found, skip filtering the next 100 changes. > Can we try different thresholds for this like 10, 50, 100, 200, etc. to decide what is a good threshold value to skip filtering changes? -- With Regards, Amit Kapila.
