On 30/03/18 00:30, Petr Jelinek wrote: > On 29/03/18 23:58, Andres Freund wrote: >> On 2018-03-29 23:52:18 +0200, Tomas Vondra wrote: >>>> I have added details about this in src/backend/storage/lmgr/README as >>>> suggested by you. >>>> >>> >>> Thanks. I think the README is a good start, but I think we also need to >>> improve the comments, which is usually more detailed than the README. >>> For example, it's not quite acceptable that LogicalLockTransaction and >>> LogicalUnlockTransaction have about no comments, especially when it's >>> meant to be public API for decoding plugins. >> >> FWIW, for me that's ground to not accept the feature. Burdening output >> plugins with this will make their development painful (because they'll >> have to adapt regularly) and correctness doubful (there's nothing >> checking for the lock being skipped). Another way needs to be found. >> > > I have to agree with Andres here. It's also visible in the latter > patches. The pgoutput patch forgets to call these new APIs completely. > The test_decoding calls them, but it does so even when it's processing > changes for committed transaction.. I think that should be avoided as it > means potentially doing SLRU lookup for every change. So doing it right > is indeed not easy.
Ah turns out it actually does not need SLRU lookup in this case (I missed the reorder buffer call), so I take that part back. -- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
