Horton

Thread

Commits

  1. Remove xloginsert.h from xlog.h

  2. Revamp the WAL record format.

  3. Move the backup-block logic from XLogInsert to a new file, xloginsert.c.

  1. Remove extra includes of "access/xloginsert.h" when "access/xlog.h" is included

    Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> — 2022-01-28T14:10:02Z

    Hi,

It seems like there are some instances where xloginsert.h is included
right after xlog.h but xlog.h has already included xloginsert.h.
Unless I'm missing something badly, we can safely remove including
xloginsert.h after xlog.h. Attempting to post a patch to remove the
extra xloginsert.h includes.

Thoughts?

Regards,
Bharath Rupireddy.

  2. Re: Remove extra includes of "access/xloginsert.h" when "access/xlog.h" is included

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2022-01-28T15:55:00Z

    On 2022-Jan-28, Bharath Rupireddy wrote:

> Hi,
> 
> It seems like there are some instances where xloginsert.h is included
> right after xlog.h but xlog.h has already included xloginsert.h.
> Unless I'm missing something badly, we can safely remove including
> xloginsert.h after xlog.h. Attempting to post a patch to remove the
> extra xloginsert.h includes.

Why isn't it better to remove the line that includes xloginsert.h in
xlog.h instead?  When xloginsert.h was introduced (commit 2076db2aea76),
XLogRecData was put there so xloginsert.h was necessary for xlog.h; but
now we have a forward declaration (per commit 2c03216d8311) so it
doesn't seem needed anymore.

-- 
Álvaro Herrera              Valdivia, Chile  —  https://www.EnterpriseDB.com/

  3. Re: Remove extra includes of "access/xloginsert.h" when "access/xlog.h" is included

    Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> — 2022-01-29T13:42:13Z

    On Fri, Jan 28, 2022 at 9:25 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>
> On 2022-Jan-28, Bharath Rupireddy wrote:
>
> > Hi,
> >
> > It seems like there are some instances where xloginsert.h is included
> > right after xlog.h but xlog.h has already included xloginsert.h.
> > Unless I'm missing something badly, we can safely remove including
> > xloginsert.h after xlog.h. Attempting to post a patch to remove the
> > extra xloginsert.h includes.
>
> Why isn't it better to remove the line that includes xloginsert.h in
> xlog.h instead?  When xloginsert.h was introduced (commit 2076db2aea76),
> XLogRecData was put there so xloginsert.h was necessary for xlog.h; but
> now we have a forward declaration (per commit 2c03216d8311) so it
> doesn't seem needed anymore.

Removing the xloginsert.h in xlog.h would need us to add xloginsert.h
in more areas. And also, it might break any non-core extensions that
includes just xlog.h and gets xloginsert.h. Instead I prefer removing
xloginsert.h if there's xlog.h included.

Attaching v2 patch removing xloginsert.h in a few more places.

Regards,
Bharath Rupireddy.

  4. Re: Remove extra includes of "access/xloginsert.h" when "access/xlog.h" is included

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2022-01-29T19:37:20Z

    On 2022-Jan-29, Bharath Rupireddy wrote:

> Removing the xloginsert.h in xlog.h would need us to add xloginsert.h
> in more areas.

Sure.

> And also, it might break any non-core extensions that
> includes just xlog.h and gets xloginsert.h.

That's a pretty easy fix anyway -- it's not even version-specific, since
the fix would work with the older versions.  It's not something that
would break on a minor version, either.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"The problem with the facetime model is not just that it's demoralizing, but
that the people pretending to work interrupt the ones actually working."
                                                           (Paul Graham)

  5. Re: Remove extra includes of "access/xloginsert.h" when "access/xlog.h" is included

    Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> — 2022-01-30T13:22:48Z

    On Sun, Jan 30, 2022 at 1:07 AM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>
> On 2022-Jan-29, Bharath Rupireddy wrote:
>
> > Removing the xloginsert.h in xlog.h would need us to add xloginsert.h
> > in more areas.
>
> Sure.
>
> > And also, it might break any non-core extensions that
> > includes just xlog.h and gets xloginsert.h.
>
> That's a pretty easy fix anyway -- it's not even version-specific, since
> the fix would work with the older versions.  It's not something that
> would break on a minor version, either.

Here's the v3 patch removing xloginsert.h from xlog.h and adding
xloginsert.h in the required files.

Regards,
Bharath Rupireddy.

  6. Re: Remove extra includes of "access/xloginsert.h" when "access/xlog.h" is included

    Julien Rouhaud <rjuju123@gmail.com> — 2022-01-30T14:49:12Z

    Hi,

On Sun, Jan 30, 2022 at 06:52:48PM +0530, Bharath Rupireddy wrote:
> 
> Here's the v3 patch removing xloginsert.h from xlog.h and adding
> xloginsert.h in the required files.

+1, this approach is better.  In general it's better to increase the number of
include lines rather than having a few that includes everything, for
compilation performance.

  7. Re: Remove extra includes of "access/xloginsert.h" when "access/xlog.h" is included

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2022-01-30T15:27:11Z

    On 2022-Jan-30, Bharath Rupireddy wrote:

> Here's the v3 patch removing xloginsert.h from xlog.h and adding
> xloginsert.h in the required files.

Pushed, but I added xloginsert.h to 9 additional files that needed it to
avoid compiler warnings.  Thanks!

-- 
Álvaro Herrera              Valdivia, Chile  —  https://www.EnterpriseDB.com/