134 2010-06-07 01:43:00 -0700 TRUNCATE support 2010-11-16 14:50:21 -0800 1 1 1 Unclassified Slony-I stored procedures devel PC Linux RESOLVED FIXED http://github.com/cbbrowne/slony1-engine/commits/local-master/bug-134 low enhancement --- 137 1 peter_e cbbrowne slony1-bugs peter_e 2010-06-07 01:43:39 -0700 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. cbbrowne 2010-06-07 08:35:43 -0700 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. peter_e 2010-06-07 11:41:14 -0700 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. cbbrowne 2010-06-07 11:48:05 -0700 (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! :-( cbbrowne 2010-08-09 14:09:17 -0700 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; cbbrowne 2010-08-10 09:33:39 -0700 I have a branch out on GitHub that has a preliminary implementation. cbbrowne 2010-08-11 12:38:26 -0700 Add in some more tests, notably involving foreign key relationships. cbbrowne 2010-08-12 14:55:53 -0700 I've got a basically working implementation, here. http://github.com/cbbrowne/slony1-engine/commits/local-master/bug-134 ssinger 2010-11-15 13:45:47 -0800 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. cbbrowne 2010-11-15 14:44:30 -0800 (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. cbbrowne 2010-11-16 09:35:23 -0800 (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 cbbrowne 2010-11-16 13:16:00 -0800 Created an attachment (id=74) 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... cbbrowne 2010-11-16 14:50:21 -0800 Committed code changes to master: http://git.postgresql.org/gitweb?p=slony1-engine.git;a=commit;h=587f926d5b2ab1ed332d8e966eede6c7e4574da0 Also, documentation changes: http://git.postgresql.org/gitweb?p=slony1-engine.git;a=commit;h=4e0eede18828b2a0dcecd159cbaa3456eb40118e 74 2010-11-16 13:16 -0800 Differences from test run db_diff.2 text/plain 945 cbbrowne NzUsNzZjNzUsMTI5CjwgIGlkIHwgIGNuYW1lICAKPCAtLS0tKy0tLS0tLS0tLQotLS0KPiAgaWQg fCAgIGNuYW1lICAgIAo+IC0tLS0rLS0tLS0tLS0tLS0tCj4gICAxIHwgbmVpbAo+ICAgMiB8IHJp Y2hhcmQKPiAgIDMgfCBiaWxsCj4gICA0IHwgcGhpbAo+ICAgNSB8IHN0ZXZlCj4gICA2IHwgZmFy b3VsdAo+ICAgNyB8IHJvYnNvbgo+ICAgOCB8IHRyZWUKPiAgIDkgfCBtYW5pcwo+ICAxMCB8IHNj aGFmZmVyCj4gIDExIHwgam9yZ2Vuc2VuCj4gIDEyIHwgZ29vc2Vucwo+ICAxMyB8IG1pdHRlbGJh Y2gKPiAgMTQgfCBjMjU1OTEKPiAgMTUgfCBjMjU1OTIKPiAgMTYgfCBjMjU1OTMKPiAgMTcgfCBj MjU1OTQKPiAgMTggfCBjMjU1OTUKPiAgMTkgfCBjMjU1OTYKPiAgMjAgfCBjMjU1OTcKPiAgMjEg fCBjMjU1OTgKPiAgMjIgfCBjMjU1OTkKPiAgMjMgfCBjMjU1OTEwCj4gIDI0IHwgYzI1NTkxMQo+ ICAyNSB8IGMyNTU5MTIKPiAgMjYgfCBjMjU1OTEzCj4gIDI3IHwgYzI1NTkxNAo+ICAyOCB8IGMy NTU5MTUKPiAgMjkgfCBjMjU1OTE2Cj4gIDMwIHwgYzI1NTkxNwo+ICAzMSB8IGMyNTU5MTgKPiAg MzIgfCBjMjU1OTE5Cj4gIDMzIHwgYzI1NTkyMAo+ICAzNCB8IGM3NDYyMQo+ICAzNSB8IGM3NDYy Mgo+ICAzNiB8IGM3NDYyMwo+ICAzNyB8IGM3NDYyNAo+ICAzOCB8IGM3NDYyNQo+ICAzOSB8IGM3 NDYyNgo+ICA0MCB8IGM3NDYyNwo+ICA0MSB8IGM3NDYyOAo+ICA0MiB8IGM3NDYyOQo+ICA0MyB8 IGM3NDYyMTAKPiAgNDQgfCBjNzQ2MjExCj4gIDQ1IHwgYzc0NjIxMgo+ICA0NiB8IGM3NDYyMTMK PiAgNDcgfCBjNzQ2MjE0Cj4gIDQ4IHwgYzc0NjIxNQo+ICA0OSB8IGM3NDYyMTYKPiAgNTAgfCBj NzQ2MjE3Cj4gIDUxIHwgYzc0NjIxOAo+ICA1MiB8IGM3NDYyMTkKPiAgNTMgfCBjNzQ2MjIwCjk3 YzE1MAo8ICgyMCByb3dzKQotLS0KPiAoNzMgcm93cykK