Bug 134 - TRUNCATE support
Summary: TRUNCATE support
Status: RESOLVED FIXED
Alias: None
Product: Slony-I
Classification: Unclassified
Component: stored procedures (show other bugs)
Version: devel
Hardware: PC Linux
: low enhancement
Assignee: Christopher Browne
URL: http://github.com/cbbrowne/slony1-eng...
Depends on:
Blocks: 137
  Show dependency tree
 
Reported: 2010-06-07 01:43 UTC by Peter Eisentraut
Modified: 2014-04-15 16:01 UTC (History)
1 user (show)

See Also:


Attachments
Differences from test run (945 bytes, text/plain)
2010-11-16 13:16 UTC, Christopher Browne
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Peter Eisentraut 2010-06-07 01:43:39 UTC
Wishlist: Add support for TRUNCATE.  Either add a trigger that prevents TRUNCATE on the master, or propagate the TRUNCATE event to the slaves.

Right now, TRUNCATE is a loose foot-gun on a Slony setup.  See http://shoaibmir.wordpress.com/2010/06/03/truncate-problems-with-slony/ for example.
Comment 1 Christopher Browne 2010-06-07 08:35:43 UTC
Yep, I had a chat with various people about this at PGCon, and had some resolutions of outstanding questions.

The trouble with TRUNCATE comes when there are foreign key relationships.  In the case of circularity, one might find it necessary to submit:

  TRUNCATE a, b, c;

in order to get all three tables emptied.

Supposing that request is submitted, we'd find, logged, 3 TRUNCATE requests that were captured by ON TRUNCATE triggers.  (I'm assuming here that all 3 tables were replicated.)

I see 4 strategies for implementing the handling of TRUNCATE:

1.  Deny

Basically, we put the same stored function used for "deny_access" onto the ON TRUNCATE action.

This isn't ideal, but is better than what we have now.

2.  Log truncate, action on subscriber is "TRUNCATE ONLY"

This won't work out in the "TRUNCATE a,b,c;" scenario described earlier, as the FK is liable to cause each of these truncate requests to fail.

3.  Log truncate...  Try to form sets of successive TRUNCATEs into one request.

This adds substantially to complexity, and there's the question of when to do the set of truncates:

  a) At the earliest point in the stream, or
  b) At the latest point in the stream.

More thought necessary to see which order is expected to be "compatible."

[Thinking a bit more...  This might well work...  The transaction claims an exclusive lock on all the tables, so it ought to be possible to, upon seeing a TRUNCATE, look for all the adjacent TRUNCATE requests in that same transaction.  The only trouble is how to not bother reprocessing the subsequent TRUNCATE requests.]

There is still some risk of failure on the subscriber; supposing there are differences in FK relations there, there is the possibility of everything rolling back, and replication becoming blocked at this point.

4.  Log the truncates, and, for each one, submit
   TRUNCATE foo CASCADE;

This has merits over all the others:

- It performs TRUNCATE (unlike 1!)
- Guaranteed to work (unlike 2)
- Much simpler than #3
- No risk of getting stuck because of a FK relationship

There's a big scary demerit:

- It'll blindly truncate everything tied to the table being truncated.

I had a chat about this with Rod Taylor and Greg Sabino Mullane at the EDB dinner; Rod suggested that #4 was the way to go, and I made sure Greg was aware of the challenge here.

Peter, your thoughts would definitely be valued.
Comment 2 Peter Eisentraut 2010-06-07 11:41:14 UTC
My immediate interest in this comes more from the point of closing possible breakage vectors, so prohibiting TRUNCATE on the master would be good for me.

I don't have a qualified opinion at the moment on how to implement replicating the TRUNCATE event.
Comment 3 Christopher Browne 2010-06-07 11:48:05 UTC
(In reply to comment #2)
> My immediate interest in this comes more from the point of closing possible
> breakage vectors, so prohibiting TRUNCATE on the master would be good for me.

Actually, we'd want to prohibit TRUNCATE on all nodes, in that case.

It's not so much of an improvement if you can still easily blow up subscribers! 
:-(
Comment 4 Christopher Browne 2010-08-09 14:09:17 UTC
Thinking about implementation...

We could:

a) Try to capture the trigger using the existing C/SPI function.  That seems like overkill, as we don't need to collect data.

b) Create a trigger function that captures the table ID as its argument, and uses that to determine which table to truncate.

Consistent with the preceding discussion, I propose using "policy #4" to handle this, that is, on replicated nodes, we'll run, for each table:
   truncate ONLY my_table CASCADE;
Comment 5 Christopher Browne 2010-08-10 09:33:39 UTC
I have a branch out on GitHub that has a preliminary implementation.
Comment 6 Christopher Browne 2010-08-11 12:38:26 UTC
Add in some more tests, notably involving foreign key relationships.
Comment 7 Christopher Browne 2010-08-12 14:55:53 UTC
I've got a basically working implementation, here.

http://github.com/cbbrowne/slony1-engine/commits/local-master/bug-134
Comment 8 Steve Singer 2010-11-15 13:45:47 UTC
I reviewed a3a2e4839f5cf21bfd076e30e916cd23112daf97 rebased to our current master (3cbbaba072b38527fa2ac7949ba680e7bd410a9a) with conflicts resolved.

A few comments

1.  Upgrade script support.  As it stands today an upgrade from 2.0
to 2.1 doesn't require re-installing the cluster.  This patch doesn't
warrent (in my opinion) requiring that.  Something else in the future
might but I would like to see an upgrade script add the truncate triggers
to all replicated tables.

2. I manually tested the functionality and it looked okay.  However run I tried the regression test you added I got the same 'result' against 8.3 and 9.1master.
Since 8.3 doesn't support truncate triggers this confused me, it might just be my unfamiliarity with the regression tests.  I was expecting a it to say it failed with 8.3 (is that what we want?)

3. This probably should also have a documentation update, limitations.sgml at a minimum to say we now support truncate in 8.4 and above.  We also might want to say somewhere that the truncate on the slave is a CASCADE.   This would be important to note for setups who have a different schema on their slave vs master. 

Other than that I'm happy with this for master.
Comment 9 Christopher Browne 2010-11-15 14:44:30 UTC
(In reply to comment #8)
> I reviewed a3a2e4839f5cf21bfd076e30e916cd23112daf97 rebased to our current
> master (3cbbaba072b38527fa2ac7949ba680e7bd410a9a) with conflicts resolved.
> 
> A few comments
> 
> 1.  Upgrade script support.  As it stands today an upgrade from 2.0
> to 2.1 doesn't require re-installing the cluster.  This patch doesn't
> warrent (in my opinion) requiring that.  Something else in the future
> might but I would like to see an upgrade script add the truncate triggers
> to all replicated tables.

Good idea; I will rework it so that it tries to add the triggers, if possible.

> 2. I manually tested the functionality and it looked okay.  However run I tried
> the regression test you added I got the same 'result' against 8.3 and
> 9.1master.
> Since 8.3 doesn't support truncate triggers this confused me, it might just be
> my unfamiliarity with the regression tests.  I was expecting a it to say it
> failed with 8.3 (is that what we want?)

Hmm.  I'd expect the test to fail on 8.3, because tables would be empty there, but not on the subscribers.

I don't think I tried running it on 8.3, because I expected it not to work properly there.

> 3. This probably should also have a documentation update, limitations.sgml at a
> minimum to say we now support truncate in 8.4 and above.  We also might want to
> say somewhere that the truncate on the slave is a CASCADE.   This would be
> important to note for setups who have a different schema on their slave vs
> master. 

Yep, something needs to be put in there.  I think I'll either need to:
a) Merge docs from HEAD into this branch, or
b) Document as part of the merge into HEAD, as
c) Try and muddle docs in as-is won't work out very happily due to the lots of upstream changes to the docs.

Probably b) is simplest.

> Other than that I'm happy with this for master.
Comment 10 Christopher Browne 2010-11-16 09:35:23 UTC
(In reply to comment #9)
> (In reply to comment #8)
> > I reviewed a3a2e4839f5cf21bfd076e30e916cd23112daf97 rebased to our current
> > master (3cbbaba072b38527fa2ac7949ba680e7bd410a9a) with conflicts resolved.
> > 
> > A few comments
> > 
> > 1.  Upgrade script support.  As it stands today an upgrade from 2.0
> > to 2.1 doesn't require re-installing the cluster.  This patch doesn't
> > warrent (in my opinion) requiring that.  Something else in the future
> > might but I would like to see an upgrade script add the truncate triggers
> > to all replicated tables.
> 
> Good idea; I will rework it so that it tries to add the triggers, if possible.

Done.

https://github.com/cbbrowne/slony1-engine/commit/f1fb66ceec3d32039f83f293a362668d9d820746
Comment 11 Christopher Browne 2010-11-16 13:16:00 UTC
Created attachment 74 [details]
Differences from test run

getting data from origin DB for diffing
done
getting data from node 2 for diffing against origin
comparing
./run_test.sh: WARNING: /tmp/slony-regress.abhpMa/db_1.dmp /tmp/slony-regress.abhpMa/db_2.dmp differ, see /tmp/slony-regress.abhpMa/db_diff.2 for details
done


Looking good...
Comment 13 Wade Colson 2014-04-15 16:01:43 UTC
*** Bug 260998 has been marked as a duplicate of this bug. ***
Seen from the domain http://volichat.com
Page where seen: http://volichat.com/gay-video-chat-rooms
Marked for reference. Resolved as fixed @bugzilla.