Re: minor improvement in snapbuild: use existing interface and removefake code

ocean_li_996 <ocean_li_996@163.com>

From: ocean_li_996 <ocean_li_996@163.com>
To: cca5507 <cca5507@qq.com>
Cc: pgsql-hackers <pgsql-hackers@lists.postgresql.org>, "mohen.lhy@alibaba-inc.com" <mohen.lhy@alibaba-inc.com>
Date: 2025-11-18T16:26:19Z
Lists: pgsql-hackers

Attachments


Hi ChangAo,


Thanks for your comments.


At 2025-11-18 10:47:20, "cca5507" <cca5507@qq.com> wrote:
> Is there a test case can reproduce the assert fail in SnapBuildGetOrBuildSnapshot()?
> 
After exploring the logicalmsg_decode(),I think the Assert in SnapBuildGetOrBuildSnapshot() will not fail.
But the assert in SnapBuildGetOrBuildSnapshot() can be removed if we want to reuse it.


> We skip xact_decode() when SnapBuildCurrentState(builder) < SNAPBUILD_FULL_SNAPSHOT, but it's a bug, so your NO.3 modification might be wrong.
> 
> See threads in this for details: https://commitfest.postgresql.org/patch/5029/


Wow, what a nice surprise! A few days ago, I reported a bug in [1], which I believe is the same issue you have described here.
Unfortunately, I was not able to provide a simple reproduction case at that time — but now we seem to have one :).
Regarding the fix for that issue: I will review the patch you have provided. My current idea is to update the snapshot using
builder->xmin once we reach the SNAPBUILD_CONSISTENT state. If you have any thoughts, please feel free to join the
discussion in [1].


As for the comment about this path, I can revert it currently.
Please find the updated patch attached.


[1] https://www.postgresql.org/message-id/2b9e5ac8.136f.19a8f7297ee.Coremail.ocean_li_996%40163.com


Regards,
Haiyang Li