On Tue, Feb 28, 2023 at 5:52 AM Nathan Bossart <nathandbossart@gmail.com> wrote:
>
> On Tue, Feb 07, 2023 at 07:30:00PM +0530, Bharath Rupireddy wrote:
> > + /*
> > + * Try updating oldest initialized XLog buffer page.
> > + *
> > + * Update it if we are initializing an XLog buffer page for the first
> > + * time or if XLog buffers are full and we are wrapping around.
> > + */
> > + if (XLogRecPtrIsInvalid(XLogCtl->OldestInitializedPage) ||
> > + (!XLogRecPtrIsInvalid(XLogCtl->OldestInitializedPage) &&
> > + XLogRecPtrToBufIdx(XLogCtl->OldestInitializedPage) == nextidx))
> > + {
> > + Assert(XLogCtl->OldestInitializedPage < NewPageBeginPtr);
> > +
> > + XLogCtl->OldestInitializedPage = NewPageBeginPtr;
> > + }
>
> nitpick: I think you can simplify the conditional to
>
> if (XLogRecPtrIsInvalid(XLogCtl->OldestInitializedPage) ||
> XLogRecPtrToBufIdx(XLogCtl->OldestInitializedPage) == nextidx)
Oh, yes, done that.
> It's confusing to me that OldestInitializedPage is set to NewPageBeginPtr.
> Doesn't that set it to the beginning of the newest initialized page?
Yes, that's the intention, see below. OldestInitializedPage points to
the start address of the oldest initialized page whereas the
InitializedUpTo points to the end address of the latest initialized
page. With this, one can easily track all the WAL between
OldestInitializedPage and InitializedUpTo.
+ /*
+ * OldestInitializedPage and InitializedUpTo are always starting and
+ * ending addresses of (same or different) XLog buffer page
+ * respectively. Hence, they can never be same even if there's only one
+ * initialized page in XLog buffers.
+ */
+ Assert(XLogCtl->OldestInitializedPage != XLogCtl->InitializedUpTo);
Thanks for looking at it. I'm attaching v2 patch with the above review
comment addressed for further review.
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
