Hi, On Thu, Aug 08, 2024 at 03:53:29PM +0800, cca5507 wrote: > Hi, > > > Thanks for pointing it out! > > > Here are the new version patches with a test case.
Thanks! I think the approach that the patch implements makes sense and that we should track the transactions that have been commmitted while building the snapshot. A few random comments: 1 === + * that the xlog in BUILDING_SNAPSHOT is only useful for build s/for build/to build? (same comment for the commit message) 2 === + * snapshot and will not be decoded. worth to mention DecodeTXNNeedSkip() here? 3 === + * point in decoding changes. Note that we only handle XLOG_HEAP2_NEW_CID + * which mark a transaction as catalog modifying in BUILDING_SNAPSHOT do you mean? Note that during BUILDING_SNAPSHOT, we only handle XLOG_HEAP2_NEW_CID as it marks a transaction as catalog modifying. If so, what about making the - if (SnapBuildCurrentState(builder) < SNAPBUILD_FULL_SNAPSHOT || + if (SnapBuildCurrentState(builder) < SNAPBUILD_BUILDING_SNAPSHOT || more clear about that? (to avoid any work when it's not needed) 4 === + * useful for build the snapshot. s/for/to/? 5 === + * point in decoding changes. Note that we only handle XLOG_HEAP_INPLACE + * which mark a transaction as catalog modifying in BUILDING_SNAPSHOT, it's s/which mark/which might mark/? do you mean? Note that during BUILDING_SNAPSHOT, we only handle XLOG_HEAP_INPLACE as it might mark a transaction as catalog modifying. If so, what about making the - if (SnapBuildCurrentState(builder) < SNAPBUILD_FULL_SNAPSHOT || + if (SnapBuildCurrentState(builder) < SNAPBUILD_BUILDING_SNAPSHOT || more clear about that? (to avoid any work when it's not needed) Idea of 3 === and 5 === is to proceed further in the SNAPBUILD_BUILDING_SNAPSHOT case only if we know that the transaction is a catalog changing one (or might be one). 6 === + * useful for build the snapshot. s/for/to/? 7 === +# Test snapshot build correctly + what about? Test tracking of committed transactions during BUILDING_SNAPSHOT Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
