Hi Ajin. Some review comments for patch v16-0001.
====== Commit message 1. A hash cache of relation file locators is implemented to cache whether a relation is filterable or not. This ensures that we only need to retrieve ~ /hash cache/hash table/ (AFAICT you called this a hash table elsewhere in all code comments) ~~~ 2. Additionally, filtering is paused for transactions containing WAL records (INTERNAL_SNAPSHOT, COMMAND_ID, or INVALIDATION) that modify the historical snapshot constructed during logical decoding. This pause is necessary because constructing a correct historical snapshot for searching publication information requires processing these WAL records. ~ IIUC, the "pause" here is really referring to the 100 changes following an unfilterable txn. So, I don't think what you are describing here is a "pause" -- it is just another reason for the tx to be marked unfilterable, and the pause logic will take care of itself. So, maybe it should be worded more like SUGGESTION Additionally, transactions containing WAL records (INTERNAL_SNAPSHOT, COMMAND_ID, or INVALIDATION) that modify the historical snapshot constructed during logical decoding are deemed unfilterable. This is necessary because constructing a correct historical snapshot for searching publication information requires processing these WAL records. ====== src/backend/replication/logical/reorderbuffer.c 3. Header comment. If you change the commit message based on previous suggestions, then change the comment here also to match it. ~~~ 4. +/* + * After encountering a change that cannot be filtered out, filtering is + * temporarily suspended. Filtering resumes after processing every 100 changes. + * This strategy helps to minimize the overhead of performing a hash table + * search for each record, especially when most changes are not filterable. + */ +#define CHANGES_THRESHOLD_FOR_FILTER 100 /every 100/the next 100/ ~~~ +ReorderBufferFilterByRelFileLocator: 5. + * if we are decoding a transaction without these records. See comment on top + * of GetDecodableRelation() to see list of relations that are not + * decoded by the reorderbuffer. /to see list of relations/to see the kinds of relation/ ====== Kind Regards, Peter Smith. Fujitsu Australia
