automatic restore point

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
30 messages Options
12
Reply | Threaded
Open this post in threaded view
|

Re: automatic restore point

Pavel Stehule


2018-08-31 10:14 GMT+02:00 Yotsunaga, Naoki <[hidden email]>:
-----Original Message-----
From: Yotsunaga, Naoki [mailto:[hidden email]]
Sent: Tuesday, June 26, 2018 10:18 AM
To: Postgres hackers <[hidden email]>
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?

I think it is useful and simple.

Regards

Pavel
 
------
Naoki Yotsunaga





Reply | Threaded
Open this post in threaded view
|

RE: automatic restore point

Yotsunaga, Naoki
In reply to this post by Yotsunaga, Naoki
-----Original Message-----
From: Yotsunaga, Naoki [mailto:[hidden email]]
Sent: Tuesday, June 26, 2018 10:18 AM
To: Postgres hackers <[hidden email]>
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





automatic_restore_point_v2.patch (3K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: automatic restore point

Peter Eisentraut-6
On 06/09/2018 02:16, Yotsunaga, Naoki wrote:
> -----Original Message-----
> From: Yotsunaga, Naoki [mailto:[hidden email]]
> Sent: Tuesday, June 26, 2018 10:18 AM
> To: Postgres hackers <[hidden email]>
> 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.

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

Reply | Threaded
Open this post in threaded view
|

Re: automatic restore point

Michael Paquier-2
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

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: automatic restore point

Alvaro Herrera-9
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

Reply | Threaded
Open this post in threaded view
|

Re: automatic restore point

Alvaro Herrera-9
In reply to this post by Jaime Casanova-4
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

Reply | Threaded
Open this post in threaded view
|

Re: automatic restore point

Peter Eisentraut-6
In reply to this post by Alvaro Herrera-9
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

Reply | Threaded
Open this post in threaded view
|

Re: automatic restore point

Alvaro Herrera-9
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

Reply | Threaded
Open this post in threaded view
|

Re: automatic restore point

Peter Eisentraut-6
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

0001-Test-that-event-triggers-work-in-functions-and-proce.patch (4K) Download Attachment
0002-Slightly-correct-context-check-for-event-triggers.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: automatic restore point

Peter Eisentraut-6
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

12