automatic restore point
Hi, I'm a newbie to the hackers but I'd like to propose the "automatic restore point" feature.
This feature automatically create backup label just before making a huge change to DB. It's useful when this change is accidental case.
The following is a description of "automatic restore point".
【Background】
When DBA's operation failure, for example DBA accidently drop table, the database is restored from the file system backup and recovered by using time or transaction ID. The transaction ID is identified from WAL.
But below are the following problems in using time or transaction ID.
-Time
・Need to memorize the time of failure operation.
(It is possible to identify the time from WAL. But it takes time and effort to identify the time.)
・Difficult to specify detail point.
-Transaction ID
・It takes time and effort to identify the transaction ID.
In order to solve the above problem,
I'd like propose a feature to implement automatic recording function of recovery point.
【Feature Description】
In PostgreSQL, there is a backup control function "pg_create_restore_point()".
User can create a named point for performing restore by using "pg_create_restore_point()".
And user can recover by using the named point.
So, execute "pg_create_restore_point()" automatically before executing the following command to create a point for performing restore(recovery point).
The name of recovery point is the date and time when the command was executed.
In this operation, target resource (database name, table name) and recovery point name are output as a message to PostgreSQL server log.
- Commands wherein this feature can be appended
・TRUNCATE
・DROP
・DELETE(Without WHERE clause)
・UPDATE(Without WHERE clause)
・COPY FROM
【How to use】
1) When executing the above command, identify the command and recovery point name that matches the resource indicating the operation failure from the server log.
ex)Message for executing TRUNCATE at 2018/6/1 12:30:30 (database name:testdb, table name:testtb)
set recovery point. operation = 'truncate'
database = 'testdb' relation = 'testtb' recovery_point_name = '2018-06-01-12:30:30'
2) Implement PostgreSQL document '25 .3.4.Recovering Using a Continuous Archive Backup.'
※Set "recovery_target_name = 'recovery_point name'" at recovery.conf.
【Setting file】
Set postgres.conf.
auto_create_restore_point = on # Switch on/off automatic recording function of recovery point. The default value is 'off'.
So what do you think about it? Do you think is it useful?
Also, when recovering with the current specification, tables other than the returned table also return to the state of the specified recovery point.
So, I’m looking for ways to recover only specific tables. Do you have any ideas?
------
Naoki Yotsunaga
On Mon, Jun 25, 2018 at 6:17 PM, Yotsunaga, Naoki <
yotsunaga.naoki@jp.fujitsu.com> wrote:
So what do you think about it? Do you think is it useful?
The cost/benefit ratio seems low...
Also, when recovering with the current specification, tables other than the
returned table also return to the state of the specified recovery point.
So, I’m looking for ways to recover only specific tables. Do you have any
ideas?
...and this lowers it even further.
I'd rather spend effort making the initial execution of said commands less
likely. Something like:
TRUNCATE table YES_I_REALLY_WANT_TO_DO_THIS;
which will fail if you don't add the keyword "YES_I..." to the end of the
command and the system was setup to require it.
Or, less annoyingly:
BEGIN;
SET LOCAL perform_dangerous_action = true; --can we require local?
TRUNCATE table;
COMMIT;
David J.
On 25 June 2018 at 21:33, David G. Johnston <david.g.johnston@gmail.com>
wrote:
On Mon, Jun 25, 2018 at 6:17 PM, Yotsunaga, Naoki <
yotsunaga.naoki@jp.fujitsu.com> wrote:
So what do you think about it? Do you think is it useful?
I'd rather spend effort making the initial execution of said commands less
likely. Something like:TRUNCATE table YES_I_REALLY_WANT_TO_DO_THIS;
I think an optional setting making DELETE and UPDATE without a WHERE clause
illegal would be handy. Obviously this would have to be optional for
backward compatibility. Perhaps even just a GUC setting, with the intent
being that one would set it in .psqlrc so that omitting the WHERE clause at
the command line would just be a syntax error. If one actually does need to
affect the whole table one can just say WHERE TRUE. For applications, which
presumably have their SQL queries tightly controlled and pre-written
anyway, this would most likely not be particularly useful.
Why not use auto commit off in the session or .psqlrc file or begin and then use rollback? \set AUTOCOMMIT off
What would be nice is if a syntax error didn’t abort the transaction when auto commit is off — being a bad typist.
On Tue, Jun 26, 2018 at 12:04:59AM -0400, Rui DeSousa wrote:
Why not use auto commit off in the session or .psqlrc file or begin and then use rollback? \set AUTOCOMMIT off
What would be nice is if a syntax error didn’t abort the transaction when auto commit is off — being a bad typist.
I think you'll get that behavior with ON_ERROR_ROLLBACK.
Justin
On Jun 26, 2018, at 12:37 AM, Justin Pryzby <pryzby@telsasoft.com> wrote:
I think you'll get that behavior with ON_ERROR_ROLLBACK.
Awesome. Thanks!
On Mon, Jun 25, 2018 at 11:01:06PM -0400, Isaac Morland wrote:
I think an optional setting making DELETE and UPDATE without a WHERE clause
illegal would be handy. Obviously this would have to be optional for
backward compatibility. Perhaps even just a GUC setting, with the intent
being that one would set it in .psqlrc so that omitting the WHERE clause at
the command line would just be a syntax error. If one actually does need to
affect the whole table one can just say WHERE TRUE. For applications, which
presumably have their SQL queries tightly controlled and pre-written
anyway, this would most likely not be particularly useful.
There was a patch doing exactly that which was discussed last year:
https://commitfest.postgresql.org/13/948/
/messages/by-id/20160721045746.GA25043@fetter.org
What was proposed was rather limiting though, see my messages on the
thread. Using a hook, that's simple enough to develop an extension
which does that.
--
Michael
On Tue, Jun 26, 2018 at 01:17:31AM +0000, Yotsunaga, Naoki wrote:
The following is a description of "automatic restore point".
【Background】
When DBA's operation failure, for example DBA accidently drop table,
the database is restored from the file system backup and recovered by
using time or transaction ID. The transaction ID is identified from
WAL.In order to solve the above problem,
I'd like propose a feature to implement automatic recording function
of recovery point.
There is also recovery_target_lsn which is new as of v10. This
parameter is way better than having to track down time or XID, which is
a reason why I developped it. Please note that this is also one of the
reasons why it is possible to delay WAL replays on standbys, so as an
operator has room to fix such operator errors. Having of course cold
backups with a proper WAL archive and a correct retention policy never
hurts.
【Setting file】
Set postgres.conf.
auto_create_restore_point = on # Switch on/off automatic recording
function of recovery point. The default value is 'off'.So what do you think about it? Do you think is it useful?
So basically what you are looking for here is a way to enforce a restore
point to be created depending on a set of pre-defined conditions? How
would you define and choose those?
Also, when recovering with the current specification, tables other
than the returned table also return to the state of the specified
recovery point.
So, I’m looking for ways to recover only specific tables. Do you have
any ideas?
Why not using the utility hook which filters out for commands you'd
like to forbid, in this case TRUNCATE or a DROP TABLE on a given
relation? Or why not simply using an event trigger at your application
level so as you can actually *prevent* the error to happen first? With
the last option you don't have to write C code, but this would not
filter TRUNCATE. In short, what you propose looks over-complicated to
me and there are options on the table which allow the problem you are
trying to solve to not happen at all. You could also use the utility
hook to log or register somewhere hte XID/time/LSN associated to a given
command and then use it as your restore point. This could also happen
out of core.
--
Michael
Hi. Thanks for comments.
Explanation of the background of the function proposal was inadequate.
So, I explain again.
I assume the following situation.
User needs to make a quick, seemingly simple fix to an important production database. User composes the query, gives it an once-over, and lets it run. Seconds later user realizes that user forgot the WHERE clause, dropped the wrong table, or made another serious mistake, and interrupts the query, but the damage has been done.
Also user did not record the time and did not look at a lsn position.
Certainly, I thought about reducing the possibility of executing the wrong command, but I thought that the possibility could not be completely eliminated.
So I proposed the “automatic restore point”.
With this function, user can recover quickly and reliably even if you perform a failure operation.
I'd rather spend effort making the initial execution of said commands less likely.
I think that the function to prohibit DELETE and UPDATE without a WHERE clause in the later response is good way.
But I think that it is impossible to completely eliminate the failure of the other commands.
For example, drop the wrong table.
-----
Naoki Yotsunaga
-----Original Message-----
From: Michael Paquier [mailto:michael@paquier.xyz]
Sent: Tuesday, June 26, 2018 2:16 PM
To: Isaac Morland <isaac.morland@gmail.com>
Cc: David G. Johnston <david.g.johnston@gmail.com>; Yotsunaga, Naoki/四ツ永 直輝 <yotsunaga.naoki@jp.fujitsu.com>; Postgres hackers <pgsql-hackers@postgresql.org>
Subject: Re: automatic restore point
On Mon, Jun 25, 2018 at 11:01:06PM -0400, Isaac Morland wrote:
I think an optional setting making DELETE and UPDATE without a WHERE
clause illegal would be handy. Obviously this would have to be
optional for backward compatibility. Perhaps even just a GUC setting,
with the intent being that one would set it in .psqlrc so that
omitting the WHERE clause at the command line would just be a syntax
error. If one actually does need to affect the whole table one can
just say WHERE TRUE. For applications, which presumably have their SQL
queries tightly controlled and pre-written anyway, this would most likely not be particularly useful.
There was a patch doing exactly that which was discussed last year:
https://commitfest.postgresql.org/13/948/
/messages/by-id/20160721045746.GA25043@fetter.org
What was proposed was rather limiting though, see my messages on the thread. Using a hook, that's simple enough to develop an extension which does that.
--
Michael
Hi. Thanks for comments.
There is also recovery_target_lsn which is new as of v10.
In this method, it is necessary to look at a lsn position before operating.
But I assume the user who did not look it before operating.
So I think that this method is not appropriate.
So basically what you are looking for here is a way to enforce a restore point to be created depending on a set of pre-defined conditions?
How would you define and choose those?
I understand that I was asked how to set up a command to apply this function.
Ex) DROP = on
TRUNCATE = off
Is my interpretation right?
If my interpretation is correct, all the above commands will be applied.
When this function is turned on, this function works when all the above commands are executed.
-------
Naoki Yotsynaga
-----Original Message-----
From: Michael Paquier [mailto:michael@paquier.xyz]
Sent: Tuesday, June 26, 2018 2:31 PM
To: Yotsunaga, Naoki/四ツ永 直輝 <yotsunaga.naoki@jp.fujitsu.com>
Cc: Postgres hackers <pgsql-hackers@postgresql.org>
Subject: Re: automatic restore point
On Tue, Jun 26, 2018 at 01:17:31AM +0000, Yotsunaga, Naoki wrote:
The following is a description of "automatic restore point".
【Background】
When DBA's operation failure, for example DBA accidently drop table,
the database is restored from the file system backup and recovered by
using time or transaction ID. The transaction ID is identified from
WAL.In order to solve the above problem,
I'd like propose a feature to implement automatic recording function
of recovery point.
There is also recovery_target_lsn which is new as of v10. This parameter is way better than having to track down time or XID, which is a reason why I developped it. Please note that this is also one of the reasons why it is possible to delay WAL replays on standbys, so as an operator has room to fix such operator errors. Having of course cold backups with a proper WAL archive and a correct retention policy never hurts.
【Setting file】
Set postgres.conf.
auto_create_restore_point = on # Switch on/off automatic recording
function of recovery point. The default value is 'off'.So what do you think about it? Do you think is it useful?
So basically what you are looking for here is a way to enforce a restore point to be created depending on a set of pre-defined conditions? How would you define and choose those?
Also, when recovering with the current specification, tables other
than the returned table also return to the state of the specified
recovery point.
So, I’m looking for ways to recover only specific tables. Do you have
any ideas?
Why not using the utility hook which filters out for commands you'd like to forbid, in this case TRUNCATE or a DROP TABLE on a given relation? Or why not simply using an event trigger at your application level so as you can actually *prevent* the error to happen first? With the last option you don't have to write C code, but this would not filter TRUNCATE. In short, what you propose looks over-complicated to me and there are options on the table which allow the problem you are trying to solve to not happen at all. You could also use the utility hook to log or register somewhere hte XID/time/LSN associated to a given command and then use it as your restore point. This could also happen out of core.
--
Michael
On Tue, Jul 03, 2018 at 01:07:41AM +0000, Yotsunaga, Naoki wrote:
There is also recovery_target_lsn which is new as of v10.
In this method, it is necessary to look at a lsn position before operating.
But I assume the user who did not look it before operating.
So I think that this method is not appropriate.
You should avoid top-posting on the mailing lists, this breaks the
consistency of the thread.
So basically what you are looking for here is a way to enforce a
restore point to be created depending on a set of pre-defined
conditions? How would you define and choose those?I understand that I was asked how to set up a command to apply this function.
Ex) DROP = on
TRUNCATE = off
Is my interpretation right?
If my interpretation is correct, all the above commands will be
applied.
When this function is turned on, this function works when all the
above commands are executed.
Yeah, but based on which factors are you able to define that such
conditions are enough to say that this feature is fully-compliant with
user's need, and how can you be sure that this is not going to result in
an additional maintenance burden if you need to define a new set of
conditions in the future. For example an operator has issued a costly
ALTER TABLE which causes a full table rewrite, which could be also an
operation that you would like to prevent. Having a set of GUCs which
define such low-level behavior is not really user-friendly.
--
Michael
On Tue, Jul 03, 2018 at 01:06:31AM +0000, Yotsunaga, Naoki wrote:
I'd rather spend effort making the initial execution of said commands
less likely.I think that the function to prohibit DELETE and UPDATE without a
WHERE clause in the later response is good way.
This has popped up already in the lists in the past.
But I think that it is impossible to completely eliminate the failure
of the other commands. For example, drop the wrong table.
This kind of thing is heavily application-dependent. For example, you
would likely not care if an operator, who has newly-joined the team in
charge of the maintenance of this data, drops unfortunately a table
which includes logs from 10 years back, and you would very likely care
about a table dropped which has user's login data. My point is that you
need to carefully design the shape of the configuration you would use,
so as any application's admin would be able to cope with it, for example
allowing exclusion filters with regular expressions could be a good idea
to dig into. And also you need to think about it so as it is backward
compatible.
--
Michael
On Mon, 2 Jul 2018 at 20:07, Yotsunaga, Naoki
<yotsunaga.naoki@jp.fujitsu.com> wrote:
Hi. Thanks for comments.
Explanation of the background of the function proposal was inadequate.
So, I explain again.I assume the following situation.
User needs to make a quick, seemingly simple fix to an important production database. User composes the query, gives it an once-over, and lets it run. Seconds later user realizes that user forgot the WHERE clause, dropped the wrong table, or made another serious mistake, and interrupts the query, but the damage has been done.
Also user did not record the time and did not look at a lsn position.
Thinking on Michael's suggestion of using event triggers, you can
create an event trigger to run pg_create_restore_point() on DROP,
here's a simple example of how that should like:
https://www.postgresql.org/docs/current/static/functions-event-triggers.html
You can also create a normal trigger BEFORE TRUNCATE to create a
restore point just before running the TRUNCATE command.
Those would run *on the background* (you don't need to call them
manually), you can use them right now, won't affect performance for
people not wanting this "functionality".
BTW, Michael's suggestion also included the idea of recording
xid/time/lsn which can be done through triggers too
--
Jaime Casanova www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
-----Original Message-----
From: Michael Paquier [mailto:michael@paquier.xyz]
Sent: Tuesday, July 3, 2018 10:22 AM
This kind of thing is heavily application-dependent. For example, you would likely not care if an operator, who has newly-joined the team in >charge of the maintenance of this data, drops unfortunately a table which includes logs from 10 years back, and you would very likely care >about a table dropped which has user's login data. My point is that you need to carefully design the shape of the configuration you would use, >so as any application's admin would be able to cope with it, for example allowing exclusion filters with regular expressions could be a good >idea to dig into. And also you need to think about it so as it is backward compatible.
Thanks for comments.
Does that mean that the application (user) is interested in which table?
For example, there are two tables A. It is ok even if one table disappears, but it is troubled if another table B disappears. So, when the table B is dropped, automatic restore point works. In the table A, automatic restore point does not work.
So, it is difficult to implement that automatic restore point in postgresql by default.
Is my interpretation right?
---
Naoki Yotsunaga
-----Original Message-----
From: Jaime Casanova [mailto:jaime.casanova@2ndquadrant.com]
Sent: Tuesday, July 3, 2018 11:06 AM
Thinking on Michael's suggestion of using event triggers, you can create an event >trigger to run pg_create_restore_point() on DROP, here's a simple example of how >that should like:
https://www.postgresql.org/docs/current/static/functions-event-triggers.html
You can also create a normal trigger BEFORE TRUNCATE to create a restore point just >before running the TRUNCATE command.
Thanks for comments.
I was able to understand.
---
Naoki Yotsunaga
-----Original Message-----
From: Yotsunaga, Naoki [mailto:yotsunaga.naoki@jp.fujitsu.com]
Sent: Friday, July 6, 2018 5:05 PM
Does that mean that the application (user) is interested in which table?
For example, there are two tables A. It is ok even if one table disappears, but it is troubled if another table B disappears. So, when the table B is dropped, automatic restore point works. In the table A, automatic restore point does not work.
So, it is difficult to implement that automatic restore point in postgresql by default.
Is my interpretation right?
I want to hear about the following in addition to the previous comment.
What would you do if your customer dropped the table and asked you to restore it?
Everyone is thinking what to do to avoid operation failure, but I’m thinking about after the user’s failure.
What I mean is that not all users will set up in advance.
For example, if you make the settings described in the manual, you will not drop the table by operation failure. However, not all users do that setting.
For such users, I think that it is necessary to have a function to easily restore data after failing operation without setting anything in advance.
So I proposed this function.
---
Naoki Yotsunaga
On Wed, Jul 11, 2018 at 06:11:01AM +0000, Yotsunaga, Naoki wrote:
I want to hear about the following in addition to the previous
comment. What would you do if your customer dropped the table and asked you to
restore it?
I can think of 4 reasons on top of my mind:
1) Don't do that.
2) Implement safe-guards using utility hooks or event triggers.
3) Have a delayed standby if you don't believe that your administrators
are skilled enough in case.
4) Have backups and a WAL archive.
Everyone is thinking what to do to avoid operation failure, but I’m
thinking about after the user’s failure.
What I mean is that not all users will set up in advance.
For example, if you make the settings described in the manual, you
will not drop the table by operation failure. However, not all users
do that setting.
For such users, I think that it is necessary to have a function to
easily restore data after failing operation without setting anything
in advance. So I proposed this function.
Well, if you put in place correct measures from the start you would not
have problems. It seems to me that there is no point in implementing
something which is a solution for a very narrow case, where the user has
shot his own foot to begin with. Having backups anyway is mandatory by
the way, standby replicas are not backups.
--
Michael
-----Original Message-----
From: Michael Paquier [mailto:michael@paquier.xyz]
Sent: Wednesday, July 11, 2018 3:34 PM
Well, if you put in place correct measures from the start you would not have problems.
It seems to me that there is no point in implementing something which is a solution for a very narrow case, where the user has shot his own foot to begin with.
Having backups anyway is mandatory by the way, standby replicas are not backups.
I think that the Undo function of AWS and Oracle's Flashback function are to save such users, and it is a function to prevent human error.
So, how about postgres implementing such a function?
Also, as an approach to achieving the goal, I thought about outputting lsn to the server log when a specific command was executed.
I do not think the source code of postgres will be complicated when implementing this function.
Do you feel it is too complicated?
-------
Naoki Yotsunaga
On Fri, Jul 13, 2018 at 08:16:00AM +0000, Yotsunaga, Naoki wrote:
Do you feel it is too complicated?
In short, yes.
--
Michael
-----Original Message-----
From: Yotsunaga, Naoki [mailto:yotsunaga.naoki@jp.fujitsu.com]
Sent: Tuesday, June 26, 2018 10:18 AM
To: Postgres hackers <pgsql-hackers@postgresql.org>
Subject: automatic restore point
Hi, I attached a patch to output the LSN before execution to the server log when executing a specific command and accidentally erasing data.
A detailed background has been presented before.
In short explain: After the DBA's operation failure and erases the data, it is necessary to perform PITR immediately.
Since it is not possible to easily obtain information for doing the current PITR, I would like to solve it.
The specification has changed from the first proposal.
-Target command
DROP TABLE
TRUNCATE
-Setting file
postgresql.conf
log_recovery_points = on #default value is 'off'. When the switch is turned on, LSN is output to the server log when DROP TABLE, TRUNCATE is executed.
-How to use
1) When executing the above command, identify the command and recovery point that matches the resource indicating the operation failure from the server log.
ex) LOG: recovery_point_lsn: 0/201BB70
STATEMENT: drop table test ;
2) Implement PostgreSQL document '25 .3.4.Recovering Using a Continuous Archive Backup.'
*Set "recovery_target_lsn = 'recovery_point_lsn'" at recovery.conf.
Although there was pointed out that the source becomes complicated in the past, we could add the function by adding about 20 steps.
What do you think about it? Do you think is it useful?
------
Naoki Yotsunaga
Attachments:
automatic_restore_point.patchapplication/octet-stream; name=automatic_restore_point.patchDownload
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index c633e11..5da8b57 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -100,6 +100,7 @@ bool wal_compression = false;
char *wal_consistency_checking_string = NULL;
bool *wal_consistency_checking = NULL;
bool log_checkpoints = false;
+bool log_recovery_points = false;
int sync_method = DEFAULT_SYNC_METHOD;
int wal_level = WAL_LEVEL_MINIMAL;
int CommitDelay = 0; /* precommit delay in microseconds */
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index bdfb66f..e6cf576 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -86,6 +86,8 @@ static void ProcessUtilitySlow(ParseState *pstate,
char *completionTag);
static void ExecDropStmt(DropStmt *stmt, bool isTopLevel);
+bool Log_recovery_points = false;
+
/*
* CommandIsReadOnly: is an executable query read-only?
@@ -538,6 +540,9 @@ standard_ProcessUtility(PlannedStmt *pstmt,
break;
case T_TruncateStmt:
+ if (log_recovery_points)
+ ereport(LOG,
+ (errmsg("recovery_point_lsn: 0/%X", (int) ProcLastRecPtr)));
ExecuteTruncate((TruncateStmt *) parsetree);
break;
@@ -1718,6 +1723,9 @@ ExecDropStmt(DropStmt *stmt, bool isTopLevel)
/* fall through */
case OBJECT_TABLE:
+ if (log_recovery_points)
+ ereport(LOG,
+ (errmsg("recovery_point_lsn: 0/%X", (int) ProcLastRecPtr)));
case OBJECT_SEQUENCE:
case OBJECT_VIEW:
case OBJECT_MATVIEW:
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 7cd2d2d..fa48393 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -1113,6 +1113,15 @@ static struct config_bool ConfigureNamesBool[] =
NULL, NULL, NULL
},
{
+ {"log_recovery_points", PGC_SIGHUP, LOGGING_WHAT,
+ gettext_noop("Logs each LSN."),
+ NULL
+ },
+ &log_recovery_points,
+ false,
+ NULL, NULL, NULL
+ },
+ {
{"log_connections", PGC_SU_BACKEND, LOGGING_WHAT,
gettext_noop("Logs each successful connection."),
NULL
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 3d88e80..c3f25c3 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -455,6 +455,7 @@
#debug_print_plan = off
#debug_pretty_print = on
#log_checkpoints = off
+#log_recovery_points = off
#log_connections = off
#log_disconnections = off
#log_duration = off
diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h
index 421ba6d..5420e84 100644
--- a/src/include/access/xlog.h
+++ b/src/include/access/xlog.h
@@ -109,6 +109,7 @@ extern bool wal_compression;
extern bool *wal_consistency_checking;
extern char *wal_consistency_checking_string;
extern bool log_checkpoints;
+extern bool log_recovery_points;
extern int CheckPointSegments;
diff --git a/src/test/regress/regression.diffs b/src/test/regress/regression.diffs
new file mode 100644
index 0000000..e69de29
diff --git a/src/test/regress/regression.out b/src/test/regress/regression.out
new file mode 100644
index 0000000..e69de29
2018-08-31 10:14 GMT+02:00 Yotsunaga, Naoki <yotsunaga.naoki@jp.fujitsu.com>
:
-----Original Message-----
From: Yotsunaga, Naoki [mailto:yotsunaga.naoki@jp.fujitsu.com]
Sent: Tuesday, June 26, 2018 10:18 AM
To: Postgres hackers <pgsql-hackers@postgresql.org>
Subject: automatic restore pointHi, I attached a patch to output the LSN before execution to the server
log when executing a specific command and accidentally erasing data.A detailed background has been presented before.
In short explain: After the DBA's operation failure and erases the data,
it is necessary to perform PITR immediately.
Since it is not possible to easily obtain information for doing the
current PITR, I would like to solve it.The specification has changed from the first proposal.
-Target command
DROP TABLE
TRUNCATE-Setting file
postgresql.conf
log_recovery_points = on #default value is 'off'. When the switch is
turned on, LSN is output to the server log when DROP TABLE, TRUNCATE is
executed.-How to use
1) When executing the above command, identify the command and recovery
point that matches the resource indicating the operation failure from the
server log.
ex) LOG: recovery_point_lsn: 0/201BB70
STATEMENT: drop table test ;
2) Implement PostgreSQL document '25 .3.4.Recovering Using a Continuous
Archive Backup.'
*Set "recovery_target_lsn = 'recovery_point_lsn'" at recovery.conf.Although there was pointed out that the source becomes complicated in the
past, we could add the function by adding about 20 steps.What do you think about it? Do you think is it useful?
I think it is useful and simple.
Regards
Pavel
Show quoted text
------
Naoki Yotsunaga
-----Original Message-----
From: Yotsunaga, Naoki [mailto:yotsunaga.naoki@jp.fujitsu.com]
Sent: Tuesday, June 26, 2018 10:18 AM
To: Postgres hackers <pgsql-hackers@postgresql.org>
Subject: automatic restore point
Hi, I attached a patch to output the LSN before execution to the server log >when executing a specific command and accidentally erasing data.
Since there was an error in the attached patch, I attached the modified patch.
------
Naoki Yotsunaga
Attachments:
automatic_restore_point_v2.patchapplication/octet-stream; name=automatic_restore_point_v2.patchDownload
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index c633e11..5da8b57 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -100,6 +100,7 @@ bool wal_compression = false;
char *wal_consistency_checking_string = NULL;
bool *wal_consistency_checking = NULL;
bool log_checkpoints = false;
+bool log_recovery_points = false;
int sync_method = DEFAULT_SYNC_METHOD;
int wal_level = WAL_LEVEL_MINIMAL;
int CommitDelay = 0; /* precommit delay in microseconds */
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index bdfb66f..e6cf576 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -86,6 +86,8 @@ static void ProcessUtilitySlow(ParseState *pstate,
char *completionTag);
static void ExecDropStmt(DropStmt *stmt, bool isTopLevel);
+bool Log_recovery_points = false;
+
/*
* CommandIsReadOnly: is an executable query read-only?
@@ -538,6 +540,9 @@ standard_ProcessUtility(PlannedStmt *pstmt,
break;
case T_TruncateStmt:
+ if (log_recovery_points)
+ ereport(LOG,
+ (errmsg("recovery_point_lsn: 0/%X", (int) ProcLastRecPtr)));
ExecuteTruncate((TruncateStmt *) parsetree);
break;
@@ -1718,6 +1723,9 @@ ExecDropStmt(DropStmt *stmt, bool isTopLevel)
/* fall through */
case OBJECT_TABLE:
+ if (log_recovery_points)
+ ereport(LOG,
+ (errmsg("recovery_point_lsn: 0/%X", (int) ProcLastRecPtr)));
case OBJECT_SEQUENCE:
case OBJECT_VIEW:
case OBJECT_MATVIEW:
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 7cd2d2d..fa48393 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -1113,6 +1113,15 @@ static struct config_bool ConfigureNamesBool[] =
NULL, NULL, NULL
},
{
+ {"log_recovery_points", PGC_SIGHUP, LOGGING_WHAT,
+ gettext_noop("Logs each LSN."),
+ NULL
+ },
+ &log_recovery_points,
+ false,
+ NULL, NULL, NULL
+ },
+ {
{"log_connections", PGC_SU_BACKEND, LOGGING_WHAT,
gettext_noop("Logs each successful connection."),
NULL
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 3d88e80..c3f25c3 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -455,6 +455,7 @@
#debug_print_plan = off
#debug_pretty_print = on
#log_checkpoints = off
+#log_recovery_points = off
#log_connections = off
#log_disconnections = off
#log_duration = off
diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h
index 421ba6d..5420e84 100644
--- a/src/include/access/xlog.h
+++ b/src/include/access/xlog.h
@@ -109,6 +109,7 @@ extern bool wal_compression;
extern bool *wal_consistency_checking;
extern char *wal_consistency_checking_string;
extern bool log_checkpoints;
+extern bool log_recovery_points;
extern int CheckPointSegments;
On 06/09/2018 02:16, Yotsunaga, Naoki wrote:
-----Original Message-----
From: Yotsunaga, Naoki [mailto:yotsunaga.naoki@jp.fujitsu.com]
Sent: Tuesday, June 26, 2018 10:18 AM
To: Postgres hackers <pgsql-hackers@postgresql.org>
Subject: automatic restore pointHi, I attached a patch to output the LSN before execution to the server log >when executing a specific command and accidentally erasing data.
Since there was an error in the attached patch, I attached the modified patch.
I think this should be done using event triggers. Right now, you just
have it hardcoded to TRUNCATE and DROP TABLE, which seems somewhat
arbitrary. With event triggers, you have the full flexibility to do
what you want. You can pick which commands to apply it to, you can log
the LSN, you can create restore points, etc.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Fri, Sep 28, 2018 at 09:13:17PM +0200, Peter Eisentraut wrote:
I think this should be done using event triggers. Right now, you just
have it hardcoded to TRUNCATE and DROP TABLE, which seems somewhat
arbitrary. With event triggers, you have the full flexibility to do
what you want. You can pick which commands to apply it to, you can log
the LSN, you can create restore points, etc.
I still unfortunately don't see what this patch brings more that you
cannot do. Event triggers are particularly useful in this prospective,
so I am marking the patch as rejected.
--
Michael
On 2018-Sep-30, Michael Paquier wrote:
On Fri, Sep 28, 2018 at 09:13:17PM +0200, Peter Eisentraut wrote:
I think this should be done using event triggers. Right now, you just
have it hardcoded to TRUNCATE and DROP TABLE, which seems somewhat
arbitrary. With event triggers, you have the full flexibility to do
what you want. You can pick which commands to apply it to, you can log
the LSN, you can create restore points, etc.I still unfortunately don't see what this patch brings more that you
cannot do. Event triggers are particularly useful in this prospective,
so I am marking the patch as rejected.
I don't see it as clear cut as all that ... particularly considering
that a useful event trigger runs *after* the DDL command in question has
already written all its WAL, so such a restore point would be completely
useless. (Or are ddl_command_start event triggers useful enough?)
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 2018-Jul-02, Jaime Casanova wrote:
You can also create a normal trigger BEFORE TRUNCATE to create a
restore point just before running the TRUNCATE command.
On every single table?
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 01/10/2018 05:34, Alvaro Herrera wrote:
I don't see it as clear cut as all that ... particularly considering
that a useful event trigger runs *after* the DDL command in question has
already written all its WAL, so such a restore point would be completely
useless. (Or are ddl_command_start event triggers useful enough?)
The following appears to work:
CREATE FUNCTION evt_automatic_restart_point() RETURNS event_trigger
LANGUAGE plpgsql
AS $$
BEGIN
PERFORM pg_create_restore_point(tg_tag);
END
$$;
CREATE EVENT TRIGGER automatic_restart_point ON ddl_command_start
EXECUTE PROCEDURE evt_automatic_restart_point();
Are there any concerns about this?
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 2018-Oct-01, Peter Eisentraut wrote:
The following appears to work:
CREATE FUNCTION evt_automatic_restart_point() RETURNS event_trigger
LANGUAGE plpgsql
AS $$
BEGIN
PERFORM pg_create_restore_point(tg_tag);
END
$$;CREATE EVENT TRIGGER automatic_restart_point ON ddl_command_start
EXECUTE PROCEDURE evt_automatic_restart_point();Are there any concerns about this?
Mumble.
Re-reading the implementation in standard_ProcessUtility, I wonder what
is PROCESS_UTILITY_QUERY_NONATOMIC -- there seems to be a maze through
SPI that determines whether this flag is set or not, which could affect
whether the event trigger is useful. Are utilities executed through a
procedure detected by event triggers? If so, then this mechanism seems
good enough to me. But if there's a way to sneak utility commands (DROP
TABLE) without the event trigger being invoked, then no (and in that
case maybe it's just a bug in procedures and we can still not include
this patch).
(Grepping for "atomic" is unsurprisingly unhelpful. I really wish we
didn't use plain words as struct member names ...)
On the TRUNCATE case it's a bit annoying that you can't do it with event
triggers -- you have to create individual regular triggers on truncate
for each table (so you probably need yet another event trigger that
creates such triggers).
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 02/10/2018 00:06, Alvaro Herrera wrote:
Re-reading the implementation in standard_ProcessUtility, I wonder what
is PROCESS_UTILITY_QUERY_NONATOMIC -- there seems to be a maze through
SPI that determines whether this flag is set or not, which could affect
whether the event trigger is useful. Are utilities executed through a
procedure detected by event triggers? If so, then this mechanism seems
good enough to me. But if there's a way to sneak utility commands (DROP
TABLE) without the event trigger being invoked, then no (and in that
case maybe it's just a bug in procedures and we can still not include
this patch).
It looked for a moment that
isCompleteQuery = (context <= PROCESS_UTILITY_QUERY)
in ProcessUtilitySlow() might be a problem, since that omits
PROCESS_UTILITY_QUERY_NONATOMIC, but it's not actually a problem, since
the commands that run this way (CALL and SET from PL/pgSQL) don't have
event triggers. But anyway, I propose the attached patch to rephrase
that. Also some tests that show it all works as expected.
On the TRUNCATE case it's a bit annoying that you can't do it with event
triggers -- you have to create individual regular triggers on truncate
for each table (so you probably need yet another event trigger that
creates such triggers).
I don't see why we couldn't add event triggers on TRUNCATE or other
commands such as DELETE.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
0001-Test-that-event-triggers-work-in-functions-and-proce.patchtext/plain; charset=UTF-8; name=0001-Test-that-event-triggers-work-in-functions-and-proce.patch; x-mac-creator=0; x-mac-type=0Download
From 7f86aabd89ce7d30873a3257f0187738d272d531 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Fri, 5 Oct 2018 15:20:01 +0200
Subject: [PATCH 1/2] Test that event triggers work in functions and procedures
---
src/test/regress/expected/event_trigger.out | 37 ++++++++++++++++++++-
src/test/regress/sql/event_trigger.sql | 21 +++++++++++-
2 files changed, 56 insertions(+), 2 deletions(-)
diff --git a/src/test/regress/expected/event_trigger.out b/src/test/regress/expected/event_trigger.out
index 6175a10d77..466c46e7f3 100644
--- a/src/test/regress/expected/event_trigger.out
+++ b/src/test/regress/expected/event_trigger.out
@@ -112,10 +112,45 @@ create table event_trigger_fire5 (a int);
NOTICE: test_event_trigger: ddl_command_start CREATE TABLE
NOTICE: test_event_trigger: ddl_command_start CREATE TABLE
NOTICE: test_event_trigger: ddl_command_end CREATE TABLE
+-- non-top-level command
+create function f1() returns int
+language plpgsql
+as $$
+begin
+ create table event_trigger_fire6 (a int);
+ return 0;
+end $$;
+NOTICE: test_event_trigger: ddl_command_start CREATE FUNCTION
+NOTICE: test_event_trigger: ddl_command_start CREATE FUNCTION
+NOTICE: test_event_trigger: ddl_command_end CREATE FUNCTION
+select f1();
+NOTICE: test_event_trigger: ddl_command_start CREATE TABLE
+NOTICE: test_event_trigger: ddl_command_start CREATE TABLE
+NOTICE: test_event_trigger: ddl_command_end CREATE TABLE
+ f1
+----
+ 0
+(1 row)
+
+-- non-top-level command
+create procedure p1()
+language plpgsql
+as $$
+begin
+ create table event_trigger_fire7 (a int);
+end $$;
+NOTICE: test_event_trigger: ddl_command_start CREATE PROCEDURE
+NOTICE: test_event_trigger: ddl_command_end CREATE PROCEDURE
+call p1();
+NOTICE: test_event_trigger: ddl_command_start CREATE TABLE
+NOTICE: test_event_trigger: ddl_command_start CREATE TABLE
+NOTICE: test_event_trigger: ddl_command_end CREATE TABLE
-- clean up
alter event trigger regress_event_trigger disable;
-drop table event_trigger_fire2, event_trigger_fire3, event_trigger_fire4, event_trigger_fire5;
+drop table event_trigger_fire2, event_trigger_fire3, event_trigger_fire4, event_trigger_fire5, event_trigger_fire6, event_trigger_fire7;
NOTICE: test_event_trigger: ddl_command_end DROP TABLE
+drop routine f1(), p1();
+NOTICE: test_event_trigger: ddl_command_end DROP ROUTINE
-- regress_event_trigger_end should fire on these commands
grant all on table event_trigger_fire1 to public;
NOTICE: test_event_trigger: ddl_command_end GRANT
diff --git a/src/test/regress/sql/event_trigger.sql b/src/test/regress/sql/event_trigger.sql
index 342aef6449..545a416252 100644
--- a/src/test/regress/sql/event_trigger.sql
+++ b/src/test/regress/sql/event_trigger.sql
@@ -106,9 +106,28 @@
reset session_replication_role;
-- fires all three
create table event_trigger_fire5 (a int);
+-- non-top-level command
+create function f1() returns int
+language plpgsql
+as $$
+begin
+ create table event_trigger_fire6 (a int);
+ return 0;
+end $$;
+select f1();
+-- non-top-level command
+create procedure p1()
+language plpgsql
+as $$
+begin
+ create table event_trigger_fire7 (a int);
+end $$;
+call p1();
+
-- clean up
alter event trigger regress_event_trigger disable;
-drop table event_trigger_fire2, event_trigger_fire3, event_trigger_fire4, event_trigger_fire5;
+drop table event_trigger_fire2, event_trigger_fire3, event_trigger_fire4, event_trigger_fire5, event_trigger_fire6, event_trigger_fire7;
+drop routine f1(), p1();
-- regress_event_trigger_end should fire on these commands
grant all on table event_trigger_fire1 to public;
base-commit: ff347f8aff04865680c19ffc818460bb2afaad5b
--
2.19.0
0002-Slightly-correct-context-check-for-event-triggers.patchtext/plain; charset=UTF-8; name=0002-Slightly-correct-context-check-for-event-triggers.patch; x-mac-creator=0; x-mac-type=0Download
From 6003ddd5dd7d9b0b36af7f852f2519c8e8ee670b Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Fri, 5 Oct 2018 15:20:32 +0200
Subject: [PATCH 2/2] Slightly correct context check for event triggers
The previous check for a "complete query" omitted the new
PROCESS_UTILITY_QUERY_NONATOMIC value. This didn't actually make a
difference in practice, because only CALL and SET from PL/pgSQL run in
this state, but it's more correct to include it anyway.
---
src/backend/tcop/utility.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index b5804f64ad..d0e2d84ea2 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -943,7 +943,7 @@ ProcessUtilitySlow(ParseState *pstate,
{
Node *parsetree = pstmt->utilityStmt;
bool isTopLevel = (context == PROCESS_UTILITY_TOPLEVEL);
- bool isCompleteQuery = (context <= PROCESS_UTILITY_QUERY);
+ bool isCompleteQuery = (context != PROCESS_UTILITY_SUBCOMMAND);
bool needCleanup;
bool commandCollected = false;
ObjectAddress address;
--
2.19.0
On 05/10/2018 15:26, Peter Eisentraut wrote:
It looked for a moment that
isCompleteQuery = (context <= PROCESS_UTILITY_QUERY)
in ProcessUtilitySlow() might be a problem, since that omits
PROCESS_UTILITY_QUERY_NONATOMIC, but it's not actually a problem, since
the commands that run this way (CALL and SET from PL/pgSQL) don't have
event triggers. But anyway, I propose the attached patch to rephrase
that. Also some tests that show it all works as expected.
I have committed these to master.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services