Re: [HACKERS] external_pid_file not removed on postmaster exit

2012-08-16 Thread Peter Eisentraut
On Fri, 2012-07-27 at 08:09 +0300, Peter Eisentraut wrote:
> It seems strange that the external_pid_file is never removed.  There is
> even a C comment about it:
> 
> /* Should we remove the pid file on postmaster exit? */
> 
> I think it should be removed with proc_exit hook just like the main
> postmaster.pid file.
> 
> Does anyone remember why this was not done originally or have any
> concerns?

Since that was not the case, I propose the attached patch to unlink the
external pid file.
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index a4fb2a4..73520a6 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -329,6 +329,7 @@
 /*
  * postmaster.c - function prototypes
  */
+static void unlink_external_pid_file(int status, Datum arg);
 static void getInstallationPaths(const char *argv0);
 static void checkDataDir(void);
 static Port *ConnCreate(int serverFd);
@@ -1071,7 +1072,6 @@ static bool save_backend_variables(BackendParameters *param, Port *port,
 		{
 			fprintf(fpidfile, "%d\n", MyProcPid);
 			fclose(fpidfile);
-			/* Should we remove the pid file on postmaster exit? */
 
 			/* Make PID file world readable */
 			if (chmod(external_pid_file, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH) != 0)
@@ -1081,6 +1081,8 @@ static bool save_backend_variables(BackendParameters *param, Port *port,
 		else
 			write_stderr("%s: could not write external PID file \"%s\": %s\n",
 		 progname, external_pid_file, strerror(errno));
+
+		on_proc_exit(unlink_external_pid_file, 0);
 	}
 
 	/*
@@ -1183,6 +1185,17 @@ static bool save_backend_variables(BackendParameters *param, Port *port,
 
 
 /*
+ * on_proc_exit callback to delete external_pid_file
+ */
+static void
+unlink_external_pid_file(int status, Datum arg)
+{
+	if (external_pid_file)
+		unlink(external_pid_file);
+}
+
+
+/*
  * Compute and check the directory paths to files that are part of the
  * installation (as deduced from the postgres executable's own location)
  */

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] external_pid_file not removed on postmaster exit

2012-07-28 Thread Amit kapila
> From: Tom Lane [t...@sss.pgh.pa.us]
> Sent: Saturday, July 28, 2012 9:46 PM
Amit kapila  writes:
>>> I think it should be removed with proc_exit hook just like the main
>>> postmaster.pid file.

>> external_pid_file is created first time when it is enabled in postgresql.conf
>> I think it should be removed once the parameter external_pid_file is unset;

> Unset?  
By Unset, I mean to say when the configuration parameter 'external_pid_file' is 
disabled (#external_pid_file).
But if the path/filename is changed to different name across restart of server, 
it will not be able to delete the previous file. 
So it will not workout the way I was trying to think.

However if it is deleted at proc_exit as suggested by Peter, there will be no 
problem.

The reason why I have thought the file not to get deleted at every proc_exit, 
is 
a. Initially I thought it might be un-necessary to delete and re-create the 
file at server shutdown and start.
b. I was not sure if this file has usage only till server is running.


With Regards,
Amit Kapila.

  

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] external_pid_file not removed on postmaster exit

2012-07-28 Thread Tom Lane
Amit kapila  writes:
>> I think it should be removed with proc_exit hook just like the main
>> postmaster.pid file.

> external_pid_file is created first time when it is enabled in postgresql.conf
> I think it should be removed once the parameter external_pid_file is unset; 

Unset?  If that parameter is not PGC_POSTMASTER, it certainly ought to
be.  In any case, that has little to do with what Peter is complaining
about ...

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] external_pid_file not removed on postmaster exit

2012-07-28 Thread Amit kapila
>From: pgsql-hackers-ow...@postgresql.org [pgsql-hackers-ow...@postgresql.org] 
>on behalf of Peter Eisentraut [pete...@gmx.net]
> Sent: Friday, July 27, 2012 10:39 AM

> It seems strange that the external_pid_file is never removed.  There is
> even a C comment about it:

> /* Should we remove the pid file on postmaster exit? */

> I think it should be removed with proc_exit hook just like the main
> postmaster.pid file.

external_pid_file is created first time when it is enabled in postgresql.conf
I think it should be removed once the parameter external_pid_file is unset; 

Making handling of both postmaster.pid and external_pid_file same in terms of 
creation and removal may not be best choice
as both have different purpose.



With Regards,
Amit Kapila.
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


[HACKERS] external_pid_file not removed on postmaster exit

2012-07-26 Thread Peter Eisentraut
It seems strange that the external_pid_file is never removed.  There is
even a C comment about it:

/* Should we remove the pid file on postmaster exit? */

I think it should be removed with proc_exit hook just like the main
postmaster.pid file.

Does anyone remember why this was not done originally or have any
concerns?



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers