Skip site navigation (1) Skip section navigation (2)

Re: Named restore points

From: Euler Taveira de Oliveira <euler(at)timbira(dot)com>
To: Jaime Casanova <jaime(at)2ndquadrant(dot)com>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Named restore points
Date: 2011-02-01 15:02:52
Message-ID: 4D48209C.7050109@timbira.com (view raw or flat)
Thread:
Lists: pgsql-hackers
Em 14-01-2011 17:41, Jaime Casanova escreveu:
> Here is a patch that implements "named restore points".
>
Sorry, I was swamped with work. :(

Your patch no longer applied so I rebased it and slightly modified it. Review 
is below...

+         The default is to recover to the end of the WAL log.
+         The precise stopping point is also influenced by
+         <xref linkend="recovery-target-inclusive">.
+        </para>

This isn't valid. recovery_target_name are not influenced by 
recovery_target_inclusive. Sentence removed.

+ static char recoveryStopNamedRestorePoint[MAXFNAMELEN];

Is MAXFNAMELEN appropriate? AFAICS it is used for file name length. [Looking 
at code...] It seems to be used for backup label too so it is not so 
inappropriate.

+ typedef struct xl_named_restore_points
+ {
+ 	TimestampTz xtime;
+ 	char		name[MAXFNAMELEN];
+ } xl_named_restore_points;
+

I prefixed those struct members so it won't get confused elsewhere.

+ 	else if (recoveryTarget == RECOVERY_TARGET_NAME)
+ 		snprintf(buffer, sizeof(buffer),
+ 				 "%s%u\t%s\t%s named restore point %s\n",
+ 				 (srcfd < 0) ? "" : "\n",
+ 				 parentTLI,
+ 				 xlogfname,
+ 				 recoveryStopAfter ? "after" : "before",
+ 				 recoveryStopNamedRestorePoint);

It doesn't matter if it is after or before the restore point. After/Before 
only make sense when we're dealing with transaction or time. Removed.

   		else if (strcmp(item->name, "recovery_target_xid") == 0)
   		{
+ 			/*
+ 			 * if recovery_target_name specified, then this overrides
+ 			 * recovery_target_xid
+ 			 */
+ 			if (recoveryTarget == RECOVERY_TARGET_NAME)
+ 				continue;
+

IMHO the right recovery precedence is xid -> name -> time. If you're 
specifying xid that's because you know what you are doing. Name takes 
precedence over time because it is easier to remember a name than a time. I 
implemented this order in the updated patch.

+ 			recoveryTargetName = pstrdup(item->value);

I also added a check for long names.

+ 	if ((record->xl_rmid == RM_XLOG_ID) && (record_info == XLOG_RESTORE_POINT))
+ 		couldStop = true;
+
+ 	if (!couldStop)
+ 		return false;
+

I reworked this code path because it seems confusing.

+ 		recordNamedRestorePoint = (xl_named_restore_points *) XLogRecGetData(record);
+ 		recordXtime = recordNamedRestorePoint->xtime;

Why don't you store the named restore point here too? You will need it a few 
lines below.

+ 		char name[MAXFNAMELEN];
+
+ 		memcpy(&xlrec, rec, sizeof(xl_named_restore_points));
+ 		strncpy(name, xlrec.name, MAXFNAMELEN);

Is it really necessary? I removed it.

+ Datum
+ pg_create_restore_point(PG_FUNCTION_ARGS)
+ {

You should have added a check for long restore point names. Added in the 
updated patch.

+         ereport(NOTICE,
+                  (errmsg("WAL archiving is not enabled; you must ensure that 
WAL segments are copied through other means for restore points to be usefull 
for you")));
+

Sentence was rewritten as "WAL archiving is not enabled; you must ensure that 
WAL segments are copied through other means to recover up to named restore point".

Finally, this is a nice feature iif we have a way to know what named restore 
points are available. DBAs need to take note of this list (that is not good) 
and the lazy ones will have a hard time to recover the right name (possibly 
with a xlog dump tool).

So how could we store this information? Perhaps a file in 
$PGDATA/pg_xlog/restore_label that contains the label (and possibly the WAL 
location). Also it must have a way to transmit the restore_label when we add 
another restore point. I didn't implement this part (Jaime?) and it seems as 
important as the new xlog record type that is in the patch. It seems 
complicate but I don't have ideas. Anyone? The restore point names could be 
obtained by querying a function (say, pg_restore_point_names or 
pg_restore_point_list).

Someone could argue that this feature could be reached if we store label and 
WAL location in a file (say restore_label). This mechanism doesn't need a new 
WAL record but the downside is that if we lost restore_label we are dead. I'm 
not in favor of this approach because it seems too fragile.

I will mark this patch waiting on author because of those open issues.

This patch needs to bump catalog version because of the new function. I'm not 
sure if the new record type requires bumping the xlog magic number.

I'm attaching the updated patch and two scripts that I used to play with the 
patch.


-- 
   Euler Taveira de Oliveira
   http://www.timbira.com/

Attachment: nrp.diff.gz
Description: application/x-gzip (5.4 KB)
Attachment: b.sh
Description: application/x-sh (886 bytes)
Attachment: a.sh
Description: application/x-sh (1.9 KB)

In response to

Responses

pgsql-hackers by date

Next:From: Peter EisentrautDate: 2011-02-01 15:11:16
Subject: Re: [pgsql-general 2011-1-21:] Are there any projects interested in object functionality? (+ rule bases)
Previous:From: Pavel StehuleDate: 2011-02-01 14:12:45
Subject: Re: [pgsql-general 2011-1-21:] Are there any projects interested in object functionality? (+ rule bases)

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group