From 94521f91ae32c129ebb28ca0b4ca4fcabb2bbc00 Mon Sep 17 00:00:00 2001 From: Steve Singer Date: Thu, 1 Aug 2013 15:09:45 -0400 Subject: [PATCH 01/15] bug 304 - record sequence values as part of a EXECUTE_SCRIPT Prior to 2.2 the EXECUTE_SCRIPT command result in a DDL_SCRIPT event and a SYNC event generated as part of the same transaction. That SYNC event also replicates the value of any sequences to the replica before the DDL SCRIPT command. In 2.2 the DDL script is replicated as part of a normal SYNC as rows in sl_log_script. This commit stores any sequence changes as extra elements in the cmdargs array. These sequence values are then set before the DDL_SCRIPT is executed on the replica This commit includes a unit test change to reproduce this issue the test now passes --- .../regression/testddl/init_schema.sql | 10 ++ clustertest/regression/testddl/testddl.js | 35 ++++++ doc/adminguide/ddlchanges.sgml | 14 +++ src/backend/slony1_base.sql | 1 - src/backend/slony1_funcs.c | 107 ++++++++++++++++-- src/backend/slony1_funcs.sql | 65 +++++++++-- 6 files changed, 214 insertions(+), 18 deletions(-) diff --git a/clustertest/regression/testddl/init_schema.sql b/clustertest/regression/testddl/init_schema.sql index ee951881..1e940753 100644 --- a/clustertest/regression/testddl/init_schema.sql +++ b/clustertest/regression/testddl/init_schema.sql @@ -60,3 +60,13 @@ zone_id integer ALTER TABLE ONLY billing_discount ADD CONSTRAINT billing_discount_pkey PRIMARY KEY (billing_discount_id); +CREATE OR REPLACE FUNCTION insert_table1() returns trigger +as $$ +declare + +begin + insert into table1(data) values (NEW.data); + return NEW; +end; +$$ +language plpgsql; \ No newline at end of file diff --git a/clustertest/regression/testddl/testddl.js b/clustertest/regression/testddl/testddl.js index 4288f49a..7ecf4755 100644 --- a/clustertest/regression/testddl/testddl.js +++ b/clustertest/regression/testddl/testddl.js @@ -29,6 +29,9 @@ function init_tables() { +"set add table (id=4, set id=1, origin=1, fully qualified name = 'public.table4');\n" +"set add table (id=5, set id=1, origin=1, fully qualified name = 'public.table5');\n" +"set add table (id=6, set id=1, origin=1, fully qualified name = 'public.billing_discount');\n" + + "set add sequence(id=1,set id=1, origin=1, fully qualified name= 'public.table1_id_seq');\n" + + "set add sequence(id=2,set id=1, origin=1, fully qualified name= 'public.table5_id_seq');\n" + return script; } @@ -121,6 +124,33 @@ function individual_ddl(coordinator, nodenum) { run_slonik('update ddl',coordinator,preamble,slonikScript); } +function trigger_function(coordinator) { + /** + * We stop the slons because we want to make sure that a SYNC does not + * happen in between the EXECUTE_SCRIPT and the next SYNC. + */ + terminate_slon(coordinator); + var sql = ''; + for(var idx=0; idx < 1000; idx++) { + sql = sql + "insert into table5(data) values ('seqtest');\n" + } + var psql = coordinator.createPsqlCommand('db1',sql); + psql.run(); + coordinator.join(psql); + premable = get_slonik_preamble(); + slonikScript = "EXECUTE SCRIPT( SQL='alter table table1 drop column seqed;create trigger table5_trigger " + + " before INSERT on public.table5 for each row execute procedure " + + " insert_table1();'" + + ' ,EVENT NODE=1 );'; + run_slonik('add trigger ddl',coordinator,preamble,slonikScript); + slonikScript = "EXECUTE SCRIPT( SQL='insert into table5(data) values (9);'" + + ' ,EVENT NODE=1);'; + run_slonik('add trigger ddl',coordinator,preamble,slonikScript); + + launch_slon(coordinator); + +} + function inline_ddl(coordinator) { premable = get_slonik_preamble(); @@ -166,6 +196,11 @@ function do_test(coordinator) { inline_ddl(coordinator); wait_for_sync(coordinator); + + + trigger_function(coordinator); + wait_for_sync(coordinator); + } function get_compare_queries() { diff --git a/doc/adminguide/ddlchanges.sgml b/doc/adminguide/ddlchanges.sgml index 549e576c..21f9f342 100644 --- a/doc/adminguide/ddlchanges.sgml +++ b/doc/adminguide/ddlchanges.sgml @@ -66,6 +66,20 @@ replica nodes. It is advisiable to not be concurrently inserting,deleting or updating rows to a table while a script changing that table (adding or deleting columns) is also running. +&slony1; 2.2.x and higher will replicate the SQL +of an EXECUTE SCRIPT as part of a SYNC. Scripts that perfrom an "ALTER TABLE" +to a replicated table will be replicated in the correct order with respect +to other concurrent activities on that table because of the exclusive lock +that the alter table received on the origin node. If your EXECUTE SCRIPT +does not obtain exclusive locks on all of the tables it uses then +you need to make sure that any transactions running concurrently with +the script are not making changes that can effect the results of the script. +For example, if your script does a nextval('some_replicated_seq') and +that sequence is being incremented by another transactions at the same time then +it is possible that script sees a different value from the sequence when +it runs on a replica than it did on the origin. + + diff --git a/src/backend/slony1_base.sql b/src/backend/slony1_base.sql index 83ee96fd..9be53974 100644 --- a/src/backend/slony1_base.sql +++ b/src/backend/slony1_base.sql @@ -728,4 +728,3 @@ comment on column @NAMESPACE@.sl_components.co_event is 'which event have I star -- ---------------------------------------------------------------------- grant usage on schema @NAMESPACE@ to public; - diff --git a/src/backend/slony1_funcs.c b/src/backend/slony1_funcs.c index 0699cbc9..cb783055 100644 --- a/src/backend/slony1_funcs.c +++ b/src/backend/slony1_funcs.c @@ -42,6 +42,7 @@ #include "utils/memutils.h" #include "utils/hsearch.h" #include "utils/timestamp.h" +#include "utils/int8.h" #ifdef HAVE_GETACTIVESNAPSHOT #include "utils/snapmgr.h" #endif @@ -989,7 +990,14 @@ versionFunc(logApply)(PG_FUNCTION_ARGS) char *ddl_script; bool localNodeFound = true; Datum script_insert_args[5]; - char query[1024]; + Datum *nodeargs; + bool *nodeargsnulls; + int nodeargsn; + Datum *seqargs; + bool *seqargsnulls; + int seqargsn; + Datum array_holder; + Datum delim_text; apply_num_script++; @@ -1001,29 +1009,64 @@ versionFunc(logApply)(PG_FUNCTION_ARGS) SPI_fnumber(tupdesc, "log_cmdargs"), &isnull); if (isnull) elog(ERROR, "Slony-I: log_cmdargs is NULL"); - deconstruct_array(DatumGetArrayTypeP(dat), TEXTOID, -1, false, 'i', - &cmdargs, &cmdargsnulls, &cmdargsn); + &cmdargs, &cmdargsnulls, &cmdargsn); + + if( cmdargsn < 3 ) + { + elog(ERROR,"Slony-I: DDL_SCRIPT events require at least 3 elements"\ + "in the argument array"); + } + nodeargs=NULL; + nodeargsn=0; + delim_text=DirectFunctionCall1(textin,CStringGetDatum(",")); + if ( (! cmdargsnulls[1]) ) + { + char * astr=DatumGetCString(DirectFunctionCall1(textout, + cmdargs[1])); + + if ( strcmp(astr,"")) + { + array_holder = DirectFunctionCall2(text_to_array,cmdargs[1], + delim_text); + deconstruct_array(DatumGetArrayTypeP(array_holder), + TEXTOID, -1, false, 'i', + &nodeargs, &nodeargsnulls, &nodeargsn); + } + } + seqargs=NULL; + seqargsn=0; + if ( (! cmdargsnulls[2]) ) + { + char * astr=DatumGetCString(DirectFunctionCall1(textout, + cmdargs[2])); + if( strcmp(astr,"") ) + { + array_holder = DirectFunctionCall2(text_to_array,cmdargs[2], + delim_text); + deconstruct_array(DatumGetArrayTypeP(array_holder), + TEXTARRAYOID, -1, false, 'i', + &seqargs, &seqargsnulls, &seqargsn); + } + } /* * The first element is the DDL statement itself. */ ddl_script = DatumGetCString(DirectFunctionCall1( textout, cmdargs[0])); - /* * If there is an optional node ID list, check that we are in it. */ - if (cmdargsn > 1) + if (nodeargsn > 0) { localNodeFound = false; - for (i = 1; i < cmdargsn; i++) + for (i = 0; i < nodeargsn; i++) { int32 nodeId = DatumGetInt32( DirectFunctionCall1(int4in, - DirectFunctionCall1(textout, cmdargs[i]))); - + DirectFunctionCall1(textout, nodeargs[i]))); if (nodeId == cs->localNodeId) { localNodeFound = true; @@ -1038,6 +1081,53 @@ versionFunc(logApply)(PG_FUNCTION_ARGS) */ if (localNodeFound) { + + char query[1024]; + Oid argtypes[3]; + argtypes[0] = INT4OID; + argtypes[1] = INT4OID; + argtypes[2] = INT8OID; + + snprintf(query,1023,"select %s.sequenceSetValue($1," \ + "$2,NULL,$3); ",tg->tg_trigger->tgargs[0]); + void * plan = SPI_prepare(query,3,argtypes); + if ( plan == NULL ) + { + + elog(ERROR,"could not prepare plan to call sequenceSetValue"); + } + /** + * before we execute the DDL we need to update the sequences. + */ + if ( seqargsn > 0 ) + { + + for( i = 0; (i+2) < seqargsn; i=i+3 ) + { + Datum call_args[3]; + bool call_nulls[3]; + call_args[0] = DirectFunctionCall1(int4in, + DirectFunctionCall1(textout,seqargs[i])); + call_args[1] = DirectFunctionCall1(int4in, + DirectFunctionCall1(textout,seqargs[i+1])); + call_args[2] = DirectFunctionCall1(int8in, + DirectFunctionCall1(textout,seqargs[i+2])); + + call_nulls[0]=0; + call_nulls[1]=0; + call_nulls[2]=0; + + if ( SPI_execp(plan,call_args,call_nulls,0) < 0 ) + { + elog(ERROR,"error executing sequenceSetValue plan"); + + } + + } + + + } + sprintf(query,"set session_replication_role to local;"); if(SPI_exec(query,0) < 0) { @@ -1606,7 +1696,6 @@ versionFunc(logApply)(PG_FUNCTION_ARGS) MemoryContextSwitchTo(oldContext); cacheEnt->typmod[i / 2] = target_rel->rd_att->attrs[colnum - 1]->atttypmod; - sprintf(applyQueryPos, "%s%s = $%d", (i > 0) ? " AND " : "", slon_quote_identifier(colname), diff --git a/src/backend/slony1_funcs.sql b/src/backend/slony1_funcs.sql index 00aad224..dcbc9d91 100644 --- a/src/backend/slony1_funcs.sql +++ b/src/backend/slony1_funcs.sql @@ -3366,10 +3366,11 @@ begin execute 'select setval(''' || v_fqname || ''', ' || p_last_value::text || ')'; - insert into @NAMESPACE@.sl_seqlog + if p_ev_seqno is not null then + insert into @NAMESPACE@.sl_seqlog (seql_seqid, seql_origin, seql_ev_seqno, seql_last_value) values (p_seq_id, p_seq_origin, p_ev_seqno, p_last_value); - + end if; return p_seq_id; end; $$ language plpgsql; @@ -3391,10 +3392,13 @@ declare c_found_origin boolean; c_node text; c_cmdargs text[]; + c_nodeargs text; + c_delim text; begin c_local_node := @NAMESPACE@.getLocalNodeId('_@CLUSTERNAME@'); c_cmdargs = array_append('{}'::text[], p_statement); + c_nodeargs = ''; if p_nodes is not null then c_found_origin := 'f'; -- p_nodes list needs to consist of a list of nodes that exist @@ -3411,8 +3415,11 @@ begin if c_local_node = (c_node::integer) then c_found_origin := 't'; end if; - - c_cmdargs = array_append(c_cmdargs, c_node); + if length(c_nodeargs)>0 then + c_nodeargs = c_nodeargs ||','|| c_node; + else + c_nodeargs=c_node; + end if; end loop; if not c_found_origin then @@ -3421,15 +3428,24 @@ begin p_statement, p_nodes, c_local_node; end if; end if; - - execute p_statement; - + c_cmdargs = array_append(c_cmdargs,c_nodeargs); + c_delim=','; + c_cmdargs = array_append(c_cmdargs, + + (select @NAMESPACE@.string_agg( seq_id::text || c_delim + || c_local_node || + c_delim || seq_last_value) + FROM ( + select seq_id, + seq_last_value from @NAMESPACE@.sl_seqlastvalue + where seq_origin = c_local_node) as FOO + where NOT @NAMESPACE@.seqtrack(seq_id,seq_last_value) is NULL)); insert into @NAMESPACE@.sl_log_script (log_origin, log_txid, log_actionseq, log_cmdtype, log_cmdargs) values (c_local_node, pg_catalog.txid_current(), nextval('@NAMESPACE@.sl_action_seq'), 'S', c_cmdargs); - + execute p_statement; return currval('@NAMESPACE@.sl_action_seq'); end; $$ language plpgsql; @@ -6224,3 +6240,36 @@ begin return v_seq_id; end $$ language plpgsql; + + + +-- +-- we create a function + aggregate for string_agg to aggregate strings +-- some versions of PG (ie prior to 9.0) don't support this +CREATE OR replace function @NAMESPACE@.agg_text_sum(txt_before TEXT, txt_new TEXT) RETURNS TEXT AS +$BODY$ +DECLARE + c_delim text; +BEGIN + c_delim = ','; + IF (txt_before IS NULL or txt_before='') THEN + RETURN txt_new; + END IF; + RETURN txt_before || c_delim || txt_new; +END; +$BODY$ +LANGUAGE plpgsql; +comment on function @NAMESPACE@.agg_text_sum(text,text) is +'An accumulator function used by the slony string_agg function to +aggregate rows into a string'; +-- +-- create a string_agg function in the slony schema. +-- PG 8.3 does not have this function so we make our own +-- when slony stops supporting PG 8.3 we can switch to +-- the PG 9.0+ provided version of string_agg +-- +CREATE AGGREGATE @NAMESPACE@.string_agg(text) ( +SFUNC=@NAMESPACE@.agg_text_sum, +STYPE=text, +INITCOND='' +); \ No newline at end of file From ee5c88189de37d72a4c2f45953e8794b29c2c2f7 Mon Sep 17 00:00:00 2001 From: Steve Singer Date: Thu, 1 Aug 2013 16:03:48 -0400 Subject: [PATCH 02/15] Allow the apply trigger to work (ie not fail) on sl_log_script rows from earlier betas of 2.2.0 --- src/backend/slony1_funcs.c | 62 +++++++++++++++++++------------------- 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/src/backend/slony1_funcs.c b/src/backend/slony1_funcs.c index cb783055..0051ea34 100644 --- a/src/backend/slony1_funcs.c +++ b/src/backend/slony1_funcs.c @@ -1013,44 +1013,44 @@ versionFunc(logApply)(PG_FUNCTION_ARGS) TEXTOID, -1, false, 'i', &cmdargs, &cmdargsnulls, &cmdargsn); - if( cmdargsn < 3 ) - { - elog(ERROR,"Slony-I: DDL_SCRIPT events require at least 3 elements"\ - "in the argument array"); - } - nodeargs=NULL; nodeargsn=0; - delim_text=DirectFunctionCall1(textin,CStringGetDatum(",")); - if ( (! cmdargsnulls[1]) ) - { - char * astr=DatumGetCString(DirectFunctionCall1(textout, - cmdargs[1])); - - if ( strcmp(astr,"")) - { - array_holder = DirectFunctionCall2(text_to_array,cmdargs[1], - delim_text); - deconstruct_array(DatumGetArrayTypeP(array_holder), - TEXTOID, -1, false, 'i', - &nodeargs, &nodeargsnulls, &nodeargsn); - } - } seqargs=NULL; seqargsn=0; - if ( (! cmdargsnulls[2]) ) + if( cmdargsn >= 2 ) { - char * astr=DatumGetCString(DirectFunctionCall1(textout, - cmdargs[2])); - if( strcmp(astr,"") ) - { - array_holder = DirectFunctionCall2(text_to_array,cmdargs[2], - delim_text); - deconstruct_array(DatumGetArrayTypeP(array_holder), - TEXTARRAYOID, -1, false, 'i', - &seqargs, &seqargsnulls, &seqargsn); + delim_text=DirectFunctionCall1(textin,CStringGetDatum(",")); + if ( (! cmdargsnulls[1]) ) + { + char * astr=DatumGetCString(DirectFunctionCall1(textout, + cmdargs[1])); + + if ( strcmp(astr,"")) + { + array_holder = DirectFunctionCall2(text_to_array,cmdargs[1], + delim_text); + deconstruct_array(DatumGetArrayTypeP(array_holder), + TEXTOID, -1, false, 'i', + &nodeargs, &nodeargsnulls, &nodeargsn); + } } } + if(cmdargsn >= 3) + { + if ( (! cmdargsnulls[2]) ) + { + char * astr=DatumGetCString(DirectFunctionCall1(textout, + cmdargs[2])); + if( strcmp(astr,"") ) + { + array_holder = DirectFunctionCall2(text_to_array,cmdargs[2], + delim_text); + deconstruct_array(DatumGetArrayTypeP(array_holder), + TEXTARRAYOID, -1, false, 'i', + &seqargs, &seqargsnulls, &seqargsn); + } + } + } /* * The first element is the DDL statement itself. */ From ff3fd0dd33e6a7c1f7b562c305612f3af271b9a4 Mon Sep 17 00:00:00 2001 From: Christopher Browne Date: Thu, 1 Aug 2013 17:32:12 -0400 Subject: [PATCH 03/15] Revisions to DDL docs --- doc/adminguide/ddlchanges.sgml | 77 ++++++++++++++++++++++++++++------ 1 file changed, 65 insertions(+), 12 deletions(-) diff --git a/doc/adminguide/ddlchanges.sgml b/doc/adminguide/ddlchanges.sgml index 21f9f342..9104ffb0 100644 --- a/doc/adminguide/ddlchanges.sgml +++ b/doc/adminguide/ddlchanges.sgml @@ -66,19 +66,72 @@ replica nodes. It is advisiable to not be concurrently inserting,deleting or updating rows to a table while a script changing that table (adding or deleting columns) is also running. -&slony1; 2.2.x and higher will replicate the SQL -of an EXECUTE SCRIPT as part of a SYNC. Scripts that perfrom an "ALTER TABLE" -to a replicated table will be replicated in the correct order with respect -to other concurrent activities on that table because of the exclusive lock -that the alter table received on the origin node. If your EXECUTE SCRIPT -does not obtain exclusive locks on all of the tables it uses then -you need to make sure that any transactions running concurrently with -the script are not making changes that can effect the results of the script. -For example, if your script does a nextval('some_replicated_seq') and -that sequence is being incremented by another transactions at the same time then -it is possible that script sees a different value from the sequence when -it runs on a replica than it did on the origin. +&slony1; 2.2.x and higher replicate the SQL requests +of an EXECUTE SCRIPT alongside other logged replication activity as +part of an ordinary SYNC. Scripts that perform an ALTER +TABLE to a replicated table are replicated in the correct +order with respect to other concurrent activities on that table, and +this is guaranteed because of the exclusive lock that the +ALTER TABLE acquired on the origin node. If your +EXECUTE SCRIPT does not obtain exclusive locks on all of the tables it +uses, then you need to make sure that any transactions running +concurrently with the script are not making changes that can adversely +affect the computations in the script. + + For example, if your script performs +nextval('some_replicated_seq'), and that sequence +is concurrently being incremented by another transaction, then it is +possible that when the queries are replayed on a replica, the sequence +may have different values on the replica than it had on the +origin. + + In addition, it is crucial that DML requests +propagated by EXECUTE SCRIPT be deterministic in their behaviour, +otherwise the requests may (legitimately) be processed differently on +different nodes, thereby corrupting data. + + For instance, the following queries are all not deterministic, +as they do not clearly indicate which of the tuples in the table will +be affected: + + Insufficiently specified + +insert into table2 (id, data) + select id, data from table1 limit 10; + + +This query may legitimately take any 10 +tuples from table1, no reason for it to behave the same +across nodes. + Ridiculously different behaviour + +insert into table2 (id, data) + select id, data from table1 order by random () limit 10; + + +This query makes it more obvious that it could draw +any 10 tuples from table1, in any +order. + + Node-local results + +insert into table3 (id, data, transaction_date, value) + select id, data, now(), random() from table1 order by id limit 10; + + +This query will compute now() based on the +time the query runs on each node, and values of +random() varying each time, so that while the +order by id clause would have eliminated the +ambiguity in the earlier cases, we can be quite certain that this +query will put substantially different data into table3 +on every node on which it is executed. + + + + + From cdfd5628bed827a844f547942e4dedecdc1a997f Mon Sep 17 00:00:00 2001 From: Christopher Browne Date: Thu, 1 Aug 2013 17:53:16 -0400 Subject: [PATCH 04/15] Add release note on #304 --- RELEASE | 6 ++++++ doc/adminguide/ddlchanges.sgml | 1 - 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/RELEASE b/RELEASE index 5b5a2ba7..d027f6ed 100644 --- a/RELEASE +++ b/RELEASE @@ -4,6 +4,12 @@ - Bug 305 :: set session_replication_role to local when applying DDL_SCRIPT on a replica +*** TBD + +- Bug 304 :: Record sequence values each time SQL is recorded in + ~sl_log_script~, and set those values before invoking the + SQL on the replica. + *** 2.2.0 b5 - Bug 296 :: Fixes for FAILOVER when non-origin nodes fail at the same time diff --git a/doc/adminguide/ddlchanges.sgml b/doc/adminguide/ddlchanges.sgml index 9104ffb0..59a7d304 100644 --- a/doc/adminguide/ddlchanges.sgml +++ b/doc/adminguide/ddlchanges.sgml @@ -137,7 +137,6 @@ on every node on which it is executed. - Applying DDL Changes Directly From 8e63eea20d6bf9a7b4bb293f6a19859ae6080382 Mon Sep 17 00:00:00 2001 From: Christopher Browne Date: Fri, 2 Aug 2013 17:31:30 -0400 Subject: [PATCH 05/15] Fix typo in sample data --- clustertest/regression/testdatestyles/init_data.sql | 2 -- tests/testdatestyles/init_data.sql | 2 -- 2 files changed, 4 deletions(-) diff --git a/clustertest/regression/testdatestyles/init_data.sql b/clustertest/regression/testdatestyles/init_data.sql index d2c79e65..384e057c 100644 --- a/clustertest/regression/testdatestyles/init_data.sql +++ b/clustertest/regression/testdatestyles/init_data.sql @@ -2,8 +2,6 @@ insert into table1(ts, tsz, ds) values ('2006-01-01', '2006-06-01', '2007-01-01' insert into table1(ts, tsz, ds) values ('infinity', 'infinity', now()); insert into table1(ts, tsz, ds) values ('-infinity', '-infinity', now()); -insert into table1 (ts, tsz, ds) values ( - -- Some nice dates that take place inside the time window that was -- a tad ambiguous in 2007 in that the definition of daylight savings time -- was changed - per Bill Moran diff --git a/tests/testdatestyles/init_data.sql b/tests/testdatestyles/init_data.sql index ca4809e9..9e770187 100644 --- a/tests/testdatestyles/init_data.sql +++ b/tests/testdatestyles/init_data.sql @@ -2,8 +2,6 @@ insert into table1(ts, tsz, ds) values ('2006-01-01', '2006-06-01', '2007-01-01' insert into table1(ts, tsz, ds) values ('infinity', 'infinity', now()); insert into table1(ts, tsz, ds) values ('-infinity', '-infinity', now()); -insert into table1 (ts, tsz, ds) values ( - -- Some nice dates that take place inside the time window that was -- a tad ambiguous in 2007 in that the definition of daylight savings time -- was changed - per Bill Moran From d7a19d00cc03160d9abca20722f4059dbc4d7915 Mon Sep 17 00:00:00 2001 From: Steve Singer Date: Fri, 9 Aug 2013 12:52:08 -0400 Subject: [PATCH 06/15] create string_agg in slony1_base.sql not slony1_funcs.sql If the CREATE AGGREGATE is in slony1_funcs.sql it will fail on an update functions since the aggregate already exists. As part of the upgrade we will check to see if we need to create the aggregate. --- src/backend/slony1_base.sql | 33 +++++++++++++++++++++++++++++++++ src/backend/slony1_funcs.sql | 19 +++++++------------ 2 files changed, 40 insertions(+), 12 deletions(-) diff --git a/src/backend/slony1_base.sql b/src/backend/slony1_base.sql index 9be53974..75c7f116 100644 --- a/src/backend/slony1_base.sql +++ b/src/backend/slony1_base.sql @@ -723,6 +723,39 @@ comment on column @NAMESPACE@.sl_components.co_eventtype is 'what kind of event comment on column @NAMESPACE@.sl_components.co_event is 'which event have I started processing?'; + +-- +-- we create a function + aggregate for string_agg to aggregate strings +-- some versions of PG (ie prior to 9.0) don't support this +CREATE OR replace function @NAMESPACE@.agg_text_sum(txt_before TEXT, txt_new TEXT) RETURNS TEXT AS +$BODY$ +DECLARE + c_delim text; +BEGIN + c_delim = ','; + IF (txt_before IS NULL or txt_before='') THEN + RETURN txt_new; + END IF; + RETURN txt_before || c_delim || txt_new; +END; +$BODY$ +LANGUAGE plpgsql; +comment on function @NAMESPACE@.agg_text_sum(text,text) is +'An accumulator function used by the slony string_agg function to +aggregate rows into a string'; +-- +-- create a string_agg function in the slony schema. +-- PG 8.3 does not have this function so we make our own +-- when slony stops supporting PG 8.3 we can switch to +-- the PG 9.0+ provided version of string_agg +-- +CREATE AGGREGATE @NAMESPACE@.string_agg(text) ( +SFUNC=@NAMESPACE@.agg_text_sum, +STYPE=text, +INITCOND='' +); + + -- ---------------------------------------------------------------------- -- Last but not least grant USAGE to the replication schema objects. -- ---------------------------------------------------------------------- diff --git a/src/backend/slony1_funcs.sql b/src/backend/slony1_funcs.sql index dcbc9d91..1519c6f1 100644 --- a/src/backend/slony1_funcs.sql +++ b/src/backend/slony1_funcs.sql @@ -5487,7 +5487,13 @@ create table @NAMESPACE@.sl_components ( alter table @NAMESPACE@.sl_log_2 enable replica trigger apply_trigger; end if; - + if not exists (select 1 from information_schema.routines where routine_schema = '_@CLUSTERNAME@' and routine_name = 'string_agg') then + CREATE AGGREGATE @NAMESPACE@.string_agg(text) ( + SFUNC=@NAMESPACE@.agg_text_sum, + STYPE=text, + INITCOND='' + ); + end if; return p_old; end; $$ language plpgsql; @@ -6262,14 +6268,3 @@ LANGUAGE plpgsql; comment on function @NAMESPACE@.agg_text_sum(text,text) is 'An accumulator function used by the slony string_agg function to aggregate rows into a string'; --- --- create a string_agg function in the slony schema. --- PG 8.3 does not have this function so we make our own --- when slony stops supporting PG 8.3 we can switch to --- the PG 9.0+ provided version of string_agg --- -CREATE AGGREGATE @NAMESPACE@.string_agg(text) ( -SFUNC=@NAMESPACE@.agg_text_sum, -STYPE=text, -INITCOND='' -); \ No newline at end of file From 177a52b0ac417baec4a458072c8bd2657bc9edb8 Mon Sep 17 00:00:00 2001 From: Steve Singer Date: Wed, 14 Aug 2013 10:46:45 -0400 Subject: [PATCH 07/15] deconstruct_array wants the OID of the element type, not the array type --- src/backend/slony1_funcs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/backend/slony1_funcs.c b/src/backend/slony1_funcs.c index 0051ea34..91792b67 100644 --- a/src/backend/slony1_funcs.c +++ b/src/backend/slony1_funcs.c @@ -1046,7 +1046,7 @@ versionFunc(logApply)(PG_FUNCTION_ARGS) array_holder = DirectFunctionCall2(text_to_array,cmdargs[2], delim_text); deconstruct_array(DatumGetArrayTypeP(array_holder), - TEXTARRAYOID, -1, false, 'i', + TEXTOID, -1, false, 'i', &seqargs, &seqargsnulls, &seqargsn); } } From b98a966ba5967b60b0482ae259a3ef32f14829fc Mon Sep 17 00:00:00 2001 From: Steve Singer Date: Wed, 14 Aug 2013 17:30:13 -0400 Subject: [PATCH 08/15] bug 310 - only issue the notify Restart if we actually change configuration The FAILOVER_NODE event is processed in two parts by slon. First failedNode(..) is called which reconfigures sl_subscribe, sl_path and then the listen network. Once this is done notify is used to signal the slon to restart. Then the slon commits this transactions and waits for any pending events from the failed origin to arrive from a third provider. Slon then continues with the failover (failoverSet_int). The problem was that when slon restarts it repeats the processing the FAILOVER_NODE. It was possible for slon to keep restarting before it gets to be caught up and recording the FAILOVER_NODE event as processed. In this patch we only restart slon if the configuration actually is changed. After the first restart the configuration shouldn't need changing so there should not be anymore restarts. --- src/backend/slony1_funcs.sql | 34 ++++++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/src/backend/slony1_funcs.sql b/src/backend/slony1_funcs.sql index 1519c6f1..6e66c817 100644 --- a/src/backend/slony1_funcs.sql +++ b/src/backend/slony1_funcs.sql @@ -1208,8 +1208,10 @@ declare v_row record; v_row2 record; v_failed boolean; + v_restart_required boolean; begin + v_restart_required:=false; -- -- any nodes other than the backup receiving -- ANY subscription from a failed node @@ -1219,7 +1221,9 @@ begin where sub_provider=p_failed_node and sub_receiver<>p_backup_node and sub_receiver <> ALL (p_failed_nodes); - + if found then + v_restart_required:=true; + end if; -- ---- -- Terminate all connections of the failed node the hard way -- ---- @@ -1231,22 +1235,32 @@ begin update @NAMESPACE@.sl_path set pa_conninfo='' WHERE pa_server=p_failed_node; - + + if found then + v_restart_required:=true; + end if; + v_failed := exists (select 1 from @NAMESPACE@.sl_node where no_failed=true and no_id=p_failed_node); - if not v_failed then + if not v_failed then update @NAMESPACE@.sl_node set no_failed=true where no_id = ANY (p_failed_nodes) - and no_failed=false; + and no_failed=false; + if found then + v_restart_required:=true; + end if; end if; - -- Rewrite sl_listen table - perform @NAMESPACE@.RebuildListenEntries(); - -- ---- - -- Make sure the node daemon will restart - -- ---- - notify "_@CLUSTERNAME@_Restart"; + if v_restart_required then + -- Rewrite sl_listen table + perform @NAMESPACE@.RebuildListenEntries(); + + -- ---- + -- Make sure the node daemon will restart + -- ---- + notify "_@CLUSTERNAME@_Restart"; + end if; -- ---- From 151c78eaf85046c39df278115980cc89a3cb7bfa Mon Sep 17 00:00:00 2001 From: Steve Singer Date: Wed, 14 Aug 2013 20:37:22 -0400 Subject: [PATCH 09/15] bug 309 - make sure sl_failover_targets gets created when upgrading to 2.2 --- src/backend/slony1_funcs.sql | 55 +++++++++++++++++++++++++++++++++++- 1 file changed, 54 insertions(+), 1 deletion(-) diff --git a/src/backend/slony1_funcs.sql b/src/backend/slony1_funcs.sql index 1519c6f1..e0a63f6b 100644 --- a/src/backend/slony1_funcs.sql +++ b/src/backend/slony1_funcs.sql @@ -5493,7 +5493,60 @@ create table @NAMESPACE@.sl_components ( STYPE=text, INITCOND='' ); - end if; + end if; + if not exists (select 1 from information_schema.views where table_schema='_@CLUSTERNAME@_' and table_name='sl_failover_targets') then + create view @NAMESPACE@.sl_failover_targets as + select set_id, + set_origin as set_origin, + sub1.sub_receiver as backup_id + + FROM + @NAMESPACE@.sl_subscribe sub1 + ,@NAMESPACE@.sl_set set1 + where + sub1.sub_set=set_id + and sub1.sub_forward=true + --exclude candidates where the set_origin + --has a path a node but the failover + --candidate has no path to that node + and sub1.sub_receiver not in + (select p1.pa_client from + @NAMESPACE@.sl_path p1 + left outer join @NAMESPACE@.sl_path p2 on + (p2.pa_client=p1.pa_client + and p2.pa_server=sub1.sub_receiver) + where p2.pa_client is null + and p1.pa_server=set_origin + and p1.pa_client<>sub1.sub_receiver + ) + and sub1.sub_provider=set_origin + --exclude any subscribers that are not + --direct subscribers of all sets on the + --origin + and sub1.sub_receiver not in + (select direct_recv.sub_receiver + from + + (--all direct receivers of the first set + select subs2.sub_receiver + from @NAMESPACE@.sl_subscribe subs2 + where subs2.sub_provider=set1.set_origin + and subs2.sub_set=set1.set_id) as + direct_recv + inner join + (--all other sets from the origin + select set_id from @NAMESPACE@.sl_set set2 + where set2.set_origin=set1.set_origin + and set2.set_id<>sub1.sub_set) + as othersets on(true) + left outer join @NAMESPACE@.sl_subscribe subs3 + on(subs3.sub_set=othersets.set_id + and subs3.sub_forward=true + and subs3.sub_provider=set1.set_origin + and direct_recv.sub_receiver=subs3.sub_receiver) + where subs3.sub_receiver is null + ); + end if; return p_old; end; $$ language plpgsql; From 4c5283b36b28469217eb63c5b5506b4bd5e8d323 Mon Sep 17 00:00:00 2001 From: Steve Singer Date: Mon, 19 Aug 2013 16:29:45 -0400 Subject: [PATCH 10/15] Bug 311 - Don't include performance.sgml ( 'man') file twice in the tar --- tools/build_release.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/build_release.sh b/tools/build_release.sh index 2b8ccbff..50eaf470 100755 --- a/tools/build_release.sh +++ b/tools/build_release.sh @@ -75,7 +75,7 @@ then fi cd .. -tar -cjf slony1-$REL_VERSION-docs.tar.bz2 slony1-$REL_VERSION/doc/adminguide/*html slony1-$REL_VERSION/doc/adminguide/*man* slony1-$REL_VERSION/doc/adminguide/*png slony1-$REL_VERSION/doc/adminguide/*css slony1-$REL_VERSION/doc/adminguide/slony.pdf +tar -cjf slony1-$REL_VERSION-docs.tar.bz2 slony1-$REL_VERSION/doc/adminguide/*html slony1-$REL_VERSION/doc/adminguide/man[0-9] slony1-$REL_VERSION/doc/adminguide/*png slony1-$REL_VERSION/doc/adminguide/*css slony1-$REL_VERSION/doc/adminguide/slony.pdf cd slony1-$REL_VERSION make distclean cd .. From 98e4c35945dd6781417fca4ef237b8372bd5570c Mon Sep 17 00:00:00 2001 From: Steve Singer Date: Mon, 19 Aug 2013 16:37:29 -0400 Subject: [PATCH 11/15] release notes update --- RELEASE | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/RELEASE b/RELEASE index d027f6ed..1f97cc4c 100644 --- a/RELEASE +++ b/RELEASE @@ -4,11 +4,10 @@ - Bug 305 :: set session_replication_role to local when applying DDL_SCRIPT on a replica -*** TBD - - Bug 304 :: Record sequence values each time SQL is recorded in ~sl_log_script~, and set those values before invoking the SQL on the replica. +- Bug 310 :: Fixing issue where the FAILOVER command can put slon in a restart loop *** 2.2.0 b5 From 1eea946ffc8bc9fcef71b0b957fa5082d8293d7f Mon Sep 17 00:00:00 2001 From: Steve Singer Date: Mon, 19 Aug 2013 16:55:46 -0400 Subject: [PATCH 12/15] Updating version numbers to 2.2.0 rc1 --- config.h.in | 6 +++--- config_msvc.h | 6 +++--- configure.ac | 2 +- src/backend/slony1_funcs.def | 26 +++++++++++++------------- src/backend/slony1_funcs.sql | 2 +- 5 files changed, 21 insertions(+), 21 deletions(-) diff --git a/config.h.in b/config.h.in index b2b687da..d9808890 100644 --- a/config.h.in +++ b/config.h.in @@ -12,9 +12,9 @@ #ifndef SLONY_I_CONFIG_H #define SLONY_I_CONFIG_H -#define SLONY_I_VERSION_STRING "2.2.0.b5" -#define SLONY_I_VERSION_STRING_DEC 2,2,0,b5 -#define SLONY_I_FUNC_VERSION_STRING 2_2_0_b5 +#define SLONY_I_VERSION_STRING "2.2.0.rc1" +#define SLONY_I_VERSION_STRING_DEC 2,2,0,rc1 +#define SLONY_I_FUNC_VERSION_STRING 2_2_0_rc1 #ifndef PG_VERSION_MAJOR #define PG_VERSION_MAJOR 0 diff --git a/config_msvc.h b/config_msvc.h index caf47311..b3299e41 100644 --- a/config_msvc.h +++ b/config_msvc.h @@ -4,9 +4,9 @@ #include -#define SLONY_I_VERSION_STRING "2.2.0.b5" -#define SLONY_I_VERSION_STRING_DEC 2,2,0,b5 -#define SLONY_I_FUNC_VERSION_STRING 2_2_0_b5 +#define SLONY_I_VERSION_STRING "2.2.0.rc1" +#define SLONY_I_VERSION_STRING_DEC 2,2,0,rc1 +#define SLONY_I_FUNC_VERSION_STRING 2_2_0_rc1 #if PG_VERSION_NUM >= 90300 #define HAVE_GETACTIVESNAPSHOT 1 diff --git a/configure.ac b/configure.ac index 8e30cd44..fe07441a 100644 --- a/configure.ac +++ b/configure.ac @@ -11,7 +11,7 @@ # ---------- m4_define([SLONREL_VERSION], esyscmd([echo "$Name: $" | \ sed -e 's/\:\ REL_//' -e 's/\$//g' -e 's/_/./g' -e 's/\./\_/3' \ - -e 's/\ //g' -e s/\:/`echo 2.2.0.b5`/ | tr -d '\n'])) + -e 's/\ //g' -e s/\:/`echo 2.2.0.rc1`/ | tr -d '\n'])) m4_pattern_allow([^SLON_AC_]) diff --git a/src/backend/slony1_funcs.def b/src/backend/slony1_funcs.def index 0f832115..5dd5a8ad 100644 --- a/src/backend/slony1_funcs.def +++ b/src/backend/slony1_funcs.def @@ -1,14 +1,14 @@ EXPORTS -_Slony_I_2_2_0_b5_createEvent -_Slony_I_2_2_0_b5_getModuleVersion -_Slony_I_2_2_0_b5_denyAccess -_Slony_I_2_2_0_b5_lockedSet -_Slony_I_2_2_0_b5_getLocalNodeId -_Slony_I_2_2_0_b5_killBackend -_Slony_I_2_2_0_b5_seqtrack -_Slony_I_2_2_0_b5_logTrigger -_Slony_I_2_2_0_b5_resetSession -_Slony_I_2_2_0_b5_logApply -_Slony_I_2_2_0_b5_logApplySetCacheSize -_Slony_I_2_2_0_b5_logApplySaveStats -_Slony_I_2_2_0_b5_slon_decode_tgargs +_Slony_I_2_2_0_rc1_createEvent +_Slony_I_2_2_0_rc1_getModuleVersion +_Slony_I_2_2_0_rc1_denyAccess +_Slony_I_2_2_0_rc1_lockedSet +_Slony_I_2_2_0_rc1_getLocalNodeId +_Slony_I_2_2_0_rc1_killBackend +_Slony_I_2_2_0_rc1_seqtrack +_Slony_I_2_2_0_rc1_logTrigger +_Slony_I_2_2_0_rc1_resetSession +_Slony_I_2_2_0_rc1_logApply +_Slony_I_2_2_0_rc1_logApplySetCacheSize +_Slony_I_2_2_0_rc1_logApplySaveStats +_Slony_I_2_2_0_rc1_slon_decode_tgargs diff --git a/src/backend/slony1_funcs.sql b/src/backend/slony1_funcs.sql index 6692a4a8..5732f0c6 100644 --- a/src/backend/slony1_funcs.sql +++ b/src/backend/slony1_funcs.sql @@ -493,7 +493,7 @@ as $$ begin return @NAMESPACE@.slonyVersionMajor()::text || '.' || @NAMESPACE@.slonyVersionMinor()::text || '.' || - @NAMESPACE@.slonyVersionPatchlevel()::text || '.b5' ; + @NAMESPACE@.slonyVersionPatchlevel()::text || '.rc1' ; end; $$ language plpgsql; comment on function @NAMESPACE@.slonyVersion() is From fe6e44d2f1cb25c6fbb8cb4bce7079979b9e6c5b Mon Sep 17 00:00:00 2001 From: Steve Singer Date: Tue, 20 Aug 2013 10:06:22 -0400 Subject: [PATCH 13/15] variable declarations must be at the start of the block (VC does still enforces this) --- src/backend/slony1_funcs.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/backend/slony1_funcs.c b/src/backend/slony1_funcs.c index 91792b67..5b287901 100644 --- a/src/backend/slony1_funcs.c +++ b/src/backend/slony1_funcs.c @@ -1084,13 +1084,15 @@ versionFunc(logApply)(PG_FUNCTION_ARGS) char query[1024]; Oid argtypes[3]; + void * plan=NULL; + argtypes[0] = INT4OID; argtypes[1] = INT4OID; argtypes[2] = INT8OID; snprintf(query,1023,"select %s.sequenceSetValue($1," \ "$2,NULL,$3); ",tg->tg_trigger->tgargs[0]); - void * plan = SPI_prepare(query,3,argtypes); + plan = SPI_prepare(query,3,argtypes); if ( plan == NULL ) { From e222fe952521933157ebfe9be4db243cc6738b97 Mon Sep 17 00:00:00 2001 From: Steve Singer Date: Tue, 20 Aug 2013 15:31:02 -0400 Subject: [PATCH 14/15] release notes update --- RELEASE | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/RELEASE b/RELEASE index 1f97cc4c..adb7c4c9 100644 --- a/RELEASE +++ b/RELEASE @@ -1,12 +1,14 @@ #+OPTIONS: ^:{} * Slony-I Release Notes -*** +*** 2.2.0 rc 1 - Bug 305 :: set session_replication_role to local when applying DDL_SCRIPT on a replica - Bug 304 :: Record sequence values each time SQL is recorded in ~sl_log_script~, and set those values before invoking the SQL on the replica. +- Bug 309 :: create sl_failover_targets when upgrading 2.1.x to 2.2.0 + - Bug 310 :: Fixing issue where the FAILOVER command can put slon in a restart loop *** 2.2.0 b5 From 6d0a5624f021e66121969866ca5cf7d5bcb4d219 Mon Sep 17 00:00:00 2001 From: Rose Nancy Date: Tue, 27 Aug 2013 17:38:49 -0400 Subject: [PATCH 15/15] BUG308 : added TCP KEEP ALIVE configurations in slon.conf-sample --- share/slon.conf-sample | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/share/slon.conf-sample b/share/slon.conf-sample index 6fe091dd..65124e01 100644 --- a/share/slon.conf-sample +++ b/share/slon.conf-sample @@ -111,3 +111,21 @@ # Should slon run the monitoring thread? # monitor_threads=true + +# TCP keep alive configurations +# Enable sending of TCP keep alive between slon and the PostgreSQL backends +# tcp_keepalive = true + +# The number of seconds after which a TCP keep alive is sent across an idle +# connection. tcp_keepalive must be enabled for this to take effect. Default +# value of 0 means use operating system default +# tcp_keepalive_idle = 0 + +# The number of keep alive requests to the server that can be lost before +# the connection is declared dead. tcp_keepalive must be on.Default value +# of 0 means use operating system default +# tcp_keepalive_count = 0 + +# The number of seconds in between TCP keep alive requests. tcp_keepalive +# must be enabled. Default value of 0 means use operating system defaut +# tcp_keepalive_interval = 0