Changes in repmgr are:

- Add checks to test if the directories are useful for us (basically
  the checks follow the same approach as initdb does)
- Add connection parameters
- Better use of rsync
- Some more clean up of code

Changes in repmgrd are:
- Add a parameter to allow the user specify an repmgr.conf
- Change the name of the repl_status table for repl_monitor
- Create a repl_status view that also shows lag in time
- Some more clean up of code

check_dir.c

@@ -0,0 +1,93 @@
+/*
+ * check_dir.c
+ * Copyright (c) 2ndQuadrant, 2010
+ *
+ * Directories management functions
+ */
+
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <dirent.h>
+
+#include "check_dir.h"
+
+/*
+ * make sure the directory either doesn't exist or is empty
+ * we use this function to check the new data directory and
+ * the directories for tablespaces
+ *
+ * This is the same check initdb does on the new PGDATA dir
+ *
+ * Returns 0 if nonexistent, 1 if exists and empty, 2 if not empty,
+ * or -1 if trouble accessing directory
+ */
+int
+check_dir(const char *dir)
+{
+    DIR        *chkdir;
+    struct 		dirent *file;
+    int         result = 1;
+
+	char		*dummy_file;
+	FILE 		*dummy_fd;
+
+    errno = 0;
+
+    chkdir = opendir(dir);
+
+    if (!chkdir)
+        return (errno == ENOENT) ? 0 : -1;
+
+    while ((file = readdir(chkdir)) != NULL)
+    {
+        if (strcmp(".", file->d_name) == 0 ||
+            strcmp("..", file->d_name) == 0)
+        {
+            /* skip this and parent directory */
+            continue;
+        }
+        else
+        {
+            result = 2;         /* not empty */
+            break;
+        }
+    }
+
+#ifdef WIN32
+    /*
+     * This fix is in mingw cvs (runtime/mingwex/dirent.c rev 1.4), but not in
+     * released version
+     */
+    if (GetLastError() == ERROR_NO_MORE_FILES)
+        errno = 0;
+#endif
+
+    closedir(chkdir);
+
+    if (errno != 0)
+        return -1          /* some kind of I/O error? */
+
+	return result;
+}
+
+
+/*
+ * Create directory 
+ */
+bool
+create_directory(const char *dir)
+{
+    if (mkdir_p(dir, 0700) == 0)
+        return true;
+
+    fprintf(stderr, _("Could not create directory \"%s\": %s\n"),
+            dir, strerror(errno));
+
+    return false;
+}
+
+bool
+set_directory_permissions(const char *dir)
+{
+	return (chmod(data_dir, 0700) != 0) ? false : true;
+}

check_dir.h

@@ -0,0 +1,9 @@
+/*
+ * check_dir.h
+ * Copyright (c) 2ndQuadrant, 2010
+ *
+ */
+
+int  check_dir(const char *dir);
+bool create_directory(const char *dir);
+bool set_directory_permissions(const char *dir);

config.c

@@ -8,10 +8,10 @@
 #include "repmgr.h"
 
 void
-parse_config(char *cluster_name, int *node, char *conninfo)
+parse_config(const *char config_file, char *cluster_name, int *node, char *conninfo)
 {
 	char *s, buff[256];
-	FILE *fp = fopen (CONFIG_FILE, "r");
+	FILE *fp = fopen (config_file, "r");
 
 	if (fp == NULL)
     	return;

config.h

@@ -4,6 +4,6 @@
  *
  */
 
-void parse_config(char *cluster_name, int *node, char *service);
+void parse_config(const char *config_file, char *cluster_name, int *node, char *service);
 void parse_line(char *buff, char *name, char *value);
 char *trim(char *s);

dbutils.c

@@ -2,9 +2,7 @@
  * dbutils.c
  * Copyright (c) 2ndQuadrant, 2010
  *
- * Database connections/managements functions
- * XXX At least i can create another function here to avoid code duplication
- *     on the main file
+ * Database connection/management functions
  *
  */
 
@@ -69,6 +67,8 @@ is_supported_version(PGconn *conn)
     {
 		fprintf(stderr, "PQexec failed: %s", PQerrorMessage(conn));
         PQclear(res);
+		PQfinish(conn);
+		exit(1);
     }
     major_version = atoi(PQgetvalue(res, 0, 0));
     PQclear(res);
@@ -92,6 +92,8 @@ guc_setted(PGconn *conn, const char *parameter, const char *op, const char *valu
     {
 		fprintf(stderr, "PQexec failed: %s", PQerrorMessage(conn));
         PQclear(res);
+		PQfinish(conn);
+		exit(1);
     }
 	if (PQgetisnull(res, 0, 0))
 	{
@@ -124,6 +126,8 @@ get_cluster_size(PGconn *conn)
     {
 		fprintf(stderr, "PQexec failed: %s", PQerrorMessage(conn));
         PQclear(res);
+		PQfinish(conn);
+		exit(1);
     }
    	strcpy(size, PQgetvalue(res, 0, 0))
 	PQclear(res);

repmgr.c

@@ -4,6 +4,9 @@
  *
  * Command interpreter for the repmgr
  * This module execute some tasks based on commands and then exit
+ * 
+ * Commands implemented are.
+ * STANDBY CLONE, STANDBY FOLLOW, STANDBY PROMOTE
  */
 
 #include "repmgr.h"
@@ -13,6 +16,8 @@
 #include <time.h>
 #include <unistd.h>
 
+#include "check_dir.h"
+
 #define RECOVERY_FILE "recovery.conf"
 #define RECOVERY_DONE_FILE "recovery.done"
 
@@ -21,6 +26,8 @@
 #define STANDBY_FOLLOW 	3
 
 static void help(const char *progname);
+static bool create_recovery_file(void);
+
 static void do_standby_clone(void);
 static void do_standby_promote(void);
 static void do_standby_follow(void);
@@ -117,7 +124,13 @@ main(int argc, char **argv)
 		}
 	}
 
-    /* Get real command from command line */
+    /* 
+	 * Now we need to obtain the action, this comes in the form:
+     * STANDBY {CLONE [node]|PROMOTE|FOLLOW [node]}
+	 * 
+     * the node part is optional, if we receive it then we shouldn't
+     * have received a -h option
+     */
     if (optind < argc)
 	{
         stndby = argv[optind++];
@@ -144,6 +157,20 @@ main(int argc, char **argv)
 		}	
 	}
 
+	/* For STANDBY CLONE and STANDBY FOLLOW we still can receive a last argument */
+	if ((action == STANDBY_CLONE) || (action == STANDBY_FOLLOW))
+	{
+	    if (optind < argc)
+		{
+			if (host != NULL)
+			{
+				fprintf(stderr, _("Conflicting parameters you can't use -h while providing a node separately. Try \"%s --help\" for more information.\n"), progname);
+				exit(1);
+			}	
+    	    host = argv[optind++];
+		}
+	}
+
 	switch (optind < argc)
 	{
 		case 0:
@@ -205,37 +232,64 @@ do_standby_clone(void)
 	char 		sqlquery[8192];
 	char 		script[8192];
 
-	int			r;
+	int			r, i;
 	char		data_dir_full_path[MAXLEN];
 	char		data_dir[MAXLEN];
-	char		recovery_file_path[MAXLEN];
-
-	char		*dummy_file;
-	FILE		*recovery_file;
 
 	char		*first_wal_segment, *last_wal_segment;
 
-	char		line[MAXLEN];
-
     /* Connection parameters for master only */
     keywords[0] = "host";
     values[0] = host;
     keywords[1] = "port";
     values[1] = masterport;
 
-	/* Can we write in this directory? write a dummy file to test that */
-	sprintf(dummy_file, "%s/dummy", ((data_dir == NULL) ? "." : data_dir));
-    recovery_file = fopen(dummy_file, "w");
-    if (recovery_file == NULL)
-	{
-		fprintf(stderr, _("Can't write in this directory, check permissions"));
-		return;
-	}
-	/* If we could write the file, unlink it... it was just a test */
-	fclose(recovery_file);
-	unlink(recovery_file);
+	if (data_dir == NULL)
+		strcpy(data_dir, ".");
 
-	/* inform the master we will start a backup */
+	/* Check this directory could be used as a PGDATA dir */
+    switch (check_data_dir(data_dir))
+    {
+        case 0:
+            /* data_dir not there, must create it */
+			if (verbose)
+	            printf(_("creating directory %s ... "), data_dir);
+            fflush(stdout);
+
+            if (!create_directory(data_dir))
+			{
+            	fprintf(stderr, _("%s: couldn't create directory %s ... "), 
+						progname, data_dir);
+				return;
+			}
+            break;
+        case 1:
+            /* Present but empty, fix permissions and use it */
+			if (verbose)
+	            printf(_("fixing permissions on existing directory %s ... "),
+   			                data_dir);
+            fflush(stdout);
+
+			if (!set_directory_permissions(data_dir))
+            {
+                fprintf(stderr, _("%s: could not change permissions of directory \"%s\": %s\n"),
+                        progname, data_dir, strerror(errno));
+				return;
+            }
+            break;
+        case 2:
+            /* Present and not empty */
+            fprintf(stderr,
+                    _("%s: directory \"%s\" exists but is not empty\n"),
+                    progname, data_dir);
+            return;     
+        default:
+            /* Trouble accessing directory */
+            fprintf(stderr, _("%s: could not access directory \"%s\": %s\n"),
+                    progname, data_dir, strerror(errno));
+    }
+
+	/* We need to connect to check configuration and start a backup */
 	conn = PQconnectdbParams(keywords, values, true);	
     if (!conn)
     {
@@ -267,10 +321,10 @@ do_standby_clone(void)
 		fprintf(stderr, _("%s needs parameter 'wal_level' to be set to 'hot_standby'\n", progname)); 
 		return;
 	}
-	if (!guc_setted("wal_keep_segments", "=", "5000"))
+	if (!guc_setted("wal_keep_segments", ">=", "5000"))
 	{
 		PQfinish(conn);
-		fprintf(stderr, _("%s needs parameter 'wal_keep_segments' to be set greater than 0\n", progname)); 
+		fprintf(stderr, _("%s needs parameter 'wal_keep_segments' to be set to 5000 or greater\n", progname)); 
 		return;
 	}
 	if (!guc_setted("archive_mode", "=", "on"))
@@ -280,9 +334,71 @@ do_standby_clone(void)
 		return;
 	}
 
-	if (verbose)
-		printf(_("Succesfully connected to primary. Current installation size is %s\n", get_cluster_size(conn)));
+	printf(_("Succesfully connected to primary. Current installation size is %s\n", get_cluster_size(conn)));
 
+	/* Check if the tablespace locations exists and that we can write to them */
+	sprintf(sqlquery, "select location from pg_tablespace where spcname not in ('pg_default', 'pg_global')");
+    res = PQexec(conn, sqlquery);
+    if (PQresultStatus(res) != PGRES_TUPLES_OK)
+	{
+        fprintf(stderr, "Can't get info about tablespaces: %s\n", PQerrorMessage(conn));
+        PQclear(res);
+        PQfinish(conn);
+        return;
+	}
+	for (i = 0; i < PQntuples(res); i++)
+	{
+		/* Check this directory could be used as a PGDATA dir */
+	    switch (check_dir(PQgetvalue(res), i, 0))
+	    {
+	        case 0:
+	            /* data_dir not there, must create it */
+				if (verbose)
+		            printf(_("creating directory \"%s\"... "), data_dir);
+	            fflush(stdout);
+	
+	            if (!create_directory(data_dir))
+				{
+	            	fprintf(stderr, _("%s: couldn't create directory \"%s\"... "), 
+							progname, data_dir);
+					PQclear(res);
+					PQfinish(conn);
+					return;
+				}
+	            break;
+	        case 1:
+	            /* Present but empty, fix permissions and use it */
+				if (verbose)
+		            printf(_("fixing permissions on existing directory \"%s\"... "),
+	   			                data_dir);
+   	         fflush(stdout);
+	
+   	         if (!set_directory_permissions(data_dir))
+   	         {
+   	             fprintf(stderr, _("%s: could not change permissions of directory \"%s\": %s\n"),
+   	                     progname, data_dir, strerror(errno));
+					PQclear(res);
+					PQfinish(conn);
+					return;
+   	         }
+   	         break;
+   	     case 2:
+   	         /* Present and not empty */
+			fprintf(stderr,
+   	        		_("%s: directory \"%s\" exists but is not empty\n"),
+   	                progname, data_dir);
+			PQclear(res);
+			PQfinish(conn);
+   	        return;     
+   	     default:
+   	         /* Trouble accessing directory */
+   	        fprintf(stderr, _("%s: could not access directory \"%s\": %s\n"),
+   	                progname, data_dir, strerror(errno));
+			PQclear(res);
+			PQfinish(conn);
+			return;
+	}
+		
 	fprintf(stderr, "Starting backup...\n");
 	
 	/* Get the data directory full path and the last subdirectory */
@@ -301,6 +417,10 @@ do_standby_clone(void)
 	strcpy(data_dir, PQgetvalue(res, 0, 1));
     PQclear(res);
 
+	/* 
+     * inform the master we will start a backup and get the first XLog filename
+	 * so we can say to the user we need those files
+     */
 	sprintf(sqlquery, "SELECT pg_xlogfile_name(pg_start_backup('repmgr_standby_clone_%ld'))", time(NULL));
     res = PQexec(conn, sqlquery);
     if (PQresultStatus(res) != PGRES_TUPLES_OK)
@@ -314,7 +434,7 @@ do_standby_clone(void)
     PQclear(res);
     PQfinish(conn);
 
-	/* rsync data directory to current location */
+	/* rsync data directory to data_dir */
 	sprintf(script, "rsync --checksum --keep-dirlinks --compress --progress -r %s:%s %s", 
 					host, data_dir_full_path, ((data_dir == NULL) ? "." : data_dir));
 	r = system(script);
@@ -360,33 +480,7 @@ do_standby_clone(void)
 		return;
 
 	/* Finally, write the recovery.conf file */
-	sprintf(recovery_file_path, "%s/%s", data_dir, RECOVERY_FILE);
-
-    recovery_file = fopen(recovery_file_path, "w");
-    if (recovery_file == NULL)
-    {
-        fprintf(stderr, "could not create recovery.conf file, it could be necesary to create it manually\n");
-		return;
-    }
-
-	sprintf(line, "standby_mode = 'on'\n");
-	if (fputs(line, recovery_file) == EOF)
-    {
-        fprintf(stderr, "recovery file could not be written, it could be necesary to create it manually\n");
-    	fclose(recovery_file);
-		return;
-    }
-
-	sprintf(line, "primary_conninfo = '%s'\n", master_conninfo);
-	if (fputs(line, recovery_file) == EOF)
-    {
-        fprintf(stderr, "recovery file could not be written, it could be necesary to create it manually\n");
-    	fclose(recovery_file);
-		return;
-    }
-
-    /*FreeFile(recovery_file);*/
-    fclose(recovery_file);
+	create_recovery_file();
 
 	/* We don't start the service because we still may want to move the directory */
 	return;
@@ -480,10 +574,6 @@ do_standby_follow(void)
 
 	int			r;
 	char		data_dir[MAXLEN];
-	char		recovery_file_path[MAXLEN];
-	FILE		*recovery_file;
-
-	char		line[MAXLEN];
 
 	/*
 	 * Read the configuration file: repmgr.conf
@@ -533,33 +623,8 @@ do_standby_follow(void)
     PQfinish(conn);
 
 	/* Finally, write the recovery.conf file */
-	sprintf(recovery_file_path, "%s/%s", data_dir, RECOVERY_FILE);
-
-    recovery_file = fopen(recovery_file_path, "w");
-    if (recovery_file == NULL)
-    {
-        fprintf(stderr, "could not create recovery.conf file, it could be necesary to create it manually\n");
-		return;
-    }
-
-	sprintf(line, "standby_mode = 'on'\n");
-	if (fputs(line, recovery_file) == EOF)
-    {
-        fprintf(stderr, "recovery file could not be written, it could be necesary to create it manually\n");
-    	fclose(recovery_file);
-		return;
-    }
-
-	sprintf(line, "primary_conninfo = '%s'\n", master_conninfo);
-	if (fputs(line, recovery_file) == EOF)
-    {
-        fprintf(stderr, "recovery file could not be written, it could be necesary to create it manually\n");
-    	fclose(recovery_file);
-		return;
-    }
-
-    /*FreeFile(recovery_file);*/
-    fclose(recovery_file);
+	if (!create_recovery_file())
+		return;		
 
 	/* Finally, restart the service */
 	/* We assume the pg_ctl script is in the PATH */
@@ -578,12 +643,62 @@ do_standby_follow(void)
 static void 
 help(const char *progname)
 {
-    printf(stderr, "\n%s: Replicator manager \n"
-					"This command program performs some tasks like clone a node, promote it "
-					"or making follow another node and then exits.\n"
-                    "COMMANDS:\n"
-                    "standby clone [node]  - allows creation of a new standby\n"
-                    "standby promote       - allows manual promotion of a specific standby into a "
-                                            "new master in the event of a failover\n"
-                    "standby follow [node] - allows the standby to re-point itself to a new master\n", progname);
+    printf(_("\n%s: Replicator manager \n", progname));
+    printf(_("Usage:\n"));
+    printf(_(" %s [OPTIONS] standby {clone|promote|follow} [master]\n", progname));
+    printf(_("\nOptions:\n"));
+	printf(_("  --help                    show this help, then exit\n"));
+	printf(_("  --version                 output version information, then exit\n"));
+	printf(_("  --verbose                 output verbose activity information\n"));
+	printf(_("\nConnection options:\n"));
+	printf(_("  -d, --dbname=DBNAME       database to connect to\n"));
+	printf(_("  -h, --host=HOSTNAME       database server host or socket directory\n"));
+	printf(_("  -p, --port=PORT           database server port\n"));
+	printf(_("  -U, --username=USERNAME   user name to connect as\n"));
+    printf(_("\n%s performs some tasks like clone a node, promote it ", progname));
+    printf(_("or making follow another node and then exits.\n"));
+    printf(_("COMMANDS:\n"));
+    printf(_(" standby clone [node]  - allows creation of a new standby\n"));
+    printf(_(" standby promote       - allows manual promotion of a specific standby into a "));
+    printf(_("new master in the event of a failover\n"));
+    printf(_(" standby follow [node] - allows the standby to re-point itself to a new master\n"));
+}
+
+
+static bool
+create_recovery_file(void)
+{
+	FILE		*recovery_file;
+	char		recovery_file_path[MAXLEN];
+	char		line[MAXLEN];
+
+	sprintf(recovery_file_path, "%s/%s", data_dir, RECOVERY_FILE);
+
+    recovery_file = fopen(recovery_file_path, "w");
+    if (recovery_file == NULL)
+    {
+        fprintf(stderr, "could not create recovery.conf file, it could be necesary to create it manually\n");
+		return false;
+    }
+
+	sprintf(line, "standby_mode = 'on'\n");
+	if (fputs(line, recovery_file) == EOF)
+    {
+        fprintf(stderr, "recovery file could not be written, it could be necesary to create it manually\n");
+    	fclose(recovery_file);
+		return false;
+    }
+
+	sprintf(line, "primary_conninfo = '%s'\n", host);
+	if (fputs(line, recovery_file) == EOF)
+    {
+        fprintf(stderr, "recovery file could not be written, it could be necesary to create it manually\n");
+    	fclose(recovery_file);
+		return false;
+    }
+
+    /*FreeFile(recovery_file);*/
+    fclose(recovery_file);
+
+	return true;
 }

repmgr.sql

@@ -1,17 +1,37 @@
+drop table if exists repl_nodes;
+
+/*
+ * The table repl_nodes keeps information about all machines in
+ * a cluster
+ */
 CREATE TABLE repl_nodes (
   id 		integer	primary key,
-  cluster   text	not null,
-  conninfo	text	not null
+  cluster   text	not null,	-- Name to identify the cluster
+  conninfo	text	not null 	
 );
 
 drop table if exists repl_status;
 
-CREATE TABLE repl_status(
+/*
+ * Keeps monitor info about every node and their "position" relative 
+ * to primary
+ */
+CREATE TABLE repl_monitor(
   primary_node                   INTEGER NOT NULL,
   standby_node                   INTEGER NOT NULL,
-  last_monitor_timestamp    	 TIMESTAMP WITH TIME ZONE NOT NULL,
-  last_wal_primary_location      TEXT NOT NULL,
+  last_monitor_time		    	 TIMESTAMP WITH TIME ZONE NOT NULL,
+  last_wal_primary_location      TEXT NOT NULL,		
   last_wal_standby_location      TEXT NOT NULL,
-  replication_lag                BIGINT NOT NULL,
-  apply_lag                      BIGINT NOT NULL
+  replication_lag                BIGINT NOT NULL, 
+  apply_lag                      BIGINT NOT NULL  
 );
+
+
+/*
+ * A useful view 
+ */
+CREATE VIEW repl_status AS
+SELECT *, now() - (select max(last_monitor_time) from repl_monitor b
+                    where b.primary_node = a.primary_node
+                      and b.standby_node = a.standby_node)
+  FROM repl_monitor;

repmgrd.c

@@ -26,6 +26,12 @@ char	primaryConninfo[MAXLEN];
 PGconn *primaryConn;
 
 
+const char *progname;
+
+const char	*config_file = NULL;
+bool		verbose = false;
+
+
 void checkClusterConfiguration(void);
 void checkNodeConfiguration(char *conninfo);
 void getPrimaryConnection(void);
@@ -39,12 +45,58 @@ unsigned long long int walLocationToBytes(char *wal_location);
 int
 main(int argc, char **argv)
 {
+	static struct option long_options[] = {
+		{"config", required_argument, NULL, 'f'},
+		{"verbose", no_argument, NULL, 'v'},
+		{NULL, 0, NULL, 0}
+	};
+
+	int			optindex;
+	int			c;
+	int			action;
+
     char conninfo[MAXLEN]; 
 
+	progname = get_progname(argv[0]);
+
+	if (argc > 1)
+	{
+		if (strcmp(argv[1], "--help") == 0 || strcmp(argv[1], "-?") == 0)
+		{
+			help(progname);
+			exit(0);
+		}
+		if (strcmp(argv[1], "--version") == 0 || strcmp(argv[1], "-V") == 0)
+		{
+			printf("%s (PostgreSQL) " PG_VERSION "\n", progname);
+			exit(0);
+		}
+	}
+
+
+	while ((c = getopt_long(argc, argv, "f:v", long_options, &optindex)) != -1)
+	{
+		switch (c)
+		{
+			case 'f':
+				config_file = optarg;
+				break;
+			case 'v':
+				verbose = true;
+				break;
+			default:
+				fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
+				exit(1);
+		}
+	}
+	
+	if (config_file == NULL)
+		sprintf(config_file, "./%s", CONFIG_FILE);
+
 	/*
 	 * Read the configuration file: repmgr.conf
      */
-	parse_config(myClusterName, &myLocalId, conninfo);
+	parse_config(config_file, myClusterName, &myLocalId, conninfo);
 	if (myLocalId == -1) 
 	{
 		fprintf(stderr, "Node information is missing. "
@@ -219,7 +271,7 @@ MonitorExecute(void)
 	 * Build the SQL to execute on primary
 	 */
 	sprintf(sqlquery,
-				"INSERT INTO repl_status "
+				"INSERT INTO repl_monitor "
 				"VALUES(%d, %d, '%s'::timestamp with time zone, "
                       " '%s', '%s', "
 					  " %lld, %lld)",