Re: Spurious "apparent wraparound" via SimpleLruTruncate() rounding

On Fri, Jan 01, 2021 at 11:05:29PM +0500, Andrey Borodin wrote:
> I've found this thread in CF looking for something to review.

Thanks for taking a look.

> > 9 нояб. 2020 г., в 09:53, Noah Misch <noah(at)leadboat(dot)com> написал(а):
> >
> > Rebased both patches, necessitated by commit c732c3f (a repair of commit
> > dee663f). As I mentioned on another branch of the thread, I'd be content if
> > someone reviews the slru-truncate-modulo patch and disclaims knowledge of the
> > slru-truncate-insurance patch; I would then abandon the latter patch.
> > <slru-truncate-modulo-v5.patch><slru-truncate-t-insurance-v4.patch>
>
> Commit c732c3f adds some SYNC_FORGET_REQUESTs.
> slru-truncate-modulo-v5.patch fixes off-by-one error in functions like *PagePrecedes(int page1, int page2).
> slru-truncate-t-insurance-v4.patch ensures that off-by-one errors do not inflict data loss.
>
> While I agree that fixing error is better than hiding it, I could not figure out how c732c3f is connected to these patches.
> Can you please give me few pointers how to understand this connection?

Commit c732c3f is the last commit that caused a merge conflict. There's no
other connection to this thread, and one can review patches on this thread
without studying commit c732c3f. Specifically, this thread's
slru-truncate-modulo patch and commit c732c3f modify adjacent lines in
SlruScanDirCbDeleteCutoff(); here's the diff after merge conflict resolution:

--- a/src/backend/access/transam/slru.c
+++ b/src/backend/access/transam/slru.c
@@@ -1525,4 -1406,4 +1517,4 @@@ SlruScanDirCbDeleteCutoff(SlruCtl ctl,

- if (ctl->PagePrecedes(segpage, cutoffPage))
+ if (SlruMayDeleteSegment(ctl, segpage, cutoffPage))
- SlruInternalDeleteSegment(ctl, filename);
+ SlruInternalDeleteSegment(ctl, segpage / SLRU_PAGES_PER_SEGMENT);