Received: from makus.postgresql.org (makus.postgresql.org [98.129.198.125]) by mail.postgresql.org (Postfix) with ESMTP id A53241698D03 for ; Mon, 14 May 2012 21:52:50 -0300 (ADT) Received: from mimolette2.dalibo.net ([88.190.221.82]) by makus.postgresql.org with esmtp (Exim 4.72) (envelope-from ) id 1SU60N-0003Qq-TD for pgsql-hackers@postgresql.org; Tue, 15 May 2012 00:52:49 +0000 Received: from [172.16.61.12] (d72-38-56-84.commercial1.cgocable.net [72.38.56.84]) by mimolette2.dalibo.net (Postfix) with ESMTPA id A7E153784E1E for ; Tue, 15 May 2012 02:52:33 +0200 (CEST) Message-ID: <4FB1A8D0.4080306@dalibo.com> Date: Tue, 15 May 2012 02:52:32 +0200 From: Gilles Darold User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.28) Gecko/20120313 Thunderbird/3.1.20 MIME-Version: 1.0 To: pgsql-hackers@postgresql.org Subject: Re: Patch pg_is_in_backup() References: <4F26E291.9020605@dalibo.com>

<4F2B470F.5070107@timbira.com> <691525170451D3F76CEF7ED1@apophis.local> <4F50E1B0.90808@dalibo.com> <4F76F7CF.2070908@2ndQuadrant.it> <4F7AEB63.1000704@dalibo.com> <4FA17491.4040406@2ndQuadrant.it> In-Reply-To: <4FA17491.4040406@2ndQuadrant.it> Content-Type: multipart/mixed; boundary="------------070107070800040505040008" X-Pg-Spam-Score: -1.9 (-) X-Archive-Number: 201205/706 X-Sequence-Number: 207961 This is a multi-part message in MIME format. --------------070107070800040505040008 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Sorry for the the double post but it seems that my previous reply doesn't reach the pgsql-hacker list. So here is the new patches that limit lines to 80 characters. Regards, Le 02/05/2012 19:53, Gabriele Bartolini a =E9crit : > Hi Gilles, > > Sorry for the delay. > > Il 03/04/12 14:21, Gilles Darold ha scritto: >> +1, this is also my point of view. > > I have looked at the patch that contains both pg_is_in_backup() and > pg_backup_start_time(). > > From a functional point of view it looks fine to me. I was thinking > of adding the BackupInProgress() at the beginning of > pg_backup_start_time(), but the AllocateFile() function already make > sure the file exists. > > I have performed some basic testing of both functions and tried to > inject invalid characters in the start time field of the backup_label > file and it is handled (with an exception) by the server. Cool. > > I spotted though some formatting issues, in particular indentation > and multi-line comments. Some rows are longer than 80 chars. > > Please resubmit with these cosmetic changes and it is fine with me. > Thank you. > > Cheers, > Gabriele > --=20 Gilles Darold Administrateur de bases de donn=E9es http://dalibo.com - http://dalibo.org --------------070107070800040505040008 Content-Type: text/x-patch; name="postgresql-pg_backup_start_time-patch-v4.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="postgresql-pg_backup_start_time-patch-v4.diff" diff -ru postgresql-head/doc/src/sgml/func.sgml postgresql/doc/src/sgml/func.sgml --- postgresql-head/doc/src/sgml/func.sgml 2012-04-03 10:44:43.979702643 +0200 +++ postgresql/doc/src/sgml/func.sgml 2012-04-03 11:21:15.075701574 +0200 @@ -14467,6 +14467,9 @@ pg_stop_backup + pg_backup_start_time + + pg_switch_xlog @@ -14532,6 +14535,13 @@ + pg_backup_start_time() + + timestamp with time zone + Get start time of an online exclusive backup in progress. + + + pg_switch_xlog() text diff -ru postgresql-head/src/backend/access/transam/xlogfuncs.c postgresql/src/backend/access/transam/xlogfuncs.c --- postgresql-head/src/backend/access/transam/xlogfuncs.c 2012-04-03 10:44:44.227702645 +0200 +++ postgresql/src/backend/access/transam/xlogfuncs.c 2012-04-03 11:21:15.075701574 +0200 @@ -29,6 +29,7 @@ #include "utils/numeric.h" #include "utils/guc.h" #include "utils/timestamp.h" +#include "storage/fd.h" static void validate_xlog_location(char *str); @@ -563,3 +564,67 @@ PG_RETURN_NUMERIC(result); } + +/* + * Returns start time of an online exclusive backup. + * + * When there's no exclusive backup in progress, the function + * returns NULL. + */ +Datum +pg_backup_start_time(PG_FUNCTION_ARGS) +{ + TimestampTz xtime; + FILE *lfp; + char fline[MAXPGPATH]; + char backup_start_time[30]; + + /* + * See if label file is present + */ + lfp = AllocateFile(BACKUP_LABEL_FILE, "r"); + if (lfp == NULL) + { + if (errno != ENOENT) + ereport(ERROR, + (errcode_for_file_access(), + errmsg("could not read file \"%s\": %m", + BACKUP_LABEL_FILE))); + PG_RETURN_NULL(); + } + + /* + * Parse the file to find the the START TIME line. + */ + backup_start_time[0] = '\0'; + while (fgets(fline, sizeof(fline), lfp) != NULL) + { + if (sscanf(fline, "START TIME: %25[^\n]\n", + backup_start_time) == 1) + break; + } + + /* + * Close the backup label file. + */ + if (ferror(lfp) || FreeFile(lfp)) { + ereport(ERROR, + (errcode_for_file_access(), + errmsg("could not read file \"%s\": %m", + BACKUP_LABEL_FILE))); + } + + if (strlen(backup_start_time) == 0) { + ereport(ERROR, + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("invalid data in file \"%s\"", + BACKUP_LABEL_FILE))); + } + + /* + * Convert the time string read from file to TimestampTz form. + */ + xtime = DatumGetTimestampTz( + DirectFunctionCall3(timestamptz_in, + CStringGetDatum(backup_start_time), + ObjectIdGetDatum(InvalidOid),Int32GetDatum(-1)) + ); + + PG_RETURN_TIMESTAMPTZ(xtime); +} + diff -ru postgresql-head/src/include/access/xlog_internal.h postgresql/src/include/access/xlog_internal.h --- postgresql-head/src/include/access/xlog_internal.h 2012-04-03 10:44:45.051702642 +0200 +++ postgresql/src/include/access/xlog_internal.h 2012-04-03 11:21:15.075701574 +0200 @@ -282,5 +282,6 @@ extern Datum pg_xlog_replay_resume(PG_FUNCTION_ARGS); extern Datum pg_is_xlog_replay_paused(PG_FUNCTION_ARGS); extern Datum pg_xlog_location_diff(PG_FUNCTION_ARGS); +extern Datum pg_backup_start_time(PG_FUNCTION_ARGS); #endif /* XLOG_INTERNAL_H */ diff -ru postgresql-head/src/include/catalog/pg_proc.h postgresql/src/include/catalog/pg_proc.h --- postgresql-head/src/include/catalog/pg_proc.h 2012-04-03 10:44:45.051702642 +0200 +++ postgresql/src/include/catalog/pg_proc.h 2012-04-03 11:21:15.075701574 +0200 @@ -2924,6 +2924,8 @@ DESCR("prepare for taking an online backup"); DATA(insert OID = 2173 ( pg_stop_backup PGNSP PGUID 12 1 0 0 0 f f f f t f v 0 0 25 "" _null_ _null_ _null_ _null_ pg_stop_backup _null_ _null_ _null_ )); DESCR("finish taking an online backup"); +DATA(insert OID = 3883 ( pg_backup_start_time PGNSP PGUID 12 1 0 0 0 f f f f t f s 0 0 1184 "" _null_ _null_ _null_ _null_ pg_backup_start_time _null_ _null_ _null_ )); +DESCR("start time of an online backup"); DATA(insert OID = 2848 ( pg_switch_xlog PGNSP PGUID 12 1 0 0 0 f f f f t f v 0 0 25 "" _null_ _null_ _null_ _null_ pg_switch_xlog _null_ _null_ _null_ )); DESCR("switch to new xlog file"); DATA(insert OID = 3098 ( pg_create_restore_point PGNSP PGUID 12 1 0 0 0 f f f f t f v 1 0 25 "25" _null_ _null_ _null_ _null_ pg_create_restore_point _null_ _null_ _null_ )); --------------070107070800040505040008 Content-Type: text/x-patch; name="postgresql-pg_backup_start_time-pg_is_in_backup-patch-v4.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="postgresql-pg_backup_start_time-pg_is_in_backup-patch-v4.dif"; filename*1="f" diff -ru postgresql-head/doc/src/sgml/func.sgml postgresql/doc/src/sgml/func.sgml --- postgresql-head/doc/src/sgml/func.sgml 2012-04-03 10:44:43.979702643 +0200 +++ postgresql/doc/src/sgml/func.sgml 2012-04-03 12:00:13.099700433 +0200 @@ -14467,6 +14467,12 @@ pg_stop_backup + pg_is_in_backup + + + pg_backup_start_time + + pg_switch_xlog @@ -14532,6 +14538,20 @@ + pg_is_in_backup() + + bool + True if an on-line exclusive backup is still in progress. + + + + pg_backup_start_time() + + timestamp with time zone + Get start time of an online exclusive backup in progress. + + + pg_switch_xlog() text diff -ru postgresql-head/src/backend/access/transam/xlogfuncs.c postgresql/src/backend/access/transam/xlogfuncs.c --- postgresql-head/src/backend/access/transam/xlogfuncs.c 2012-04-03 10:44:44.227702645 +0200 +++ postgresql/src/backend/access/transam/xlogfuncs.c 2012-04-03 12:02:51.043700358 +0200 @@ -29,7 +29,7 @@ #include "utils/numeric.h" #include "utils/guc.h" #include "utils/timestamp.h" - +#include "storage/fd.h" static void validate_xlog_location(char *str); @@ -563,3 +563,75 @@ PG_RETURN_NUMERIC(result); } + +/* + * Returns bool with current on-line backup mode, a global state. + */ +Datum +pg_is_in_backup(PG_FUNCTION_ARGS) +{ + PG_RETURN_BOOL(BackupInProgress()); +} + +/* + * Returns start time of an online exclusive backup. + * + * When there's no exclusive backup in progress, the function + * returns NULL. + */ +Datum +pg_backup_start_time(PG_FUNCTION_ARGS) +{ + TimestampTz xtime; + FILE *lfp; + char fline[MAXPGPATH]; + char backup_start_time[30]; + + /* + * See if label file is present + */ + lfp = AllocateFile(BACKUP_LABEL_FILE, "r"); + if (lfp == NULL) + { + if (errno != ENOENT) + ereport(ERROR, + (errcode_for_file_access(), + errmsg("could not read file \"%s\": %m", + BACKUP_LABEL_FILE))); + PG_RETURN_NULL(); + } + + /* + * Parse the file to find the the START TIME line. + */ + backup_start_time[0] = '\0'; + while (fgets(fline, sizeof(fline), lfp) != NULL) + { + if (sscanf(fline, "START TIME: %25[^\n]\n", + backup_start_time) == 1) + break; + } + + /* + * Close the backup label file. + */ + if (ferror(lfp) || FreeFile(lfp)) { + ereport(ERROR, + (errcode_for_file_access(), + errmsg("could not read file \"%s\": %m", + BACKUP_LABEL_FILE))); + } + + if (strlen(backup_start_time) == 0) { + ereport(ERROR, + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("invalid data in file \"%s\"", + BACKUP_LABEL_FILE))); + } + + /* + * Convert the time string read from file to TimestampTz form. + */ + xtime = DatumGetTimestampTz( + DirectFunctionCall3(timestamptz_in, + CStringGetDatum(backup_start_time), + ObjectIdGetDatum(InvalidOid),Int32GetDatum(-1)) + ); + + PG_RETURN_TIMESTAMPTZ(xtime); +} diff -ru postgresql-head/src/include/access/xlog_internal.h postgresql/src/include/access/xlog_internal.h --- postgresql-head/src/include/access/xlog_internal.h 2012-04-03 10:44:45.051702642 +0200 +++ postgresql/src/include/access/xlog_internal.h 2012-04-03 12:06:38.955700246 +0200 @@ -282,5 +282,7 @@ extern Datum pg_xlog_replay_resume(PG_FUNCTION_ARGS); extern Datum pg_is_xlog_replay_paused(PG_FUNCTION_ARGS); extern Datum pg_xlog_location_diff(PG_FUNCTION_ARGS); +extern Datum pg_is_in_backup(PG_FUNCTION_ARGS); +extern Datum pg_backup_start_time(PG_FUNCTION_ARGS); #endif /* XLOG_INTERNAL_H */ diff -ru postgresql-head/src/include/catalog/pg_proc.h postgresql/src/include/catalog/pg_proc.h --- postgresql-head/src/include/catalog/pg_proc.h 2012-04-03 10:44:45.051702642 +0200 +++ postgresql/src/include/catalog/pg_proc.h 2012-04-03 12:07:44.555700213 +0200 @@ -2924,6 +2924,10 @@ DESCR("prepare for taking an online backup"); DATA(insert OID = 2173 ( pg_stop_backup PGNSP PGUID 12 1 0 0 0 f f f f t f v 0 0 25 "" _null_ _null_ _null_ _null_ pg_stop_backup _null_ _null_ _null_ )); DESCR("finish taking an online backup"); +DATA(insert OID = 3882 ( pg_is_in_backup PGNSP PGUID 12 1 0 0 0 f f f f t f v 0 0 16 "" _null_ _null_ _null_ _null_ pg_is_in_backup _null_ _null_ _null_ )); +DESCR("true if server is in online backup"); +DATA(insert OID = 3883 ( pg_backup_start_time PGNSP PGUID 12 1 0 0 0 f f f f t f s 0 0 1184 "" _null_ _null_ _null_ _null_ pg_backup_start_time _null_ _null_ _null_ )); +DESCR("start time of an online backup"); DATA(insert OID = 2848 ( pg_switch_xlog PGNSP PGUID 12 1 0 0 0 f f f f t f v 0 0 25 "" _null_ _null_ _null_ _null_ pg_switch_xlog _null_ _null_ _null_ )); DESCR("switch to new xlog file"); DATA(insert OID = 3098 ( pg_create_restore_point PGNSP PGUID 12 1 0 0 0 f f f f t f v 1 0 25 "25" _null_ _null_ _null_ _null_ pg_create_restore_point _null_ _null_ _null_ )); --------------070107070800040505040008 Content-Type: text/x-patch; name="postgresql-pg_is_in_backup-patch-v4.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="postgresql-pg_is_in_backup-patch-v4.diff" diff -ru postgresql-head/doc/src/sgml/func.sgml postgresql/doc/src/sgml/func.sgml --- postgresql-head/doc/src/sgml/func.sgml 2012-04-03 10:44:43.979702643 +0200 +++ postgresql/doc/src/sgml/func.sgml 2012-04-03 11:24:41.427701474 +0200 @@ -14467,6 +14467,9 @@ pg_stop_backup + pg_is_in_backup + + pg_switch_xlog @@ -14532,6 +14535,13 @@ + pg_is_in_backup() + + bool + True if an on-line exclusive backup is still in progress. + + + pg_switch_xlog() text diff -ru postgresql-head/src/backend/access/transam/xlogfuncs.c postgresql/src/backend/access/transam/xlogfuncs.c --- postgresql-head/src/backend/access/transam/xlogfuncs.c 2012-04-03 10:44:44.227702645 +0200 +++ postgresql/src/backend/access/transam/xlogfuncs.c 2012-04-03 11:24:41.427701474 +0200 @@ -563,3 +563,12 @@ PG_RETURN_NUMERIC(result); } + +/* + * Returns bool with current on-line backup mode, a global state. + */ +Datum +pg_is_in_backup(PG_FUNCTION_ARGS) +{ + PG_RETURN_BOOL(BackupInProgress()); +} diff -ru postgresql-head/src/include/access/xlog_internal.h postgresql/src/include/access/xlog_internal.h --- postgresql-head/src/include/access/xlog_internal.h 2012-04-03 10:44:45.051702642 +0200 +++ postgresql/src/include/access/xlog_internal.h 2012-04-03 11:24:41.427701474 +0200 @@ -282,5 +282,6 @@ extern Datum pg_xlog_replay_resume(PG_FUNCTION_ARGS); extern Datum pg_is_xlog_replay_paused(PG_FUNCTION_ARGS); extern Datum pg_xlog_location_diff(PG_FUNCTION_ARGS); +extern Datum pg_is_in_backup(PG_FUNCTION_ARGS); #endif /* XLOG_INTERNAL_H */ diff -ru postgresql-head/src/include/catalog/pg_proc.h postgresql/src/include/catalog/pg_proc.h --- postgresql-head/src/include/catalog/pg_proc.h 2012-04-03 10:44:45.051702642 +0200 +++ postgresql/src/include/catalog/pg_proc.h 2012-04-03 11:24:41.427701474 +0200 @@ -2924,6 +2924,8 @@ DESCR("prepare for taking an online backup"); DATA(insert OID = 2173 ( pg_stop_backup PGNSP PGUID 12 1 0 0 0 f f f f t f v 0 0 25 "" _null_ _null_ _null_ _null_ pg_stop_backup _null_ _null_ _null_ )); DESCR("finish taking an online backup"); +DATA(insert OID = 3882 ( pg_is_in_backup PGNSP PGUID 12 1 0 0 0 f f f f t f v 0 0 16 "" _null_ _null_ _null_ _null_ pg_is_in_backup _null_ _null_ _null_ )); +DESCR("true if server is in online backup"); DATA(insert OID = 2848 ( pg_switch_xlog PGNSP PGUID 12 1 0 0 0 f f f f t f v 0 0 25 "" _null_ _null_ _null_ _null_ pg_switch_xlog _null_ _null_ _null_ )); DESCR("switch to new xlog file"); DATA(insert OID = 3098 ( pg_create_restore_point PGNSP PGUID 12 1 0 0 0 f f f f t f v 1 0 25 "25" _null_ _null_ _null_ _null_ pg_create_restore_point _null_ _null_ _null_ )); --------------070107070800040505040008--