Re: ps command does not show walsender's connected db

2022-11-23 Thread Michael Paquier
On Tue, Nov 22, 2022 at 08:46:22AM +0530, Bharath Rupireddy wrote:
> Are you looking at the latest v3 patch
> https://www.postgresql.org/message-id/4b5691462b994c18ff370aaa84cef0d0%40oss.nttdata.com?
> It has no printf() calls.

Yes, I was looking at v1.  v3 can be simpler.  All this information in
the Port comes from the startup packet, and the database name would
default to the user name so it can never be NULL.  And applied.
--
Michael


signature.asc
Description: PGP signature


Re: ps command does not show walsender's connected db

2022-11-21 Thread Bharath Rupireddy
On Tue, Nov 22, 2022 at 6:14 AM Michael Paquier  wrote:
>
> On Thu, Nov 17, 2022 at 01:32:11PM +0900, Ian Lawrence Barwick wrote:
> > Fujii-san is marked as committer on the commifest entry for this patch [1];
> > are you able to go ahead and get it committed?
>
> That's the state of the patch since the 11th of October.  Seeing the
> lack of activity, I propose to take care of that myself if there are
> no objections, only on HEAD.  That could be qualified as a bug, but
> that does not seem worth bothering the back-branches, either.  Except
> if somebody has a different opinion?

-1 for back-patching as it's not something critical to any of the
server's operations.

> The patch looks to do the job correctly, still I would leave the extra
> printf() calls in BackendStartup() out.

Are you looking at the latest v3 patch
https://www.postgresql.org/message-id/4b5691462b994c18ff370aaa84cef0d0%40oss.nttdata.com?
It has no printf() calls.

-- 
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com




Re: ps command does not show walsender's connected db

2022-11-21 Thread Michael Paquier
On Thu, Nov 17, 2022 at 01:32:11PM +0900, Ian Lawrence Barwick wrote:
> Fujii-san is marked as committer on the commifest entry for this patch [1];
> are you able to go ahead and get it committed?

That's the state of the patch since the 11th of October.  Seeing the
lack of activity, I propose to take care of that myself if there are
no objections, only on HEAD.  That could be qualified as a bug, but
that does not seem worth bothering the back-branches, either.  Except
if somebody has a different opinion?

The patch looks to do the job correctly, still I would leave the extra
printf() calls in BackendStartup() out.
--
Michael


signature.asc
Description: PGP signature


Re: ps command does not show walsender's connected db

2022-11-16 Thread Ian Lawrence Barwick
2022年10月11日(火) 20:06 Bharath Rupireddy :
>
> On Tue, Oct 11, 2022 at 2:11 PM bt22nakamorit
>  wrote:
> >
> > 2022-10-10 16:12 Bharath Rupireddy wrote:
> > > Thanks. LGTM.
> > Thank you for your review!
> > I have this issue posted on Commitfest 2022-11 with title "show
> > walsender's connected db for ps command entry".
> > May I change the status to "Ready for Committer"?
>
> Done - https://commitfest.postgresql.org/40/3937/.

Hi

Fujii-san is marked as committer on the commifest entry for this patch [1];
are you able to go ahead and get it committed?

[1] https://commitfest.postgresql.org/40/3937/

Thanks

Ian Barwick




Re: ps command does not show walsender's connected db

2022-10-11 Thread Bharath Rupireddy
On Tue, Oct 11, 2022 at 2:11 PM bt22nakamorit
 wrote:
>
> 2022-10-10 16:12 Bharath Rupireddy wrote:
> > Thanks. LGTM.
> Thank you for your review!
> I have this issue posted on Commitfest 2022-11 with title "show
> walsender's connected db for ps command entry".
> May I change the status to "Ready for Committer"?

Done - https://commitfest.postgresql.org/40/3937/.

-- 
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com




Re: ps command does not show walsender's connected db

2022-10-11 Thread bt22nakamorit

2022-10-10 16:12 Bharath Rupireddy wrote:

Thanks. LGTM.

Thank you for your review!
I have this issue posted on Commitfest 2022-11 with title "show 
walsender's connected db for ps command entry".

May I change the status to "Ready for Committer"?

Best,
Tatsuhiro Nakamori




Re: ps command does not show walsender's connected db

2022-10-10 Thread Bharath Rupireddy
On Mon, Oct 10, 2022 at 12:20 PM bt22nakamorit
 wrote:
>
> 2022-10-10 12:27 Bharath Rupireddy wrote:
> > if (port->database_name != NULL && port->database_name[0] != '\0')
> > appendStringInfo(_data, "%s ", port->database_name);
> >
> > The above works better.
>
> Thank you for the suggestion.
> I have edited the patch to reflect your idea.

Thanks. LGTM.

-- 
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com




Re: ps command does not show walsender's connected db

2022-10-10 Thread bt22nakamorit

2022-10-10 12:27 Bharath Rupireddy wrote:

if (port->database_name != NULL && port->database_name[0] != '\0')
appendStringInfo(_data, "%s ", port->database_name);

The above works better.


Thank you for the suggestion.
I have edited the patch to reflect your idea.

Best,
Tatsuhiro Nakamoridiff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 383bc4776e..e17b62e155 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -4451,7 +4451,7 @@ BackendInitialize(Port *port)
 	if (am_walsender)
 		appendStringInfo(_data, "%s ", GetBackendTypeDesc(B_WAL_SENDER));
 	appendStringInfo(_data, "%s ", port->user_name);
-	if (!am_walsender)
+	if (port->database_name != NULL && port->database_name[0] != '\0')
 		appendStringInfo(_data, "%s ", port->database_name);
 	appendStringInfoString(_data, port->remote_host);
 	if (port->remote_port[0] != '\0')


Re: ps command does not show walsender's connected db

2022-10-09 Thread Bharath Rupireddy
On Mon, Oct 10, 2022 at 8:00 AM bt22nakamorit
 wrote:
>
> appendStringInfo will append extra space after null (since "%s "), so
> the ps entry will look less neat in that case.
> How about we check whether port->database_name is null or not, instead
> of making it unconditional?
> It will look like this.
> -if (!am_walsender)
> +if (port->database_name != NULL)
>  appendStringInfo(_data, "%s ", port->database_name);

if (port->database_name != NULL && port->database_name[0] != '\0')
appendStringInfo(_data, "%s ", port->database_name);

The above works better.

-- 
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com




Re: ps command does not show walsender's connected db

2022-10-09 Thread bt22nakamorit

2022-10-09 18:30 Bharath Rupireddy wrote:

-if (!am_walsender)
+if (!am_walsender || am_db_walsender)
 appendStringInfo(_data, "%s ", port->database_name);

Can the appendStringInfo be just unconditional? That is more readable
IMO. We want the database_name to be appended whenever it isn't null.

I agree that the patch makes the code less neat.


The only case we expect database_name to be null is for walsenders
serving streaming replication standbys and even when database_name is
null, nothing gets appended, it's just the appendStringInfo() call
gets wasted, but that's okay.
appendStringInfo will append extra space after null (since "%s "), so 
the ps entry will look less neat in that case.
How about we check whether port->database_name is null or not, instead 
of making it unconditional?

It will look like this.
-if (!am_walsender)
+if (port->database_name != NULL)
appendStringInfo(_data, "%s ", port->database_name);

Thank you for your response,
Tatsuhiro Nakamori




Re: ps command does not show walsender's connected db

2022-10-09 Thread Bharath Rupireddy
On Fri, Oct 7, 2022 at 7:25 PM Fujii Masao  wrote:
>
> Thanks for updating the patch! LGTM.

-if (!am_walsender)
+if (!am_walsender || am_db_walsender)
 appendStringInfo(_data, "%s ", port->database_name);

Can the appendStringInfo be just unconditional? That is more readable
IMO. We want the database_name to be appended whenever it isn't null.
The only case we expect database_name to be null is for walsenders
serving streaming replication standbys and even when database_name is
null, nothing gets appended, it's just the appendStringInfo() call
gets wasted, but that's okay.

-- 
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com




Re: ps command does not show walsender's connected db

2022-10-07 Thread Fujii Masao




On 2022/10/07 18:43, bt22nakamorit wrote:

2022-10-07 16:59  Fujii Masao wrote:


s/subscriber/publisher ?


I did not understand what this means.


Sorry for confusing wording... I was just trying to say that walsender
is connected to a database of the publisher instead of subscriber.



Thanks for the patch!

-
+    printf("fork child process\n");
+    printf("    am_walsender: %d\n", am_walsender);
+    printf("    am_db_walsender: %d\n", am_db_walsender);

The patch seems to include the debug code accidentally.
Except this, the patch looks good to me.


I apologize for the silly mistake.
I edited the patch and attached it to this mail.


Thanks for updating the patch! LGTM.

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION




Re: ps command does not show walsender's connected db

2022-10-07 Thread bt22nakamorit

2022-10-07 16:59  Fujii Masao wrote:


s/subscriber/publisher ?


I did not understand what this means.


Thanks for the patch!

-
+   printf("fork child process\n");
+   printf("   am_walsender: %d\n", am_walsender);
+   printf("   am_db_walsender: %d\n", am_db_walsender);

The patch seems to include the debug code accidentally.
Except this, the patch looks good to me.


I apologize for the silly mistake.
I edited the patch and attached it to this mail.

Best,
Tatsuhiro Nakamoridiff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 383bc4776e..e17b62e155 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -4451,7 +4451,7 @@ BackendInitialize(Port *port)
 	if (am_walsender)
 		appendStringInfo(_data, "%s ", GetBackendTypeDesc(B_WAL_SENDER));
 	appendStringInfo(_data, "%s ", port->user_name);
-	if (!am_walsender)
+	if (!am_walsender || am_db_walsender)
 		appendStringInfo(_data, "%s ", port->database_name);
 	appendStringInfoString(_data, port->remote_host);
 	if (port->remote_port[0] != '\0')


Re: ps command does not show walsender's connected db

2022-10-07 Thread Fujii Masao




On 2022/10/06 22:30, bt22nakamorit wrote:

Hi,

When walsender process is evoked for logical replication, walsender is 
connected to a database of the subscriber.
Naturally, ones would want the name of the connected database to show in the 
entry of ps command for walsender.
In detail, running ps aux during the logical replication shows results like the 
following:
postgres=# \! ps aux | grep postgres;
...
ACTC-I\+ 14575  0.0  0.0 298620 14228 ?    Ss   18:22   0:00 postgres: 
walsender ACTC-I\nakamorit [local] S

However, since walsender is connected to a database of the subscriber in 
logical replication,


s/subscriber/publisher ?



it should show the database name, as in the following:
postgres=# \! ps aux | grep postgres
...
ACTC-I\+ 15627  0.0  0.0 298624 13936 ?    Ss   15:45   0:00 postgres: 
walsender ACTC-I\nakamorit postgres

Showing the database name should not apply in streaming replication though 
since walsender process is not connected to any specific database.

The attached patch adds the name of the connected database to the ps entry of 
walsender in logical replication, and not in streaming replication.

Thoughts?


+1

Thanks for the patch!

-
+   printf("fork child process\n");
+   printf("   am_walsender: %d\n", am_walsender);
+   printf("   am_db_walsender: %d\n", am_db_walsender);

The patch seems to include the debug code accidentally.
Except this, the patch looks good to me.

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION




ps command does not show walsender's connected db

2022-10-06 Thread bt22nakamorit

Hi,

When walsender process is evoked for logical replication, walsender is 
connected to a database of the subscriber.
Naturally, ones would want the name of the connected database to show in 
the entry of ps command for walsender.
In detail, running ps aux during the logical replication shows results 
like the following:

postgres=# \! ps aux | grep postgres;
...
ACTC-I\+ 14575  0.0  0.0 298620 14228 ?Ss   18:22   0:00 
postgres: walsender ACTC-I\nakamorit [local] S


However, since walsender is connected to a database of the subscriber in 
logical replication, it should show the database name, as in the 
following:

postgres=# \! ps aux | grep postgres
...
ACTC-I\+ 15627  0.0  0.0 298624 13936 ?Ss   15:45   0:00 
postgres: walsender ACTC-I\nakamorit postgres


Showing the database name should not apply in streaming replication 
though since walsender process is not connected to any specific 
database.


The attached patch adds the name of the connected database to the ps 
entry of walsender in logical replication, and not in streaming 
replication.


Thoughts?

Tatsuhiro Nakamoridiff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 383bc4776e..6cbb69767c 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -4187,7 +4187,9 @@ BackendStartup(Port *port)
 	if (pid == 0)/* child */
 	{
 		free(bn);
-
+		printf("fork child process\n");
+		printf("	am_walsender: %d\n", am_walsender);
+		printf("	am_db_walsender: %d\n", am_db_walsender);
 		/* Detangle from postmaster */
 		InitPostmasterChild();
 
@@ -4451,7 +4453,7 @@ BackendInitialize(Port *port)
 	if (am_walsender)
 		appendStringInfo(_data, "%s ", GetBackendTypeDesc(B_WAL_SENDER));
 	appendStringInfo(_data, "%s ", port->user_name);
-	if (!am_walsender)
+	if (!am_walsender || am_db_walsender)
 		appendStringInfo(_data, "%s ", port->database_name);
 	appendStringInfoString(_data, port->remote_host);
 	if (port->remote_port[0] != '\0')