pgsql: Remove secondary checkpoint

On 2017-11-07 17:57:31 +0000, Simon Riggs wrote:

Remove secondary checkpoint

Previously server reserved WAL for last two checkpoints,
which used too much disk space for small servers.

Bumps PG_CONTROL_VERSION

FWIW, I don't think this should be applied without a pg_resetxlog
feature allowing to manually use older checkpoints.

Greetings,

Andres Freund

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

Andres Freund <andres@anarazel.de> writes:

On 2017-11-07 17:57:31 +0000, Simon Riggs wrote:

Remove secondary checkpoint

FWIW, I don't think this should be applied without a pg_resetxlog
feature allowing to manually use older checkpoints.

Uh, what? We have never before insisted on pg_resetxlog being able
to use WAL across a WAL format change, and there have been many of
those.

regards, tom lane

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

On 2017-11-07 14:12:19 -0500, Tom Lane wrote:

Andres Freund <andres@anarazel.de> writes:

On 2017-11-07 17:57:31 +0000, Simon Riggs wrote:

Remove secondary checkpoint

FWIW, I don't think this should be applied without a pg_resetxlog
feature allowing to manually use older checkpoints.

Uh, what? We have never before insisted on pg_resetxlog being able
to use WAL across a WAL format change, and there have been many of
those.

I think you misunderstand my point - I'm saying that pg_resetxlog should
be able to force the use of older checkpoints, basically as a fallback
to cases where the previous approach might actually have worked, not
that it needs to work across format changes.

Greetings,

Andres Freund

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

Andres Freund <andres@anarazel.de> writes:

I think you misunderstand my point - I'm saying that pg_resetxlog should
be able to force the use of older checkpoints, basically as a fallback
to cases where the previous approach might actually have worked, not
that it needs to work across format changes.

That seems like a completely separate feature --- and one of dubious
value, frankly. The further back you go, the less likely it'd be
to work.

regards, tom lane

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

On Wed, Nov 8, 2017 at 4:43 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Andres Freund <andres@anarazel.de> writes:

I think you misunderstand my point - I'm saying that pg_resetxlog should
be able to force the use of older checkpoints, basically as a fallback
to cases where the previous approach might actually have worked, not
that it needs to work across format changes.

That seems like a completely separate feature --- and one of dubious
value, frankly. The further back you go, the less likely it'd be
to work.

Because the less guarantees you would have to reach a consistent point
with a non-corrupted instance. I think that Andres' idea here are
worth debating though. Why not just spawning a new thread on the
matter and summarize what you have in mind?
--
Michael

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

Hi,

sqlsmith doesn't like commit 4b0d28de06:

,----
| regression=> select * from pg_control_checkpoint();
| server closed the connection unexpectedly
| TRAP: FailedAssertion("!((atti->attalign) == 's')", File: "heaptuple.c", Line: 126)
`----

On a build with assertions disabled, the statement fails with an error
instead:

,----
| regression=> select * from pg_control_checkpoint();
| ERROR: function return row and query-specified return row do not match
| DETAIL: Returned row contains 19 attributes, but query expects 18.
`----

The attached patch fixes it for me.

regards,
Andreas

On Sat, Nov 11, 2017 at 10:58 PM, Andreas Seltenreich
<seltenreich@gmx.de> wrote:

Hi,

sqlsmith doesn't like commit 4b0d28de06:

,----
| regression=> select * from pg_control_checkpoint();
| server closed the connection unexpectedly
| TRAP: FailedAssertion("!((atti->attalign) == 's')", File: "heaptuple.c", Line: 126)
`----

On a build with assertions disabled, the statement fails with an error
instead:

,----
| regression=> select * from pg_control_checkpoint();
| ERROR: function return row and query-specified return row do not match
| DETAIL: Returned row contains 19 attributes, but query expects 18.
`----

The attached patch fixes it for me.

Your patch looks correct to me. I can reproduce the problem and
verified that patch fixes the problem. It is better to track this in
CF if not already tracked.

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

On 20 November 2017 at 08:38, Amit Kapila <amit.kapila16@gmail.com> wrote:

On Sat, Nov 11, 2017 at 10:58 PM, Andreas Seltenreich
<seltenreich@gmx.de> wrote:

Hi,

sqlsmith doesn't like commit 4b0d28de06:

,----
| regression=> select * from pg_control_checkpoint();
| server closed the connection unexpectedly
| TRAP: FailedAssertion("!((atti->attalign) == 's')", File: "heaptuple.c", Line: 126)
`----

On a build with assertions disabled, the statement fails with an error
instead:

,----
| regression=> select * from pg_control_checkpoint();
| ERROR: function return row and query-specified return row do not match
| DETAIL: Returned row contains 19 attributes, but query expects 18.
`----

The attached patch fixes it for me.

Your patch looks correct to me. I can reproduce the problem and
verified that patch fixes the problem. It is better to track this in
CF if not already tracked.

What email and patch is this referring to?

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 2017-11-20 15:50:40 -0500, Simon Riggs wrote:

On 20 November 2017 at 08:38, Amit Kapila <amit.kapila16@gmail.com> wrote:

Your patch looks correct to me. I can reproduce the problem and
verified that patch fixes the problem. It is better to track this in
CF if not already tracked.

What email and patch is this referring to?

/messages/by-id/878tfcsp1t.fsf@ansel.ydns.eu

On 20 November 2017 at 15:55, Andres Freund <andres@anarazel.de> wrote:

On 2017-11-20 15:50:40 -0500, Simon Riggs wrote:

On 20 November 2017 at 08:38, Amit Kapila <amit.kapila16@gmail.com> wrote:

Your patch looks correct to me. I can reproduce the problem and
verified that patch fixes the problem. It is better to track this in
CF if not already tracked.

What email and patch is this referring to?

/messages/by-id/878tfcsp1t.fsf@ansel.ydns.eu

Apologies to Andreas, I didn't receive that email

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Wed, Nov 8, 2017 at 6:57 AM, Simon Riggs <simon@2ndquadrant.com> wrote:

Remove secondary checkpoint

I think this comment in xlog.c was missed:

-               /*
-                * Get the last valid checkpoint record.  If the
latest one according
-                * to pg_control is broken, try the next-to-last one.
-                */
+               /* Get the last valid checkpoint record. */

--
Thomas Munro
http://www.enterprisedb.com

On Tue, Feb 6, 2018 at 4:42 PM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:

On Wed, Nov 8, 2017 at 6:57 AM, Simon Riggs <simon@2ndquadrant.com> wrote:

Remove secondary checkpoint

I think this comment in xlog.c was missed:

-               /*
-                * Get the last valid checkpoint record.  If the
latest one according
-                * to pg_control is broken, try the next-to-last one.
-                */
+               /* Get the last valid checkpoint record. */

Committed.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company