Re: Injection Points remaining stats
On 8/18/24 20:09, Michael Paquier wrote: f68cd847fa40 but I've just lacked a combination of time and energy while the original commit was already enough. The code indentation was a bit incorrect, and I think that we should also have tests to stress that the insertion of the new stats is correct. I have fixed the indentation, added some tests and improved a couple of surrounding descriptions while on it. Thank you for committing. I was thinking to add such test in next patch set. I have a updated .vimrc to have correct indentation. I'm tempted to propose a separate improvement for the template of the fixed-numbered stats. We could do like pgstatfuncs.c where we use a macro to define the routines of the counters, and have one function for each counter incremented. That's a separate refactoring, so I'll propose that on a different thread. I will take a look on this. -- Kind Regards, Yogesh Sharma PostgreSQL, Linux, and Networking Open Source Enthusiast and Advocate PostgreSQL Contributors Team @ RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Injection Points remaining stats
Hi all,
Attaching a patch to add remaining cached and loaded stats as mentioned
in commit f68cd847fa40ead44a786b9c34aff9ccc048004b message. Existing TAP
tests were updated to handle new stats. This patch has been tested on
HEAD using "make check-world" after enabling injection points via
"--enable-injection-points".
--
Kind Regards,
Yogesh SharmaFrom 52ac2b14ee34d606aae97b2648c6708901c324b4 Mon Sep 17 00:00:00 2001
From: Yogesh Sharma
Date: Wed, 7 Aug 2024 15:32:35 -0400
Subject: [PATCH] This patch adds missing cached and loaded stats to
injection_points_stats_fixed as mentioned in commit #f68cd847fa
---
.../injection_points--1.0.sql | 4 +++-
.../injection_points/injection_points.c | 8 ---
.../injection_points/injection_stats.h| 4 +++-
.../injection_points/injection_stats_fixed.c | 24 +++
.../modules/injection_points/t/001_stats.pl | 6 ++---
5 files changed, 34 insertions(+), 12 deletions(-)
diff --git a/src/test/modules/injection_points/injection_points--1.0.sql b/src/test/modules/injection_points/injection_points--1.0.sql
index 1b2a4938a9..6c81d55e0d 100644
--- a/src/test/modules/injection_points/injection_points--1.0.sql
+++ b/src/test/modules/injection_points/injection_points--1.0.sql
@@ -91,7 +91,9 @@ LANGUAGE C STRICT;
-- Reports fixed-numbered statistics for injection points.
CREATE FUNCTION injection_points_stats_fixed(OUT numattach int8,
OUT numdetach int8,
- OUT numrun int8)
+ OUT numrun int8,
+ OUT numcached int8,
+ OUT numloaded int8)
RETURNS record
AS 'MODULE_PATHNAME', 'injection_points_stats_fixed'
LANGUAGE C STRICT;
diff --git a/src/test/modules/injection_points/injection_points.c b/src/test/modules/injection_points/injection_points.c
index dc02be1bbf..221b826d8d 100644
--- a/src/test/modules/injection_points/injection_points.c
+++ b/src/test/modules/injection_points/injection_points.c
@@ -297,7 +297,7 @@ injection_points_attach(PG_FUNCTION_ARGS)
condition.pid = MyProcPid;
}
- pgstat_report_inj_fixed(1, 0, 0);
+ pgstat_report_inj_fixed(1, 0, 0, 0, 0);
InjectionPointAttach(name, "injection_points", function, &condition,
sizeof(InjectionPointCondition));
@@ -329,6 +329,7 @@ injection_points_load(PG_FUNCTION_ARGS)
if (inj_state == NULL)
injection_init_shmem();
+ pgstat_report_inj_fixed(0, 0, 0, 0, 1);
INJECTION_POINT_LOAD(name);
PG_RETURN_VOID();
@@ -343,7 +344,7 @@ injection_points_run(PG_FUNCTION_ARGS)
{
char *name = text_to_cstring(PG_GETARG_TEXT_PP(0));
- pgstat_report_inj_fixed(0, 0, 1);
+ pgstat_report_inj_fixed(0, 0, 1, 0, 0);
INJECTION_POINT(name);
PG_RETURN_VOID();
@@ -358,6 +359,7 @@ injection_points_cached(PG_FUNCTION_ARGS)
{
char *name = text_to_cstring(PG_GETARG_TEXT_PP(0));
+ pgstat_report_inj_fixed(0, 0, 0, 1, 0);
INJECTION_POINT_CACHED(name);
PG_RETURN_VOID();
@@ -434,7 +436,7 @@ injection_points_detach(PG_FUNCTION_ARGS)
{
char *name = text_to_cstring(PG_GETARG_TEXT_PP(0));
- pgstat_report_inj_fixed(0, 1, 0);
+ pgstat_report_inj_fixed(0, 1, 0, 0, 0);
if (!InjectionPointDetach(name))
elog(ERROR, "could not detach injection point \"%s\"", name);
diff --git a/src/test/modules/injection_points/injection_stats.h b/src/test/modules/injection_points/injection_stats.h
index d519f29f83..126c110169 100644
--- a/src/test/modules/injection_points/injection_stats.h
+++ b/src/test/modules/injection_points/injection_stats.h
@@ -25,6 +25,8 @@ extern void pgstat_report_inj(const char *name);
extern void pgstat_register_inj_fixed(void);
extern void pgstat_report_inj_fixed(uint32 numattach,
uint32 numdetach,
- uint32 numrun);
+ uint32 numrun,
+ uint32 numcached,
+ uint32 numloaded);
#endif
diff --git a/src/test/modules/injection_points/injection_stats_fixed.c b/src/test/modules/injection_points/injection_stats_fixed.c
index 72a5f9decb..e01da5c688 100644
--- a/src/test/modules/injection_points/injection_stats_fixed.c
+++ b/src/test/modules/injection_points/injection_stats_fixed.c
@@ -29,6 +29,8 @@ typedef struct PgStat_StatInjFixedEntry
PgStat_Counter numattach; /* number of points attached */
PgStat_Counter numdetach; /* number of points detached */
PgStat_Counter numrun; /* number of points run */
+ PgStat_Counter numcached; /* number of points cached */
+ PgStat_Counter numloaded; /* number of points loaded */
TimestampTz stat_reset_timestamp;
} PgStat_StatInjFixedEntry;
@@ -114,6 +116,8 @@ injection_stats_fixed_snapshot_cb(void)
FIXED_COMP(numattach);
FIXED_COMP(numdetach);
FIXED_COMP(numrun);
+ FIXED_COMP(numcached);
+ FIXED_COMP(numloaded);
#undef FIXED_COMP
}
@@ -135,7 +139,9 @@ pgstat_register_inj_fixed(void)
void
pgstat_report_inj_fixed(uint32 numattach,
uint32 numdetach,
- uint32 numrun)
+ uint32 numrun,
+ uint32 numcached,
+ uint32 numload
Re: ODBC Source Downloads Missing
On 6/10/24 15:33, Mark Hill wrote: Is there an issue with the ODBC Source downloads today? The source download URL isn’t working: https://www.postgresql.org/ftp/odbc/versions/src/ Thanks, Mark Here is github repo https://github.com/postgresql-interfaces/psqlodbc -- Kind Regards, Yogesh Sharma PostgreSQL, Linux, and Networking Expert Open Source Enthusiast and Advocate PostgreSQL Contributors Team @ RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: Logical Replication of sequences
On 6/4/24 06:57, Amit Kapila wrote: 1. Provide a tool to copy all the sequences from publisher to subscriber. The major drawback is that users need to perform this as an additional step during the upgrade which would be inconvenient and probably not as useful as some built-in mechanism. Agree, this requires additional steps. Not a preferred approach in my opinion. When a large set of sequences are present, it will add additional downtime for upgrade process. 2. Provide a command say Alter Subscription ... Replicate Sequences (or something like that) which users can perform before shutdown of the publisher node during upgrade. This will allow copying all the sequences from the publisher node to the subscriber node directly. Similar to previous approach, this could also be inconvenient for users. This is similar to option 1 except that it is a SQL command now. Still not a preferred approach in my opinion. When a large set of sequences are present, it will add additional downtime for upgrade process. 3. Replicate published sequences via walsender at the time of shutdown or incrementally while decoding checkpoint record. The two ways to achieve this are: (a) WAL log a special NOOP record just before shutting down checkpointer. Then allow the WALsender to read the sequence data and send it to the subscriber while decoding the new NOOP record. (b) Similar to the previous idea but instead of WAL logging a new record directly invokes a decoding callback after walsender receives a request to shutdown which will allow pgoutput to read and send required sequences. This approach has a drawback that we are adding more work at the time of shutdown but note that we already waits for all the WAL records to be decoded and sent before shutting down the walsender during shutdown of the node. At the time of shutdown a) most logical upgrades don't necessarily call for shutdown b) it will still add to total downtime with large set of sequences. Incremental option is better as it will not require a shutdown. I do see a scenario where sequence of events can lead to loss of sequence and generate duplicate sequence values, if subscriber starts consuming sequences while publisher is also consuming them. In such cases, subscriber shall not be allowed sequence consumption. -- Kind Regards, Yogesh Sharma Open Source Enthusiast and Advocate PostgreSQL Contributors Team @ RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: Have better wording for snapshot file reading failure
On 9/14/23 20:33, Andres Freund wrote: I'd probably just go for something like "snapshot \"%s\" does not exist", similar to what we report for unknown tables etc. Arguably changing the errcode to ERRCODE_UNDEFINED_OBJECT would make this more precise? +1 better informative message compare to the original patch. -- Kind Regards, Yogesh Sharma PostgreSQL, Linux, and Networking Expert Open Source Enthusiast and Advocate
Re: Have better wording for snapshot file reading failure
On 9/13/23 02:10, Bharath Rupireddy wrote:
Hi,
When a snapshot file reading fails in ImportSnapshot(), it errors out
with "invalid snapshot identifier". This message better suits for
snapshot identifier parsing errors which is being done just before the
file reading. The attached patch adds a generic file reading error
message with path to help distinguish if the issue is with snapshot
identifier parsing or file reading.
I suggest error message to include "snapshot" keyword in message, like this:
errmsg("could not open snapshot file \"%s\" for reading: %m",
and also tweak other messages accordingly.
--
Kind Regards,
Yogesh Sharma
PostgreSQL, Linux, and Networking Expert
Open Source Enthusiast and Advocate
