Skip to content

Commit c25314d

Browse files
alvherrealvherre
committed
pg_createsubscriber: reword dry-run log messages
The original messages were confusing in dry-run mode in that they state that something is being done, when in reality it isn't. Use alternative wording in that case, to make the distinction clear. Author: Peter Smith <smithpb2250@gmail.com> Reviewed-by: Chao Li <li.evan.chao@gmail.com> Reviewed-by: Euler Taveira <euler@eulerto.com> Backpatch-through: 18 Discussion: https://postgr.es/m/CAHut+PsvQJQnQO0KT0S2oegenkvJ8FUuY-QS5syyqmT24R2xFQ@mail.gmail.com
1 parent 59aeb69 commit c25314d

File tree

2 files changed

+70
-37
lines changed

2 files changed

+70
-37
lines changed

‎src/bin/pg_basebackup/pg_createsubscriber.c‎

Lines changed: 67 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -679,13 +679,20 @@ modify_subscriber_sysid(const struct CreateSubscriberOptions *opt)
679679
cf->system_identifier |= ((uint64) tv.tv_usec) << 12;
680680
cf->system_identifier |= getpid() & 0xFFF;
681681

682-
if (!dry_run)
682+
if (dry_run)
683+
pg_log_info("dry-run: would set system identifier to %" PRIu64 " on subscriber",
684+
cf->system_identifier);
685+
else
686+
{
683687
update_controlfile(subscriber_dir, cf, true);
688+
pg_log_info("system identifier is %" PRIu64 " on subscriber",
689+
cf->system_identifier);
690+
}
684691

685-
pg_log_info("system identifier is %" PRIu64 " on subscriber",
686-
cf->system_identifier);
687-
688-
pg_log_info("running pg_resetwal on the subscriber");
692+
if (dry_run)
693+
pg_log_info("dry-run: would run pg_resetwal on the subscriber");
694+
else
695+
pg_log_info("running pg_resetwal on the subscriber");
689696

690697
cmd_str = psprintf("\"%s\" -D \"%s\" > \"%s\"", pg_resetwal_path,
691698
subscriber_dir, DEVNULL);
@@ -977,9 +984,9 @@ check_publisher(const struct LogicalRepInfo *dbinfo)
977984
}
978985

979986
/*
980-
* Validate 'max_slot_wal_keep_size'. If this parameter is set to a
981-
* non-default value, it may cause replication failures due to required
982-
* WAL files being prematurely removed.
987+
* In dry-run mode, validate 'max_slot_wal_keep_size'. If this parameter
988+
* is set to a non-default value, it may cause replication failures due to
989+
* required WAL files being prematurely removed.
983990
*/
984991
if (dry_run && (strcmp(max_slot_wal_keep_size, "-1") != 0))
985992
{
@@ -1124,11 +1131,14 @@ drop_existing_subscriptions(PGconn *conn, const char *subname, const char *dbnam
11241131
subname);
11251132
appendPQExpBuffer(query, " DROP SUBSCRIPTION %s;", subname);
11261133

1127-
pg_log_info("dropping subscription \"%s\" in database \"%s\"",
1128-
subname, dbname);
1129-
1130-
if (!dry_run)
1134+
if (dry_run)
1135+
pg_log_info("dry-run: would drop subscription \"%s\" in database \"%s\"",
1136+
subname, dbname);
1137+
else
11311138
{
1139+
pg_log_info("dropping subscription \"%s\" in database \"%s\"",
1140+
subname, dbname);
1141+
11321142
res = PQexec(conn, query->data);
11331143

11341144
if (PQresultStatus(res) != PGRES_COMMAND_OK)
@@ -1269,7 +1279,7 @@ setup_recovery(const struct LogicalRepInfo *dbinfo, const char *datadir, const c
12691279

12701280
if (dry_run)
12711281
{
1272-
appendPQExpBufferStr(recoveryconfcontents, "# dry run mode");
1282+
appendPQExpBufferStr(recoveryconfcontents, "# dry run mode\n");
12731283
appendPQExpBuffer(recoveryconfcontents,
12741284
"recovery_target_lsn = '%X/%X'\n",
12751285
LSN_FORMAT_ARGS((XLogRecPtr) InvalidXLogRecPtr));
@@ -1375,8 +1385,12 @@ create_logical_replication_slot(PGconn *conn, struct LogicalRepInfo *dbinfo)
13751385

13761386
Assert(conn != NULL);
13771387

1378-
pg_log_info("creating the replication slot \"%s\" in database \"%s\"",
1379-
slot_name, dbinfo->dbname);
1388+
if (dry_run)
1389+
pg_log_info("dry-run: would create the replication slot \"%s\" in database \"%s\" on publisher",
1390+
slot_name, dbinfo->dbname);
1391+
else
1392+
pg_log_info("creating the replication slot \"%s\" in database \"%s\" on publisher",
1393+
slot_name, dbinfo->dbname);
13801394

13811395
slot_name_esc = PQescapeLiteral(conn, slot_name, strlen(slot_name));
13821396

@@ -1424,8 +1438,12 @@ drop_replication_slot(PGconn *conn, struct LogicalRepInfo *dbinfo,
14241438

14251439
Assert(conn != NULL);
14261440

1427-
pg_log_info("dropping the replication slot \"%s\" in database \"%s\"",
1428-
slot_name, dbinfo->dbname);
1441+
if (dry_run)
1442+
pg_log_info("dry-run: would drop the replication slot \"%s\" in database \"%s\"",
1443+
slot_name, dbinfo->dbname);
1444+
else
1445+
pg_log_info("dropping the replication slot \"%s\" in database \"%s\"",
1446+
slot_name, dbinfo->dbname);
14291447

14301448
slot_name_esc = PQescapeLiteral(conn, slot_name, strlen(slot_name));
14311449

@@ -1569,13 +1587,8 @@ wait_for_end_recovery(const char *conninfo, const struct CreateSubscriberOptions
15691587

15701588
for (;;)
15711589
{
1572-
bool in_recovery = server_is_in_recovery(conn);
1573-
1574-
/*
1575-
* Does the recovery process finish? In dry run mode, there is no
1576-
* recovery mode. Bail out as the recovery process has ended.
1577-
*/
1578-
if (!in_recovery || dry_run)
1590+
/* Did the recovery process finish? We're done if so. */
1591+
if (dry_run || !server_is_in_recovery(conn))
15791592
{
15801593
status = POSTMASTER_READY;
15811594
recovery_ended = true;
@@ -1651,8 +1664,12 @@ create_publication(PGconn *conn, struct LogicalRepInfo *dbinfo)
16511664
PQclear(res);
16521665
resetPQExpBuffer(str);
16531666

1654-
pg_log_info("creating publication \"%s\" in database \"%s\"",
1655-
dbinfo->pubname, dbinfo->dbname);
1667+
if (dry_run)
1668+
pg_log_info("dry-run: would create publication \"%s\" in database \"%s\"",
1669+
dbinfo->pubname, dbinfo->dbname);
1670+
else
1671+
pg_log_info("creating publication \"%s\" in database \"%s\"",
1672+
dbinfo->pubname, dbinfo->dbname);
16561673

16571674
appendPQExpBuffer(str, "CREATE PUBLICATION %s FOR ALL TABLES",
16581675
ipubname_esc);
@@ -1694,8 +1711,12 @@ drop_publication(PGconn *conn, const char *pubname, const char *dbname,
16941711

16951712
pubname_esc = PQescapeIdentifier(conn, pubname, strlen(pubname));
16961713

1697-
pg_log_info("dropping publication \"%s\" in database \"%s\"",
1698-
pubname, dbname);
1714+
if (dry_run)
1715+
pg_log_info("dry-run: would drop publication \"%s\" in database \"%s\"",
1716+
pubname, dbname);
1717+
else
1718+
pg_log_info("dropping publication \"%s\" in database \"%s\"",
1719+
pubname, dbname);
16991720

17001721
appendPQExpBuffer(str, "DROP PUBLICATION %s", pubname_esc);
17011722

@@ -1803,8 +1824,12 @@ create_subscription(PGconn *conn, const struct LogicalRepInfo *dbinfo)
18031824
pubconninfo_esc = PQescapeLiteral(conn, dbinfo->pubconninfo, strlen(dbinfo->pubconninfo));
18041825
replslotname_esc = PQescapeLiteral(conn, dbinfo->replslotname, strlen(dbinfo->replslotname));
18051826

1806-
pg_log_info("creating subscription \"%s\" in database \"%s\"",
1807-
dbinfo->subname, dbinfo->dbname);
1827+
if (dry_run)
1828+
pg_log_info("dry-run: would create subscription \"%s\" in database \"%s\"",
1829+
dbinfo->subname, dbinfo->dbname);
1830+
else
1831+
pg_log_info("creating subscription \"%s\" in database \"%s\"",
1832+
dbinfo->subname, dbinfo->dbname);
18081833

18091834
appendPQExpBuffer(str,
18101835
"CREATE SUBSCRIPTION %s CONNECTION %s PUBLICATION %s "
@@ -1901,8 +1926,12 @@ set_replication_progress(PGconn *conn, const struct LogicalRepInfo *dbinfo, cons
19011926
*/
19021927
originname = psprintf("pg_%u", suboid);
19031928

1904-
pg_log_info("setting the replication progress (node name \"%s\", LSN %s) in database \"%s\"",
1905-
originname, lsnstr, dbinfo->dbname);
1929+
if (dry_run)
1930+
pg_log_info("dry-run: would set the replication progress (node name \"%s\", LSN %s) in database \"%s\"",
1931+
originname, lsnstr, dbinfo->dbname);
1932+
else
1933+
pg_log_info("setting the replication progress (node name \"%s\", LSN %s) in database \"%s\"",
1934+
originname, lsnstr, dbinfo->dbname);
19061935

19071936
resetPQExpBuffer(str);
19081937
appendPQExpBuffer(str,
@@ -1947,8 +1976,12 @@ enable_subscription(PGconn *conn, const struct LogicalRepInfo *dbinfo)
19471976

19481977
subname = PQescapeIdentifier(conn, dbinfo->subname, strlen(dbinfo->subname));
19491978

1950-
pg_log_info("enabling subscription \"%s\" in database \"%s\"",
1951-
dbinfo->subname, dbinfo->dbname);
1979+
if (dry_run)
1980+
pg_log_info("dry-run: would enable subscription \"%s\" in database \"%s\"",
1981+
dbinfo->subname, dbinfo->dbname);
1982+
else
1983+
pg_log_info("enabling subscription \"%s\" in database \"%s\"",
1984+
dbinfo->subname, dbinfo->dbname);
19521985

19531986
appendPQExpBuffer(str, "ALTER SUBSCRIPTION %s ENABLE", subname);
19541987

‎src/bin/pg_basebackup/t/040_pg_createsubscriber.pl‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -436,11 +436,11 @@ sub generate_db
436436

437437
# Verify that the required logical replication objects are output.
438438
# The expected count 3 refers to postgres, $db1 and $db2 databases.
439-
is(scalar(() = $stderr =~ /creating publication/g),
439+
is(scalar(() = $stderr =~ /would create publication/g),
440440
3, "verify publications are created for all databases");
441-
is(scalar(() = $stderr =~ /creating the replication slot/g),
441+
is(scalar(() = $stderr =~ /would create the replication slot/g),
442442
3, "verify replication slots are created for all databases");
443-
is(scalar(() = $stderr =~ /creating subscription/g),
443+
is(scalar(() = $stderr =~ /would create subscription/g),
444444
3, "verify subscriptions are created for all databases");
445445

446446
# Run pg_createsubscriber on node S. --verbose is used twice

0 commit comments

Comments
 (0)