Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 801792e

Browse files
author
Amit Kapila
committed
Improve ERROR/LOG messages added by commits ddd5f4f and 7a424ec.
Additionally, in slotsync.c, replace one StringInfoData variable usage with a constant string to avoid palloc/pfree. Also, replace the inclusion of "logical.h" with "slot.h" to prevent the exposure of unnecessary implementation details. Reported-by: Kyotaro Horiguchi, Masahiko Sawada Author: Shveta Malik based on suggestions by Robert Haas and Amit Kapila Reviewed-by: Kyotaro Horiguchi, Amit Kapila Discussion: https://postgr.es/m/20240214.162652.773291409747353211.horikyota.ntt@gmail.com Discussion: https://postgr.es/m/20240219.134015.1888940527023074780.horikyota.ntt@gmail.com Discussion: https://postgr.es/m/CAD21AoCYXhDYOQDAS-rhGasC2T+tYbV=8Y18o94sB=5AxcW+yA@mail.gmail.com
1 parent 011d60c commit 801792e

File tree

2 files changed

+28
-39
lines changed

2 files changed

+28
-39
lines changed

src/backend/replication/logical/slotsync.c

+27-38
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@
3535
#include "access/xlogrecovery.h"
3636
#include "catalog/pg_database.h"
3737
#include "commands/dbcommands.h"
38-
#include "replication/logical.h"
38+
#include "replication/slot.h"
3939
#include "replication/slotsync.h"
4040
#include "storage/ipc.h"
4141
#include "storage/lmgr.h"
@@ -368,14 +368,13 @@ update_and_persist_local_synced_slot(RemoteSlot *remote_slot, Oid remote_dbid)
368368
* XXX should this be changed to elog(DEBUG1) perhaps?
369369
*/
370370
ereport(LOG,
371-
errmsg("could not sync slot information as remote slot precedes local slot:"
372-
" remote slot \"%s\": LSN (%X/%X), catalog xmin (%u) local slot: LSN (%X/%X), catalog xmin (%u)",
373-
remote_slot->name,
374-
LSN_FORMAT_ARGS(remote_slot->restart_lsn),
375-
remote_slot->catalog_xmin,
376-
LSN_FORMAT_ARGS(slot->data.restart_lsn),
377-
slot->data.catalog_xmin));
378-
371+
errmsg("could not sync slot \"%s\" as remote slot precedes local slot",
372+
remote_slot->name),
373+
errdetail("Remote slot has LSN %X/%X and catalog xmin %u, but local slot has LSN %X/%X and catalog xmin %u.",
374+
LSN_FORMAT_ARGS(remote_slot->restart_lsn),
375+
remote_slot->catalog_xmin,
376+
LSN_FORMAT_ARGS(slot->data.restart_lsn),
377+
slot->data.catalog_xmin));
379378
return;
380379
}
381380

@@ -569,8 +568,12 @@ synchronize_slots(WalReceiverConn *wrconn)
569568

570569
WalRcvExecResult *res;
571570
TupleTableSlot *tupslot;
572-
StringInfoData s;
573571
List *remote_slot_list = NIL;
572+
const char *query = "SELECT slot_name, plugin, confirmed_flush_lsn,"
573+
" restart_lsn, catalog_xmin, two_phase, failover,"
574+
" database, conflict_reason"
575+
" FROM pg_catalog.pg_replication_slots"
576+
" WHERE failover and NOT temporary";
574577

575578
SpinLockAcquire(&SlotSyncCtx->mutex);
576579
if (SlotSyncCtx->syncing)
@@ -586,19 +589,8 @@ synchronize_slots(WalReceiverConn *wrconn)
586589

587590
syncing_slots = true;
588591

589-
initStringInfo(&s);
590-
591-
/* Construct query to fetch slots with failover enabled. */
592-
appendStringInfo(&s,
593-
"SELECT slot_name, plugin, confirmed_flush_lsn,"
594-
" restart_lsn, catalog_xmin, two_phase, failover,"
595-
" database, conflict_reason"
596-
" FROM pg_catalog.pg_replication_slots"
597-
" WHERE failover and NOT temporary");
598-
599592
/* Execute the query */
600-
res = walrcv_exec(wrconn, s.data, SLOTSYNC_COLUMN_COUNT, slotRow);
601-
pfree(s.data);
593+
res = walrcv_exec(wrconn, query, SLOTSYNC_COLUMN_COUNT, slotRow);
602594

603595
if (res->status != WALRCV_OK_TUPLES)
604596
ereport(ERROR,
@@ -743,12 +735,12 @@ validate_remote_info(WalReceiverConn *wrconn)
743735
ereport(ERROR,
744736
errmsg("could not fetch primary_slot_name \"%s\" info from the primary server: %s",
745737
PrimarySlotName, res->err),
746-
errhint("Check if \"primary_slot_name\" is configured correctly."));
738+
errhint("Check if primary_slot_name is configured correctly."));
747739

748740
tupslot = MakeSingleTupleTableSlot(res->tupledesc, &TTSOpsMinimalTuple);
749741
if (!tuplestore_gettupleslot(res->tuplestore, true, false, tupslot))
750742
elog(ERROR,
751-
"failed to fetch tuple for the primary server slot specified by \"primary_slot_name\"");
743+
"failed to fetch tuple for the primary server slot specified by primary_slot_name");
752744

753745
remote_in_recovery = DatumGetBool(slot_getattr(tupslot, 1, &isnull));
754746
Assert(!isnull);
@@ -764,9 +756,9 @@ validate_remote_info(WalReceiverConn *wrconn)
764756
if (!primary_slot_valid)
765757
ereport(ERROR,
766758
errcode(ERRCODE_INVALID_PARAMETER_VALUE),
767-
errmsg("bad configuration for slot synchronization"),
759+
errmsg("slot synchronization requires valid primary_slot_name"),
768760
/* translator: second %s is a GUC variable name */
769-
errdetail("The replication slot \"%s\" specified by \"%s\" does not exist on the primary server.",
761+
errdetail("The replication slot \"%s\" specified by %s does not exist on the primary server.",
770762
PrimarySlotName, "primary_slot_name"));
771763

772764
ExecClearTuple(tupslot);
@@ -792,8 +784,7 @@ ValidateSlotSyncParams(void)
792784
ereport(ERROR,
793785
/* translator: %s is a GUC variable name */
794786
errcode(ERRCODE_INVALID_PARAMETER_VALUE),
795-
errmsg("bad configuration for slot synchronization"),
796-
errhint("\"%s\" must be defined.", "primary_slot_name"));
787+
errmsg("slot synchronization requires %s to be defined", "primary_slot_name"));
797788

798789
/*
799790
* hot_standby_feedback must be enabled to cooperate with the physical
@@ -804,15 +795,14 @@ ValidateSlotSyncParams(void)
804795
ereport(ERROR,
805796
/* translator: %s is a GUC variable name */
806797
errcode(ERRCODE_INVALID_PARAMETER_VALUE),
807-
errmsg("bad configuration for slot synchronization"),
808-
errhint("\"%s\" must be enabled.", "hot_standby_feedback"));
798+
errmsg("slot synchronization requires %s to be enabled",
799+
"hot_standby_feedback"));
809800

810801
/* Logical slot sync/creation requires wal_level >= logical. */
811802
if (wal_level < WAL_LEVEL_LOGICAL)
812803
ereport(ERROR,
813804
errcode(ERRCODE_INVALID_PARAMETER_VALUE),
814-
errmsg("bad configuration for slot synchronization"),
815-
errhint("\"wal_level\" must be >= logical."));
805+
errmsg("slot synchronization requires wal_level >= \"logical\""));
816806

817807
/*
818808
* The primary_conninfo is required to make connection to primary for
@@ -822,8 +812,8 @@ ValidateSlotSyncParams(void)
822812
ereport(ERROR,
823813
/* translator: %s is a GUC variable name */
824814
errcode(ERRCODE_INVALID_PARAMETER_VALUE),
825-
errmsg("bad configuration for slot synchronization"),
826-
errhint("\"%s\" must be defined.", "primary_conninfo"));
815+
errmsg("slot synchronization requires %s to be defined",
816+
"primary_conninfo"));
827817

828818
/*
829819
* The slot synchronization needs a database connection for walrcv_exec to
@@ -834,12 +824,11 @@ ValidateSlotSyncParams(void)
834824
ereport(ERROR,
835825

836826
/*
837-
* translator: 'dbname' is a specific option; %s is a GUC variable
838-
* name
827+
* translator: dbname is a specific option; %s is a GUC variable name
839828
*/
840829
errcode(ERRCODE_INVALID_PARAMETER_VALUE),
841-
errmsg("bad configuration for slot synchronization"),
842-
errhint("'dbname' must be specified in \"%s\".", "primary_conninfo"));
830+
errmsg("slot synchronization requires dbname to be specified in %s",
831+
"primary_conninfo"));
843832
}
844833

845834
/*

src/test/recovery/t/040_standby_failover_slots_sync.pl

+1-1
Original file line numberDiff line numberDiff line change
@@ -319,7 +319,7 @@
319319
($result, $stdout, $stderr) =
320320
$standby1->psql('postgres', "SELECT pg_sync_replication_slots();");
321321
ok( $stderr =~
322-
/HINT: 'dbname' must be specified in "primary_conninfo"/,
322+
/ERROR: slot synchronization requires dbname to be specified in primary_conninfo/,
323323
"cannot sync slots if dbname is not specified in primary_conninfo");
324324

325325
##################################################

0 commit comments

Comments
 (0)