On Fri, Jan 10, 2025 at 8:28 PM Masahiko Sawada <sawada.m...@gmail.com> wrote:
>
> Hi,
>
> On Tue, Jan 7, 2025 at 11:30 PM Peter Smith <smithpb2...@gmail.com> wrote:
> >
> > Hi Sawada-San.
> >
> > FWIW, I also thought it was a good idea suggested by Bertrand [1] to
> > "hide" everything behind the slot create/delete, and thereby eliminate
> > the need for user intervention using those new
> > pg_activate/deactivate_logical_decoding functions.
> >
> > But, one concern doing it this way is how to prevent a user
> > (accidentally?) getting themselves into a different replication mode
> > without realising it? Or say they change the mode and then "forget"
> > that they did that.
>
> I think that as long as there is at least one logical replication slot
> users still have the will to execute logical decoding, so they require
> logical info WAL-logging. I think it would rather be good for users to
> be able to forget about that.
>
> > For example, If wal_level=replica and then somebody does CREATE
> > PUBLICATION/SUBSCRIPTION won't that implicitly enable the logical
> > decoding and then leave it enabled until all the logical replication
> > slots are eventually dropped?
>
> Yes.
>
> > Now, when the user examines wal_level it
> > is still going to show 'replica', which could be misleading --- e.g.
> > how will they even know that they can't really trust that anymore, and
> > they must also check the pg_get_logical_decoding_status?
>
> I think that if we automatically enable logical decoding even when
> wal_level=replica, we would not need pg_get_logical_decoding_status().
> These changes would break compatibility tools checking if wal_level is
> 'logical', but I guess that tools will not need to check that anymore.
> Users can simply understand that if there is at least one logical
> replication slot the system writes necessary information to use it.
> Having said that we anyway need to mention it in the doc and raising
> NOTICE/WARNING would also be a good idea as you mentioned.
>
Hi Sawada-San
IMO it seems misleading to be having logical replication behaviour
while the wal_level still displays as 'replica'.
So, I was wondering if it would be worthwhile to introduce a
'show_hook' for this GUC.
I hacked a quick implementation (attached) which now displays a middle
value for this scenario as "replica-logical". See below:
test_pub=# show wal_level;
wal_level
-----------
replica
(1 row)
test_pub=# select pg_get_logical_decoding_status();
pg_get_logical_decoding_status
--------------------------------
disabled
(1 row)
test_pub=# select pg_activate_logical_decoding();
pg_activate_logical_decoding
------------------------------
(1 row)
test_pub=# show wal_level;
wal_level
-----------------
replica-logical
(1 row)
test_pub=# select pg_get_logical_decoding_status();
pg_get_logical_decoding_status
--------------------------------
ready
(1 row)
test_pub=# \x
Expanded display is on.
test_pub=# select * from pg_settings where name in ('wal_level');
-[ RECORD 1 ]---+--------------------------------------------------
name | wal_level
setting | replica-logical
unit |
category | Write-Ahead Log / Settings
short_desc | Sets the level of information written to the WAL.
extra_desc |
context | postmaster
vartype | enum
source | configuration file
min_val |
max_val |
enumvals | {minimal,replica,logical}
boot_val | replica
reset_val | replica
sourcefile | /home/postgres/MYDATAOSS_2PC_PUB/postgresql.conf
sourceline | 199
pending_restart | f
~~
OTOH, you might prefer to display the "replica-logical" case as
"logical". (doing so might make things easier for existing tools that
are already checking if wal_level is "logical").
Of course, it doesn't really change the 'wal_level' variable value, it
just changes how it is *displayed*. Anyway, maybe it's an idea to
consider.
======
Kind Regards,
Peter Smith.
Fujitsu Australia
diff --git a/src/backend/replication/logical/logicalxlog.c
b/src/backend/replication/logical/logicalxlog.c
index d409eff..7defe97 100644
--- a/src/backend/replication/logical/logicalxlog.c
+++ b/src/backend/replication/logical/logicalxlog.c
@@ -27,6 +27,7 @@
#include "storage/procarray.h"
#include "storage/procsignal.h"
#include "utils/builtins.h"
+#include "utils/guc_hooks.h"
#include "utils/wait_event.h"
typedef struct XLogLogicalInfoCtlData
@@ -138,6 +139,23 @@ XLogLogicalInfoEnabled(void)
}
/*
+ * The show_hook for GUC wal_level.
+ */
+const char *show_wal_level(void)
+{
+ switch (wal_level)
+ {
+ case WAL_LEVEL_MINIMAL:
+ return "minimal";
+ case WAL_LEVEL_REPLICA:
+ return IsLogicalDecodingActive() ? "replica-logical" :
"replica";
+ case WAL_LEVEL_LOGICAL:
+ return "logical";
+ }
+ return "???";
+}
+
+/*
* Is logical decoding active?
*/
bool inline
diff --git a/src/backend/utils/misc/guc_tables.c
b/src/backend/utils/misc/guc_tables.c
index c9d8cd7..b7e2b9f 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -5068,7 +5068,7 @@ struct config_enum ConfigureNamesEnum[] =
},
&wal_level,
WAL_LEVEL_REPLICA, wal_level_options,
- NULL, NULL, NULL
+ NULL, NULL, show_wal_level
},
{
diff --git a/src/include/utils/guc_hooks.h b/src/include/utils/guc_hooks.h
index 8799921..53fa454 100644
--- a/src/include/utils/guc_hooks.h
+++ b/src/include/utils/guc_hooks.h
@@ -174,5 +174,5 @@ extern void assign_wal_sync_method(int new_wal_sync_method,
void *extra);
extern bool check_synchronized_standby_slots(char **newval, void **extra,
GucSource source);
extern void assign_synchronized_standby_slots(const char *newval, void *extra);
-
+extern const char *show_wal_level(void);
#endif /* GUC_HOOKS_H */
