On Fri, Mar  7, 2014 at 07:18:04PM +0530, Amit Kapila wrote:
> > OK, done with the attached patch  Three is returned for status, but one
> > for other cases.
> 
> I think it would have been better if it return status as 4 for cases where
> directory/file is not accessible (current new cases added by this patch).
> 
> I think status 3 should be only return only when the data directory is valid
> and pid file is missing, because in script after getting this code, the next
> thing they will probably do is to start the server which doesn't seem a
> good fit for newly added cases.

OK, I have updated the attached patch to reflect this, and added a C
comment.

-- 
  Bruce Momjian  <br...@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + Everyone has their own god. +
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
new file mode 100644
index 0dbaa6e..56d238f
*** a/src/bin/pg_ctl/pg_ctl.c
--- b/src/bin/pg_ctl/pg_ctl.c
*************** static bool allow_core_files = false;
*** 97,102 ****
--- 97,103 ----
  static time_t start_time;
  
  static char postopts_file[MAXPGPATH];
+ static char version_file[MAXPGPATH];
  static char pid_file[MAXPGPATH];
  static char backup_file[MAXPGPATH];
  static char recovery_file[MAXPGPATH];
*************** static void pgwin32_doRunAsService(void)
*** 152,158 ****
  static int	CreateRestrictedProcess(char *cmd, PROCESS_INFORMATION *processInfo, bool as_service);
  #endif
  
! static pgpid_t get_pgpid(void);
  static char **readfile(const char *path);
  static void free_readfile(char **optlines);
  static int	start_postmaster(void);
--- 153,159 ----
  static int	CreateRestrictedProcess(char *cmd, PROCESS_INFORMATION *processInfo, bool as_service);
  #endif
  
! static pgpid_t get_pgpid(bool is_status_request);
  static char **readfile(const char *path);
  static void free_readfile(char **optlines);
  static int	start_postmaster(void);
*************** print_msg(const char *msg)
*** 246,255 ****
  }
  
  static pgpid_t
! get_pgpid(void)
  {
  	FILE	   *pidf;
  	long		pid;
  
  	pidf = fopen(pid_file, "r");
  	if (pidf == NULL)
--- 247,280 ----
  }
  
  static pgpid_t
! get_pgpid(bool is_status_request)
  {
  	FILE	   *pidf;
  	long		pid;
+ 	struct stat statbuf;
+ 
+ 	if (stat(pg_data, &statbuf) != 0)
+ 	{
+ 		if (errno == ENOENT)
+ 			printf(_("%s: directory \"%s\" does not exist\n"), progname,
+ 					 pg_data);
+ 		else
+ 			printf(_("%s: cannot access directory \"%s\"\n"), progname,
+ 					 pg_data);
+ 		/*
+ 		 * The Linux Standard Base Core Specification 3.1 says this should return
+ 		 * '4, program or service status is unknown'
+ 		 * https://refspecs.linuxbase.org/LSB_3.1.0/LSB-Core-generic/LSB-Core-generic/iniscrptact.html
+ 		 */
+ 		exit(is_status_request ? 4 : 1);
+ 	}
+ 
+ 	if (stat(version_file, &statbuf) != 0 && errno == ENOENT)
+ 	{
+ 		printf(_("%s: directory \"%s\" is not a database cluster directory\n"),
+ 				 progname, pg_data);
+ 		exit(is_status_request ? 4 : 1);
+ 	}
  
  	pidf = fopen(pid_file, "r");
  	if (pidf == NULL)
*************** do_start(void)
*** 810,816 ****
  
  	if (ctl_command != RESTART_COMMAND)
  	{
! 		old_pid = get_pgpid();
  		if (old_pid != 0)
  			write_stderr(_("%s: another server might be running; "
  						   "trying to start server anyway\n"),
--- 835,841 ----
  
  	if (ctl_command != RESTART_COMMAND)
  	{
! 		old_pid = get_pgpid(false);
  		if (old_pid != 0)
  			write_stderr(_("%s: another server might be running; "
  						   "trying to start server anyway\n"),
*************** do_stop(void)
*** 894,900 ****
  	pgpid_t		pid;
  	struct stat statbuf;
  
! 	pid = get_pgpid();
  
  	if (pid == 0)				/* no pid file */
  	{
--- 919,925 ----
  	pgpid_t		pid;
  	struct stat statbuf;
  
! 	pid = get_pgpid(false);
  
  	if (pid == 0)				/* no pid file */
  	{
*************** do_stop(void)
*** 943,949 ****
  
  		for (cnt = 0; cnt < wait_seconds; cnt++)
  		{
! 			if ((pid = get_pgpid()) != 0)
  			{
  				print_msg(".");
  				pg_usleep(1000000);		/* 1 sec */
--- 968,974 ----
  
  		for (cnt = 0; cnt < wait_seconds; cnt++)
  		{
! 			if ((pid = get_pgpid(false)) != 0)
  			{
  				print_msg(".");
  				pg_usleep(1000000);		/* 1 sec */
*************** do_restart(void)
*** 980,986 ****
  	pgpid_t		pid;
  	struct stat statbuf;
  
! 	pid = get_pgpid();
  
  	if (pid == 0)				/* no pid file */
  	{
--- 1005,1011 ----
  	pgpid_t		pid;
  	struct stat statbuf;
  
! 	pid = get_pgpid(false);
  
  	if (pid == 0)				/* no pid file */
  	{
*************** do_restart(void)
*** 1033,1039 ****
  
  		for (cnt = 0; cnt < wait_seconds; cnt++)
  		{
! 			if ((pid = get_pgpid()) != 0)
  			{
  				print_msg(".");
  				pg_usleep(1000000);		/* 1 sec */
--- 1058,1064 ----
  
  		for (cnt = 0; cnt < wait_seconds; cnt++)
  		{
! 			if ((pid = get_pgpid(false)) != 0)
  			{
  				print_msg(".");
  				pg_usleep(1000000);		/* 1 sec */
*************** do_reload(void)
*** 1071,1077 ****
  {
  	pgpid_t		pid;
  
! 	pid = get_pgpid();
  	if (pid == 0)				/* no pid file */
  	{
  		write_stderr(_("%s: PID file \"%s\" does not exist\n"), progname, pid_file);
--- 1096,1102 ----
  {
  	pgpid_t		pid;
  
! 	pid = get_pgpid(false);
  	if (pid == 0)				/* no pid file */
  	{
  		write_stderr(_("%s: PID file \"%s\" does not exist\n"), progname, pid_file);
*************** do_promote(void)
*** 1110,1116 ****
  	pgpid_t		pid;
  	struct stat statbuf;
  
! 	pid = get_pgpid();
  
  	if (pid == 0)				/* no pid file */
  	{
--- 1135,1141 ----
  	pgpid_t		pid;
  	struct stat statbuf;
  
! 	pid = get_pgpid(false);
  
  	if (pid == 0)				/* no pid file */
  	{
*************** do_status(void)
*** 1204,1210 ****
  {
  	pgpid_t		pid;
  
! 	pid = get_pgpid();
  	/* Is there a pid file? */
  	if (pid != 0)
  	{
--- 1229,1235 ----
  {
  	pgpid_t		pid;
  
! 	pid = get_pgpid(true);
  	/* Is there a pid file? */
  	if (pid != 0)
  	{
*************** do_status(void)
*** 1247,1253 ****
  
  	/*
  	 * The Linux Standard Base Core Specification 3.1 says this should return
! 	 * '3'
  	 * https://refspecs.linuxbase.org/LSB_3.1.0/LSB-Core-generic/LSB-Core-generic/iniscrptact.html
  	 */
  	exit(3);
--- 1272,1278 ----
  
  	/*
  	 * The Linux Standard Base Core Specification 3.1 says this should return
! 	 * '3, program is not running'
  	 * https://refspecs.linuxbase.org/LSB_3.1.0/LSB-Core-generic/LSB-Core-generic/iniscrptact.html
  	 */
  	exit(3);
*************** main(int argc, char **argv)
*** 2285,2290 ****
--- 2310,2316 ----
  	if (pg_data)
  	{
  		snprintf(postopts_file, MAXPGPATH, "%s/postmaster.opts", pg_data);
+ 		snprintf(version_file, MAXPGPATH, "%s/PG_VERSION", pg_data);
  		snprintf(pid_file, MAXPGPATH, "%s/postmaster.pid", pg_data);
  		snprintf(backup_file, MAXPGPATH, "%s/backup_label", pg_data);
  		snprintf(recovery_file, MAXPGPATH, "%s/recovery.conf", pg_data);
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to