Bug 128 - DROP TABLE replicatedTable leaves the cluster in a bad state
Summary: DROP TABLE replicatedTable leaves the cluster in a bad state
Alias: None
Product: Slony-I
Classification: Unclassified
Component: stored procedures (show other bugs)
Version: 2.0
Hardware: PC Linux
: medium enhancement
Assignee: Jan Wieck
URL: http://github.com/wieck/slony1-engine...
Depends on:
Blocks: 156
  Show dependency tree
Reported: 2010-05-25 11:15 UTC by Steve Singer
Modified: 2010-09-09 10:12 UTC (History)
1 user (show)

See Also:


Note You need to log in before you can comment on or make changes to this bug.
Description Steve Singer 2010-05-25 11:15:47 UTC
Add a table to replication (say sometable)

Then do 

DROP TABLE sometable. (even via execute script)

Then try:
1) MOVE SET:  fails with: 
    <stdin>:13: PGRES_FATAL_ERROR select "_disorder_replica".moveSet(3, 3);  - ERROR:  Slony-I: alterTableConfigureTriggers(): Table with id 8 not found

2) SET DROP TABLE(id=8,origin=1) gives
 <stdin>:12: PGRES_FATAL_ERROR select "_disorder_replica".setDropTable(8);  - ERROR:  Slony-I: alterTableDropTriggers(): Table with id 8 not found

Forgetting to remove a table from replication before dropping it is a pretty common mistake.  There needs to be a clean (and obvious) way to recover from this short of manually changing sl_table.

I propose that the setDropTable be modified so that if the alterTableDropTriggers() function fails on a table we are about to drop then we proceed with dropping the table anyway.   Having moveSet() fail when the table deosn't exist is still resonable.
Comment 1 Christopher Browne 2010-08-19 12:55:15 UTC
That seems pretty apropos to me.

There's still a problem can take place on a remote node...

Suppose the sequence of events is thus:

1.  Table gets dropped on several nodes, possibly not the origin.

2.  MOVE SET is submitted, and, as the table was there on the origin, works fine, and starts propagating.

3.  SET DROP TABLE is submitted.

The MOVE SET can keep on failing on the subscriber due to the table being gone.  It'll never get around to the SET DROP TABLE request.

A resolution to this scenario would be for MOVE SET to have an exception block that catches the "table not found" problem, and then peeks into the future event stream to see if a suitable SET DROP TABLE is waiting.

It could, still, of course, be a terrible idea to do this.  

Note that a similar problem can happen with plain SYNCs, with some sl_log_[1|2] entries getting "orphaned" because the table got dropped a subscriber before relevant sl_log_* data got drained out.

The two problems likely need similarly shaped resolutions.
Comment 3 Christopher Browne 2010-08-27 15:22:53 UTC
(In reply to comment #2)
> Posted fix on github. 
> http://github.com/wieck/slony1-engine/commit/1045aa586fc1772d2ec68712cd434f9173c49fba

I have added a regression test to that branch, which breaks, on earlier versions, and which works fine once your fix is applied.
Comment 4 Steve Singer 2010-08-31 08:56:06 UTC
I'm not sure this fixes the bug described here.

Recall the sequence of events described here is

1. User does a "DROP TABLE someReplicatedTable"  (Say he does this on all nodes)
2. The user does a MOVE SET via slonik

The move set will fail on the subscriber in alterTableConfigureTriggers

the subscriber will be stuck on the move set and won't be able to get to a 'set drop table' command that your patch changes.

I think alterTableConfigureTriggers() also needs the check against pg_class and to log a warning if it isn't found.  Right not it throws a fatal exception.
Comment 5 Jan Wieck 2010-08-31 11:49:26 UTC
Well, that would mean that we want to do the check, if the table still exists, inside of alterTableXYZTriggers() (there are multiple of those).
Comment 6 Jan Wieck 2010-09-02 11:56:24 UTC
Here is a different attempt. It simply downgrades the RAISE in alterTableDropTriggers() and alterTableConfigureTriggers() from EXCEPTION to NOTICE, if the underlying table isn't found. 

There is however more outstanding in this case. If a replicated table is dropped on a subscriber, then incoming replication data for updates to that table will probably cause the SYNC to fail. I still need to test that case.
Comment 7 Jan Wieck 2010-09-02 12:03:43 UTC
Having the link may help:
Comment 8 Steve Singer 2010-09-03 08:31:54 UTC
With the patch applied consider this sequence of events:

1) have a replication with table "a" running between node 1 and node 2.
2) stop all slons
3) pg_dump node 2.  
4) drop the database on node 2. 
5) create a new database on node to and restore the pg_dump (say your upgrading postgres versions on this node or something
6) make sure that you do NOT run 'REPAIR CONFIG' because you've forgotten to
7) Do the MOVE SET from node 1 => node 2 

With the patch applied the move set will 'appear' to work and log the NOTICE.  However it leaves you in a state where both node 1 and node 2 have the _deny trigger enabled on them.

I'm thinking that if a row in pg_class with that oid is NOT found then we want to check to see if a table by that name can be found.  If the OID doesn't match but a table of the same name exists then we maybe want to raise an exception (like we do without the patch) so the DBA can run a 'REPAIR CONFIG' to fix things.

If both the oid and tablename can't be found in pg_class then we can just raise the notice.

OR  we could leave alterTableXXXXTriggers() as is and make REPAIR CONFIG remove the rows from sl_table if it can't match by oid or by name.   slonik connects directly to the EVENT_NODE and can run a repair config so we can skip-ahead of the failing moveSet' in the event queue.

Comment 9 Christopher Browne 2010-09-03 11:35:10 UTC
(In reply to comment #8)
> 6) make sure that you do NOT run 'REPAIR CONFIG' because you've forgotten to
> Thoughts?

Step #6 is the "grand mistake" portion of this.  The node's really rather broken because the tables aren't connected properly.

Perhaps the solution is to make sure that the slon refuses to start if you forget to REPAIR CONFIG.

One might do that by having slon do some "health checks" on the schema before it allows worker threads in to do work.  If things are found "unhealthy," then the slon reports the problems, and terminates.

The unhealthy state here would be that there is a disagreement between sl_table and the real set of tables in the database.
Comment 10 Christopher Browne 2010-09-09 10:12:08 UTC
(In reply to comment #9)
> One might do that by having slon do some "health checks" on the schema before
> it allows worker threads in to do work.  If things are found "unhealthy," then
> the slon reports the problems, and terminates.

This is being worked on separately in bug #156.