At Thu, 21 Jan 2021 00:19:58 -0800, Noah Misch <n...@leadboat.com> wrote in > On Thu, Jan 21, 2021 at 12:28:44AM +0900, Kyotaro Horiguchi wrote: > > At Wed, 20 Jan 2021 17:34:44 +0900 (JST), Kyotaro Horiguchi > > <horikyota....@gmail.com> wrote in > > > Anyway, it seems actually dangerous that cause pruning on wal-skipped > > > relation. > > > > > > > with your patch versions. Could you try implementing both test > > > > procedures in > > > > src/test/modules/snapshot_too_old? There's no need to make the test use > > > > wal_level=minimal explicitly; it's sufficient to catch these bugs when > > > > wal_level=minimal is in the TEMP_CONFIG file. > > > > > > In the attached, TestForOldSnapshot() considers XLogIsNeeded(), > > > instead of moving the condition on LSN to TestForOldSnapshot_impl for > > > performance. > > > > > > I'll add the test part in the next version. > > That test helped me. I now see "there's not a single tuple removed due to > old_snapshot_threshold in src/test/modules/snapshot_too_old"[1], which limits > our ability to test using this infrastructure.
Yes. > > However, with the previous patch, two existing tests sto_using_cursor > > and sto_using_select behaves differently from the master. That change > > is coming from the omission of actual LSN check in TestForOldSnapshot > > while wal_level=minimal. So we have no choice other than actually > > updating page LSN. > > > > In the scenario under discussion there are two places we need to do > > that. one is heap_page_prune, and the other is RelationCopyStorge. As > > a PoC, I used gistXLogAssignLSN() as is for thie purpose. See the > > attached third file. > > Fake LSNs make the system harder to understand, so I prefer not to spread fake > LSNs to more access methods. What I had in mind is to simply suppress early > pruning when the relation is skipping WAL. Attached. Is this reasonable? It > passes the older tests. While it changes the sto_wal_optimized.spec output, I > think it preserves the old_snapshot_threshold behavior contract. Perhaps I'm missing something, but the patch doesn't pass the v5-0001 test with wal_level=minimal? > [1] > https://www.postgresql.org/message-id/20200403001235.e6jfdll3gh2ygbuc%40alap3.anarazel.de regards. -- Kyotaro Horiguchi NTT Open Source Software Center
