public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v5 2/2] Add a --outdated option to reindexdb
9+ messages / 4 participants
[nested] [flat]
* [PATCH v5 2/2] Add a --outdated option to reindexdb
@ 2021-02-24 17:33 Julien Rouhaud <[email protected]>
0 siblings, 0 replies; 9+ messages in thread
From: Julien Rouhaud @ 2021-02-24 17:33 UTC (permalink / raw)
This uses the new OUTDATED option for REINDEX. If user asks for multiple job,
the list of tables to process will be sorted by the total size of underlying
indexes that have outdated dependency.
Author: Julien Rouhaud <[email protected]>
Reviewed-by:
Discussion: https://postgr.es/m/20201203093143.GA64934%40nol
---
src/bin/scripts/reindexdb.c | 143 ++++++++++++++++++++++++-----
src/bin/scripts/t/090_reindexdb.pl | 34 ++++++-
2 files changed, 151 insertions(+), 26 deletions(-)
diff --git a/src/bin/scripts/reindexdb.c b/src/bin/scripts/reindexdb.c
index cf28176243..369d164e65 100644
--- a/src/bin/scripts/reindexdb.c
+++ b/src/bin/scripts/reindexdb.c
@@ -35,20 +35,23 @@ typedef enum ReindexType
static SimpleStringList *get_parallel_object_list(PGconn *conn,
ReindexType type,
SimpleStringList *user_list,
+ bool outdated,
bool echo);
static void reindex_one_database(const ConnParams *cparams, ReindexType type,
SimpleStringList *user_list,
const char *progname,
bool echo, bool verbose, bool concurrently,
- int concurrentCons, const char *tablespace);
+ int concurrentCons, const char *tablespace,
+ bool outdated);
static void reindex_all_databases(ConnParams *cparams,
const char *progname, bool echo,
bool quiet, bool verbose, bool concurrently,
- int concurrentCons, const char *tablespace);
+ int concurrentCons, const char *tablespace,
+ bool outdated);
static void run_reindex_command(PGconn *conn, ReindexType type,
const char *name, bool echo, bool verbose,
bool concurrently, bool async,
- const char *tablespace);
+ const char *tablespace, bool outdated);
static void help(const char *progname);
@@ -74,6 +77,7 @@ main(int argc, char *argv[])
{"concurrently", no_argument, NULL, 1},
{"maintenance-db", required_argument, NULL, 2},
{"tablespace", required_argument, NULL, 3},
+ {"outdated", no_argument, NULL, 4},
{NULL, 0, NULL, 0}
};
@@ -95,6 +99,7 @@ main(int argc, char *argv[])
bool quiet = false;
bool verbose = false;
bool concurrently = false;
+ bool outdated = false;
SimpleStringList indexes = {NULL, NULL};
SimpleStringList tables = {NULL, NULL};
SimpleStringList schemas = {NULL, NULL};
@@ -170,6 +175,9 @@ main(int argc, char *argv[])
case 3:
tablespace = pg_strdup(optarg);
break;
+ case 4:
+ outdated = true;
+ break;
default:
fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
exit(1);
@@ -234,7 +242,8 @@ main(int argc, char *argv[])
cparams.dbname = maintenance_db;
reindex_all_databases(&cparams, progname, echo, quiet, verbose,
- concurrently, concurrentCons, tablespace);
+ concurrently, concurrentCons, tablespace,
+ outdated);
}
else if (syscatalog)
{
@@ -253,12 +262,17 @@ main(int argc, char *argv[])
pg_log_error("cannot reindex specific index(es) and system catalogs at the same time");
exit(1);
}
-
if (concurrentCons > 1)
{
pg_log_error("cannot use multiple jobs to reindex system catalogs");
exit(1);
}
+ if (outdated)
+ {
+ pg_log_error("cannot filter indexes having outdated dependencies "
+ "and reindex system catalogs at the same time");
+ exit(1);
+ }
if (dbname == NULL)
{
@@ -274,7 +288,7 @@ main(int argc, char *argv[])
reindex_one_database(&cparams, REINDEX_SYSTEM, NULL,
progname, echo, verbose,
- concurrently, 1, tablespace);
+ concurrently, 1, tablespace, outdated);
}
else
{
@@ -304,17 +318,20 @@ main(int argc, char *argv[])
if (schemas.head != NULL)
reindex_one_database(&cparams, REINDEX_SCHEMA, &schemas,
progname, echo, verbose,
- concurrently, concurrentCons, tablespace);
+ concurrently, concurrentCons, tablespace,
+ outdated);
if (indexes.head != NULL)
reindex_one_database(&cparams, REINDEX_INDEX, &indexes,
progname, echo, verbose,
- concurrently, 1, tablespace);
+ concurrently, 1, tablespace,
+ outdated);
if (tables.head != NULL)
reindex_one_database(&cparams, REINDEX_TABLE, &tables,
progname, echo, verbose,
- concurrently, concurrentCons, tablespace);
+ concurrently, concurrentCons, tablespace,
+ outdated);
/*
* reindex database only if neither index nor table nor schema is
@@ -323,7 +340,8 @@ main(int argc, char *argv[])
if (indexes.head == NULL && tables.head == NULL && schemas.head == NULL)
reindex_one_database(&cparams, REINDEX_DATABASE, NULL,
progname, echo, verbose,
- concurrently, concurrentCons, tablespace);
+ concurrently, concurrentCons, tablespace,
+ outdated);
}
exit(0);
@@ -334,7 +352,7 @@ reindex_one_database(const ConnParams *cparams, ReindexType type,
SimpleStringList *user_list,
const char *progname, bool echo,
bool verbose, bool concurrently, int concurrentCons,
- const char *tablespace)
+ const char *tablespace, bool outdated)
{
PGconn *conn;
SimpleStringListCell *cell;
@@ -363,6 +381,14 @@ reindex_one_database(const ConnParams *cparams, ReindexType type,
exit(1);
}
+ if (outdated && PQserverVersion(conn) < 140000)
+ {
+ PQfinish(conn);
+ pg_log_error("cannot use the \"%s\" option on server versions older than PostgreSQL %s",
+ "outdated", "14");
+ exit(1);
+ }
+
if (!parallel)
{
switch (process_type)
@@ -399,14 +425,24 @@ reindex_one_database(const ConnParams *cparams, ReindexType type,
*/
if (concurrently)
pg_log_warning("cannot reindex system catalogs concurrently, skipping all");
+ else if (outdated)
+ {
+ /*
+ * The only supported kind of object that can be outdated
+ * is collation. No system catalog has any index that can
+ * depend on an outdated collation, so skip system
+ * catalogs.
+ */
+ }
else
run_reindex_command(conn, REINDEX_SYSTEM, PQdb(conn), echo,
verbose, concurrently, false,
- tablespace);
+ tablespace, outdated);
/* Build a list of relations from the database */
process_list = get_parallel_object_list(conn, process_type,
- user_list, echo);
+ user_list, outdated,
+ echo);
process_type = REINDEX_TABLE;
/* Bail out if nothing to process */
@@ -419,7 +455,8 @@ reindex_one_database(const ConnParams *cparams, ReindexType type,
/* Build a list of relations from all the schemas */
process_list = get_parallel_object_list(conn, process_type,
- user_list, echo);
+ user_list, outdated,
+ echo);
process_type = REINDEX_TABLE;
/* Bail out if nothing to process */
@@ -484,7 +521,8 @@ reindex_one_database(const ConnParams *cparams, ReindexType type,
ParallelSlotSetHandler(free_slot, TableCommandResultHandler, NULL);
run_reindex_command(free_slot->connection, process_type, objname,
- echo, verbose, concurrently, true, tablespace);
+ echo, verbose, concurrently, true, tablespace,
+ outdated);
cell = cell->next;
} while (cell != NULL);
@@ -509,7 +547,7 @@ finish:
static void
run_reindex_command(PGconn *conn, ReindexType type, const char *name,
bool echo, bool verbose, bool concurrently, bool async,
- const char *tablespace)
+ const char *tablespace, bool outdated)
{
const char *paren = "(";
const char *comma = ", ";
@@ -536,6 +574,12 @@ run_reindex_command(PGconn *conn, ReindexType type, const char *name,
sep = comma;
}
+ if (outdated)
+ {
+ appendPQExpBuffer(&sql, "%sOUTDATED", sep);
+ sep = comma;
+ }
+
if (sep != paren)
appendPQExpBufferStr(&sql, ") ");
@@ -641,7 +685,7 @@ run_reindex_command(PGconn *conn, ReindexType type, const char *name,
*/
static SimpleStringList *
get_parallel_object_list(PGconn *conn, ReindexType type,
- SimpleStringList *user_list, bool echo)
+ SimpleStringList *user_list, bool outdated, bool echo)
{
PQExpBufferData catalog_query;
PQExpBufferData buf;
@@ -660,16 +704,41 @@ get_parallel_object_list(PGconn *conn, ReindexType type,
{
case REINDEX_DATABASE:
Assert(user_list == NULL);
+
appendPQExpBufferStr(&catalog_query,
"SELECT c.relname, ns.nspname\n"
" FROM pg_catalog.pg_class c\n"
" JOIN pg_catalog.pg_namespace ns"
- " ON c.relnamespace = ns.oid\n"
+ " ON c.relnamespace = ns.oid\n");
+
+ if (outdated)
+ {
+ appendPQExpBufferStr(&catalog_query,
+ " JOIN pg_catalog.pg_index i"
+ " ON c.oid = i.indrelid\n"
+ " JOIN pg_catalog.pg_class ci"
+ " ON i.indexrelid = ci.oid\n");
+ }
+
+ appendPQExpBufferStr(&catalog_query,
" WHERE ns.nspname != 'pg_catalog'\n"
" AND c.relkind IN ("
CppAsString2(RELKIND_RELATION) ", "
- CppAsString2(RELKIND_MATVIEW) ")\n"
- " ORDER BY c.relpages DESC;");
+ CppAsString2(RELKIND_MATVIEW) ")\n");
+
+ if (outdated)
+ {
+ appendPQExpBufferStr(&catalog_query,
+ " GROUP BY c.relname, ns.nspname\n"
+ " ORDER BY sum(ci.relpages)"
+ " FILTER (WHERE pg_catalog.pg_index_has_outdated_dependency(ci.oid)) DESC;");
+ }
+ else
+ {
+ appendPQExpBufferStr(&catalog_query,
+ " ORDER BY c.relpages DESC;");
+ }
+
break;
case REINDEX_SCHEMA:
@@ -687,7 +756,18 @@ get_parallel_object_list(PGconn *conn, ReindexType type,
"SELECT c.relname, ns.nspname\n"
" FROM pg_catalog.pg_class c\n"
" JOIN pg_catalog.pg_namespace ns"
- " ON c.relnamespace = ns.oid\n"
+ " ON c.relnamespace = ns.oid\n");
+
+ if (outdated)
+ {
+ appendPQExpBufferStr(&catalog_query,
+ " JOIN pg_catalog.pg_index i"
+ " ON c.oid = i.indrelid\n"
+ " JOIN pg_catalog.pg_class ci"
+ " ON i.indexrelid = ci.oid\n");
+ }
+
+ appendPQExpBufferStr(&catalog_query,
" WHERE c.relkind IN ("
CppAsString2(RELKIND_RELATION) ", "
CppAsString2(RELKIND_MATVIEW) ")\n"
@@ -705,8 +785,20 @@ get_parallel_object_list(PGconn *conn, ReindexType type,
appendStringLiteralConn(&catalog_query, nspname, conn);
}
- appendPQExpBufferStr(&catalog_query, ")\n"
- " ORDER BY c.relpages DESC;");
+ appendPQExpBufferStr(&catalog_query, ")\n");
+
+ if (outdated)
+ {
+ appendPQExpBufferStr(&catalog_query,
+ " GROUP BY c.relname, ns.nspname\n"
+ " ORDER BY sum(ci.relpages)"
+ " FILTER (WHERE pg_catalog.pg_index_has_outdated_dependency(ci.oid)) DESC;");
+ }
+ else
+ {
+ appendPQExpBufferStr(&catalog_query,
+ " ORDER BY c.relpages DESC;");
+ }
}
break;
@@ -754,7 +846,7 @@ static void
reindex_all_databases(ConnParams *cparams,
const char *progname, bool echo, bool quiet, bool verbose,
bool concurrently, int concurrentCons,
- const char *tablespace)
+ const char *tablespace, bool outdated)
{
PGconn *conn;
PGresult *result;
@@ -778,7 +870,7 @@ reindex_all_databases(ConnParams *cparams,
reindex_one_database(cparams, REINDEX_DATABASE, NULL,
progname, echo, verbose, concurrently,
- concurrentCons, tablespace);
+ concurrentCons, tablespace, outdated);
}
PQclear(result);
@@ -797,6 +889,7 @@ help(const char *progname)
printf(_(" -e, --echo show the commands being sent to the server\n"));
printf(_(" -i, --index=INDEX recreate specific index(es) only\n"));
printf(_(" -j, --jobs=NUM use this many concurrent connections to reindex\n"));
+ printf(_(" --outdated only process indexes having outdated depencies\n"));
printf(_(" -q, --quiet don't write any messages\n"));
printf(_(" -s, --system reindex system catalogs\n"));
printf(_(" -S, --schema=SCHEMA reindex specific schema(s) only\n"));
diff --git a/src/bin/scripts/t/090_reindexdb.pl b/src/bin/scripts/t/090_reindexdb.pl
index 159b637230..b60cac6081 100644
--- a/src/bin/scripts/t/090_reindexdb.pl
+++ b/src/bin/scripts/t/090_reindexdb.pl
@@ -3,7 +3,7 @@ use warnings;
use PostgresNode;
use TestLib;
-use Test::More tests => 58;
+use Test::More tests => 70;
program_help_ok('reindexdb');
program_version_ok('reindexdb');
@@ -174,6 +174,9 @@ $node->command_fails(
$node->command_fails(
[ 'reindexdb', '-j', '2', '-i', 'i1', 'postgres' ],
'parallel reindexdb cannot process indexes');
+$node->command_fails(
+ [ 'reindexdb', '-s', '--outdated' ],
+ 'cannot reindex system catalog and filter indexes having outdated dependencies');
$node->issues_sql_like(
[ 'reindexdb', '-j', '2', 'postgres' ],
qr/statement:\ REINDEX SYSTEM postgres;
@@ -196,3 +199,32 @@ $node->command_checks_all(
qr/^reindexdb: warning: cannot reindex system catalogs concurrently, skipping all/s
],
'parallel reindexdb for system with --concurrently skips catalogs');
+
+# Temporarily downgrade client-min-message to get the no-op report
+$ENV{PGOPTIONS} = '--client-min-messages=NOTICE';
+$node->command_checks_all(
+ [ 'reindexdb', '--outdated', '-v', '-t', 's1.t1', 'postgres' ],
+ 0,
+ [qr/^$/],
+ [qr/table "t1" has no indexes to reindex/],
+ 'verbose reindexdb for outdated dependencies on a specific table reports no-op tables');
+
+$node->command_checks_all(
+ [ 'reindexdb', '--outdated', '-v', '-d', 'postgres' ],
+ 0,
+ [qr/^$/],
+ [qr/^$/],
+ 'verbose reindexdb for outdated dependencies database wide silently ignore all tables');
+$node->command_checks_all(
+ [ 'reindexdb', '--outdated', '-v', '-j', '2', '-d', 'postgres' ],
+ 0,
+ [qr/^$/],
+ [qr/table "t1" has no indexes to reindex/],
+ 'parallel verbose reindexdb for outdated dependencies database wide reports no-op tables');
+
+# Switch back to WARNING client-min-message
+$ENV{PGOPTIONS} = '--client-min-messages=WARNING';
+$node->issues_sql_like(
+ [ 'reindexdb', '--outdated', '-t', 's1.t1', 'postgres' ],
+ qr/.*statement: REINDEX \(OUTDATED\) TABLE s1\.t1;/,
+ 'reindexdb for outdated dependencies specify the OUTDATED keyword');
--
2.30.1
--oao5se3im4osqowp--
^ permalink raw reply [nested|flat] 9+ messages in thread
* deferred writing of two-phase state files adds fragility
@ 2024-12-04 17:04 Robert Haas <[email protected]>
2024-12-04 17:19 ` Re: deferred writing of two-phase state files adds fragility Tom Lane <[email protected]>
2024-12-04 23:36 ` Re: deferred writing of two-phase state files adds fragility Andres Freund <[email protected]>
0 siblings, 2 replies; 9+ messages in thread
From: Robert Haas @ 2024-12-04 17:04 UTC (permalink / raw)
To: pgsql-hackers
Let's suppose that you execute PREPARE TRANSACTION and, before the
next CHECKPOINT, the WAL record for the PREPARE TRANSACTION gets
corrupted on disk. This might seem like an unlikely scenario, and it
is, but we saw a case at EDB not too long ago.
To a first approximation, the world ends. You can't execute COMMIT
TRANSACTION or ROLLBACK TRANSACTION, so there's now way to resolve the
prepared transaction. You also can't checkpoint, because that requires
writing a twophase state file for the prepared transaction, and that's
not possible because the WAL can't be read. What you have is a mostly
working system, except that it's going to bloat over time because the
prepared transaction is going to hold back the VACUUM horizon. And you
basically have no way out of that problem, because there's no tool
that says "I understand that my database is going to be corrupted,
that's ok, just forget about that twophase transaction".
If you shut down the database, then things become truly awful. You
can't get a clean shutdown because you can't checkpoint, so you're
going to resume recovery from the last checkpoint before the problem
happened, find the corrupted WAL, and fail. As long as your database
was up, you at least had the possibility of getting all of your data
out of it by running pg_dump, as long as you can survive the amount of
time that's going to take. And, if you did do that, you wouldn't even
have corruption. But once your database has gone down, you can't get
it back up again without running pg_resetwal. Running pg_resetwal is
not very appealing here -- first because now you do have corruption
whereas before the shutdown you didn't, and second because the last
checkpoint could already be a long time in the past, depending on how
quickly you realized you have this problem.
Before 728bd991c3c4389fb39c45dcb0fe57e4a1dccd71, things would not have
been quite so bad. Checkpoints wouldn't fail, so you might never even
realize you had a problem, or you might just need to rebuild your
standbys. If you had corruption in a different place, like the
twophase file itself, you could simply shut down cleanly, remove the
twophase file, and start back up. I'm not quite sure whether that's
equivalent to a forced abort of the twophase transaction or whether it
might leave you with some latent corruption, but I suspect the
problems you'll have will be pretty tame compared to what happens in
the scenario described above.
Just to be clear, I am not suggesting that we should revert that
commit. I'm actually not sure whether we should change anything at
all, but I'm not very comfortable with the status quo, either. It's
unavoidable that the database will sometimes end up in a bad state --
Murphy's law, entropy, or whatever you want to call it guarantees
that. But I like it a lot better when there's something that I can
reasonably do to get the database OUT of that bad state, and in this
situation nothing works -- or at least, nothing that I could think of
works. It would be nice to improve on that somehow, if anybody has a
good idea.
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: deferred writing of two-phase state files adds fragility
2024-12-04 17:04 deferred writing of two-phase state files adds fragility Robert Haas <[email protected]>
@ 2024-12-04 17:19 ` Tom Lane <[email protected]>
2024-12-04 17:44 ` Re: deferred writing of two-phase state files adds fragility Robert Haas <[email protected]>
1 sibling, 1 reply; 9+ messages in thread
From: Tom Lane @ 2024-12-04 17:19 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: pgsql-hackers
Robert Haas <[email protected]> writes:
> Let's suppose that you execute PREPARE TRANSACTION and, before the
> next CHECKPOINT, the WAL record for the PREPARE TRANSACTION gets
> corrupted on disk. This might seem like an unlikely scenario, and it
> is, but we saw a case at EDB not too long ago.
> To a first approximation, the world ends.
Ugh.
> You can't execute COMMIT
> TRANSACTION or ROLLBACK TRANSACTION, so there's now way to resolve the
> prepared transaction.
Could we fix it so ROLLBACK TRANSACTION removes the GID from the
list of prepared xacts that need to be written out? Then we'd
no longer have a pending requirement to read the broken WAL record.
> ... I'm not quite sure whether that's
> equivalent to a forced abort of the twophase transaction or whether it
> might leave you with some latent corruption, but I suspect the
> problems you'll have will be pretty tame compared to what happens in
> the scenario described above.
It should be fully equivalent to a database crash immediately before
committing an ordinary transaction. (I know various people keep
proposing that we load UNDO requirements onto transaction abort,
but I continue to think that's an awful idea, for precisely this
reason: you can't guarantee being able to execute the UNDO steps.
In any case, we don't have such requirements yet.)
regards, tom lane
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: deferred writing of two-phase state files adds fragility
2024-12-04 17:04 deferred writing of two-phase state files adds fragility Robert Haas <[email protected]>
2024-12-04 17:19 ` Re: deferred writing of two-phase state files adds fragility Tom Lane <[email protected]>
@ 2024-12-04 17:44 ` Robert Haas <[email protected]>
2024-12-04 17:58 ` Re: deferred writing of two-phase state files adds fragility Tom Lane <[email protected]>
0 siblings, 1 reply; 9+ messages in thread
From: Robert Haas @ 2024-12-04 17:44 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: pgsql-hackers
On Wed, Dec 4, 2024 at 12:19 PM Tom Lane <[email protected]> wrote:
> > You can't execute COMMIT
> > TRANSACTION or ROLLBACK TRANSACTION, so there's now way to resolve the
> > prepared transaction.
>
> Could we fix it so ROLLBACK TRANSACTION removes the GID from the
> list of prepared xacts that need to be written out? Then we'd
> no longer have a pending requirement to read the broken WAL record.
That would be nice, but I'm not sure that it's possible. As currently
implemented, FinishPreparedTransaction() always reads the two-phase
state data either from the two-phase file or the WAL, whether it's
committing or rolling back. One might expect the commit or rollback to
proceed purely on the basis of in-memory state, but I think that does
not work because nsubxacts might be greater than
PGPROC_MAX_CACHED_SUBXIDS. Even when it isn't, we have no shared
memory record of abortrels or abortstats.
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: deferred writing of two-phase state files adds fragility
2024-12-04 17:04 deferred writing of two-phase state files adds fragility Robert Haas <[email protected]>
2024-12-04 17:19 ` Re: deferred writing of two-phase state files adds fragility Tom Lane <[email protected]>
2024-12-04 17:44 ` Re: deferred writing of two-phase state files adds fragility Robert Haas <[email protected]>
@ 2024-12-04 17:58 ` Tom Lane <[email protected]>
2024-12-04 18:01 ` Re: deferred writing of two-phase state files adds fragility Robert Haas <[email protected]>
0 siblings, 1 reply; 9+ messages in thread
From: Tom Lane @ 2024-12-04 17:58 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: pgsql-hackers
Robert Haas <[email protected]> writes:
> On Wed, Dec 4, 2024 at 12:19 PM Tom Lane <[email protected]> wrote:
>> Could we fix it so ROLLBACK TRANSACTION removes the GID from the
>> list of prepared xacts that need to be written out? Then we'd
>> no longer have a pending requirement to read the broken WAL record.
> That would be nice, but I'm not sure that it's possible. As currently
> implemented, FinishPreparedTransaction() always reads the two-phase
> state data either from the two-phase file or the WAL, whether it's
> committing or rolling back.
I'm not following. FinishPreparedTransaction is not what's preventing
checkpoints or holding back the VACUUM horizon. What is doing that
is the in-memory fake PGPROC representing the prepared transaction
(I forget the exact terminology). I'm suggesting that we could have
some way to nuke one of those without properly cleaning up the
prepared xact. Maybe it'd need to be invoked via a different command
than ROLLBACK TRANSACTION.
regards, tom lane
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: deferred writing of two-phase state files adds fragility
2024-12-04 17:04 deferred writing of two-phase state files adds fragility Robert Haas <[email protected]>
2024-12-04 17:19 ` Re: deferred writing of two-phase state files adds fragility Tom Lane <[email protected]>
2024-12-04 17:44 ` Re: deferred writing of two-phase state files adds fragility Robert Haas <[email protected]>
2024-12-04 17:58 ` Re: deferred writing of two-phase state files adds fragility Tom Lane <[email protected]>
@ 2024-12-04 18:01 ` Robert Haas <[email protected]>
0 siblings, 0 replies; 9+ messages in thread
From: Robert Haas @ 2024-12-04 18:01 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: pgsql-hackers
On Wed, Dec 4, 2024 at 12:58 PM Tom Lane <[email protected]> wrote:
> Robert Haas <[email protected]> writes:
> > On Wed, Dec 4, 2024 at 12:19 PM Tom Lane <[email protected]> wrote:
> >> Could we fix it so ROLLBACK TRANSACTION removes the GID from the
> >> list of prepared xacts that need to be written out? Then we'd
> >> no longer have a pending requirement to read the broken WAL record.
>
> > That would be nice, but I'm not sure that it's possible. As currently
> > implemented, FinishPreparedTransaction() always reads the two-phase
> > state data either from the two-phase file or the WAL, whether it's
> > committing or rolling back.
>
> I'm not following. FinishPreparedTransaction is not what's preventing
> checkpoints or holding back the VACUUM horizon. What is doing that
> is the in-memory fake PGPROC representing the prepared transaction
> (I forget the exact terminology). I'm suggesting that we could have
> some way to nuke one of those without properly cleaning up the
> prepared xact. Maybe it'd need to be invoked via a different command
> than ROLLBACK TRANSACTION.
Yes, that we could do. Perhaps it could be added to pg_surgery.
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: deferred writing of two-phase state files adds fragility
2024-12-04 17:04 deferred writing of two-phase state files adds fragility Robert Haas <[email protected]>
@ 2024-12-04 23:36 ` Andres Freund <[email protected]>
2024-12-05 16:21 ` Re: deferred writing of two-phase state files adds fragility Robert Haas <[email protected]>
1 sibling, 1 reply; 9+ messages in thread
From: Andres Freund @ 2024-12-04 23:36 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: pgsql-hackers
Hi,
On 2024-12-04 12:04:47 -0500, Robert Haas wrote:
> Let's suppose that you execute PREPARE TRANSACTION and, before the
> next CHECKPOINT, the WAL record for the PREPARE TRANSACTION gets
> corrupted on disk. This might seem like an unlikely scenario, and it
> is, but we saw a case at EDB not too long ago.
>
> To a first approximation, the world ends. You can't execute COMMIT
> TRANSACTION or ROLLBACK TRANSACTION, so there's now way to resolve the
> prepared transaction.
Is 2PC really that special in that regard? If the WAL that contains the
checkpoint record itself gets corrupted, you're also in a world of hurt, once
you shut down? Or, to a slightly lower degree, if there's any corrupted
record between the redo pointer and the checkpoint record. And that's
obviously a lot more records than just 2PC COMMIT/RECORD, making the
likelihood of some corruption higher.
The only reason it seems somewhat special is that it can more easily be
noticed while the server is running.
How did this corruption actually come about? Did it actually really just
affect that single WAL segment? Somehow that doesn't seem too likely.
> You also can't checkpoint, because that requires
> writing a twophase state file for the prepared transaction, and that's
> not possible because the WAL can't be read. What you have is a mostly
> working system, except that it's going to bloat over time because the
> prepared transaction is going to hold back the VACUUM horizon. And you
> basically have no way out of that problem, because there's no tool
> that says "I understand that my database is going to be corrupted,
> that's ok, just forget about that twophase transaction".
> If you shut down the database, then things become truly awful. You
> can't get a clean shutdown because you can't checkpoint, so you're
> going to resume recovery from the last checkpoint before the problem
> happened, find the corrupted WAL, and fail. As long as your database
> was up, you at least had the possibility of getting all of your data
> out of it by running pg_dump, as long as you can survive the amount of
> time that's going to take. And, if you did do that, you wouldn't even
> have corruption. But once your database has gone down, you can't get
> it back up again without running pg_resetwal. Running pg_resetwal is
> not very appealing here -- first because now you do have corruption
> whereas before the shutdown you didn't, and second because the last
> checkpoint could already be a long time in the past, depending on how
> quickly you realized you have this problem.
pg_resetwal also won't actually remove the pg_twophase/* files if they did end
up getting created. But that's probably not a too common scenario.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: deferred writing of two-phase state files adds fragility
2024-12-04 17:04 deferred writing of two-phase state files adds fragility Robert Haas <[email protected]>
2024-12-04 23:36 ` Re: deferred writing of two-phase state files adds fragility Andres Freund <[email protected]>
@ 2024-12-05 16:21 ` Robert Haas <[email protected]>
2024-12-05 20:39 ` Re: deferred writing of two-phase state files adds fragility Andres Freund <[email protected]>
0 siblings, 1 reply; 9+ messages in thread
From: Robert Haas @ 2024-12-05 16:21 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: pgsql-hackers
On Wed, Dec 4, 2024 at 6:36 PM Andres Freund <[email protected]> wrote:
> Is 2PC really that special in that regard? If the WAL that contains the
> checkpoint record itself gets corrupted, you're also in a world of hurt, once
> you shut down? Or, to a slightly lower degree, if there's any corrupted
> record between the redo pointer and the checkpoint record. And that's
> obviously a lot more records than just 2PC COMMIT/RECORD, making the
> likelihood of some corruption higher.
Sure, that's true. I think my point is just that in a lot of cases
where the WAL gets corrupted, you can eventually move on from the
problem. Let's say some bad hardware or some annoying "security"
software decides to overwrite the most recent CHECKPOINT record. If
you go down at that point, you're sad, but if you don't, the server
will eventually write a new checkpoint record and then the old, bad
one doesn't really matter any more. If you have standbys you may need
to rebuild them and if you need logical decoding you may need to
recreate subscriptions or something, but since you didn't really end
up needing the bad WAL, the fact that it happened doesn't have to
cripple the system in any enduring sense.
> The only reason it seems somewhat special is that it can more easily be
> noticed while the server is running.
I think there are two things that make it special. The first is that
this is nearly the only case where the primary has a critical
dependency on the WAL in the absence of a crash. The second is that,
AFAICT, there's no reasonable recovery strategy.
> How did this corruption actually come about? Did it actually really just
> affect that single WAL segment? Somehow that doesn't seem too likely.
I don't know and might not be able to tell you even if I did.
> pg_resetwal also won't actually remove the pg_twophase/* files if they did end
> up getting created. But that's probably not a too common scenario.
Sure, but also, you can remove them yourself. IME, WAL corruption is
one of the worst case scenarios in terms of being able to get the
database back into reasonable shape. I can advise a customer to remove
an entire file if I need to; I have also written code to create fake
files to replace real ones that were lost; I have also written code to
fix broken heap pages. But when the problem is WAL, how are you
supposed to repair it? It's very difficult, I think, bordering on
impossible. Does anyone ever try to reconstruct a valid WAL stream to
allow replay to continue? AFAICT the only realistic solution is to run
pg_resetwal and hope that's good enough. That's often acceptable, but
it's not very nice in a case like this. Because you can't checkpoint,
you have no way to force the system to flush all dirty pages before
shutting it down, which means you may lose a bunch of data if you shut
down to run pg_resetwal. But if you don't shut down then you have no
way out of the bad state unless you can repair the WAL.
I don't think this is going to be a frequent case, so maybe it's not
worth doing anything about. But it does seem objectively worse than
most failure scenarios, at least to me.
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: deferred writing of two-phase state files adds fragility
2024-12-04 17:04 deferred writing of two-phase state files adds fragility Robert Haas <[email protected]>
2024-12-04 23:36 ` Re: deferred writing of two-phase state files adds fragility Andres Freund <[email protected]>
2024-12-05 16:21 ` Re: deferred writing of two-phase state files adds fragility Robert Haas <[email protected]>
@ 2024-12-05 20:39 ` Andres Freund <[email protected]>
0 siblings, 0 replies; 9+ messages in thread
From: Andres Freund @ 2024-12-05 20:39 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: pgsql-hackers
Hi,
On 2024-12-05 11:21:12 -0500, Robert Haas wrote:
> On Wed, Dec 4, 2024 at 6:36 PM Andres Freund <[email protected]> wrote:
> > How did this corruption actually come about? Did it actually really just
> > affect that single WAL segment? Somehow that doesn't seem too likely.
>
> I don't know and might not be able to tell you even if I did.
Understandable. I was mainly asking because I don't really see it making sense
to make significant investments into coping with WAL getting arbitrarily
corrupted. But if there's some systematic issue leading to corruption, adding
more infrastructure to detect problems seems more worthwhile / realistic.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 9+ messages in thread
end of thread, other threads:[~2024-12-05 20:39 UTC | newest]
Thread overview: 9+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-02-24 17:33 [PATCH v5 2/2] Add a --outdated option to reindexdb Julien Rouhaud <[email protected]>
2024-12-04 17:04 deferred writing of two-phase state files adds fragility Robert Haas <[email protected]>
2024-12-04 17:19 ` Re: deferred writing of two-phase state files adds fragility Tom Lane <[email protected]>
2024-12-04 17:44 ` Re: deferred writing of two-phase state files adds fragility Robert Haas <[email protected]>
2024-12-04 17:58 ` Re: deferred writing of two-phase state files adds fragility Tom Lane <[email protected]>
2024-12-04 18:01 ` Re: deferred writing of two-phase state files adds fragility Robert Haas <[email protected]>
2024-12-04 23:36 ` Re: deferred writing of two-phase state files adds fragility Andres Freund <[email protected]>
2024-12-05 16:21 ` Re: deferred writing of two-phase state files adds fragility Robert Haas <[email protected]>
2024-12-05 20:39 ` Re: deferred writing of two-phase state files adds fragility Andres Freund <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox