Hi,

The following patch adds a new column to sl_table: tab_lock_order, 
and modifies the order in which tables are locked during all calls 
that require extensive locking (i.e. all those that call 
alterTableForReplication or alterTableRestore on a set of tables): 
they are locked in coalesce(tab_lock_order,0),tab_id order rather 
than random order or just tab_id order.

This means that by default (tab_lock_order null for all tables), they 
are locked in tab_id order (in all those calls rather than just some 
of them), but the order can be overridden by setting tab_lock_order 
appropriately (this allow reordering the locks, as tab_id's cannot be 
easily changed).

Alternatives would be to set tab_lock_order's default value to be 
tab_id (+ not null and maybe unique + remove coalesce and maybe 
tab_id from order by), or to order by coalesce(tab_lock_order,tab_id).

Note that I'm not sure the schema upgrade is the "right way" to do 
it, maybe add_missing_table_field() should be used for this, and 
obviously the schema version needed to be bumped up, not sure what 
the best practice is for that?

Comments welcome,

Jacques.

--- slony1-1.2.12.orig/src/backend/slony1_funcs.sql     Mon Oct 22 
17:19:50 2007
+++ slony1-1.2.12/src/backend/slony1_funcs.sql  Thu Nov 22 00:19:29 2007
@@ -430,7 +430,7 @@
  returns int4
  as '
  begin
-       return 12;
+       return 13;
  end;
  ' language plpgsql;
  comment on function @NAMESPACE at .slonyVersionPatchlevel () is
@@ -1219,6 +1219,7 @@
                         if p_backup_node = 
@NAMESPACE at .getLocalNodeId(''_ at CLUSTERNAME@'') then
                                 for v_row2 in select * from 
@NAMESPACE at .sl_table
                                                 where tab_set = v_row.set_id
+                                               order by 
coalesce(tab_lock_order,0),tab_id
                                 loop
                                         perform 
@NAMESPACE at .alterTableRestore(v_row2.tab_id);
                                 end loop;
@@ -1231,6 +1232,7 @@

                                 for v_row2 in select * from 
@NAMESPACE at .sl_table
                                                 where tab_set = v_row.set_id
+                                               order by 
coalesce(tab_lock_order,0),tab_id
                                 loop
                                         perform 
@NAMESPACE at .alterTableForReplication(v_row2.tab_id);
                                 end loop;
@@ -1380,6 +1382,7 @@
         if p_backup_node = @NAMESPACE at .getLocalNodeId(''_ at CLUSTERNAME@'') then
                 for v_row in select * from @NAMESPACE at .sl_table
                                 where tab_set = p_set_id
+                               order by coalesce(tab_lock_order,0),tab_id
                 loop
                         perform @NAMESPACE at .alterTableRestore(v_row.tab_id);
                 end loop;
@@ -1395,6 +1398,7 @@

                 for v_row in select * from @NAMESPACE at .sl_table
                                 where tab_set = p_set_id
+                               order by coalesce(tab_lock_order,0),tab_id
                 loop
                         perform 
@NAMESPACE at .alterTableForReplication(v_row.tab_id);
                 end loop;
@@ -1484,7 +1488,7 @@
         -- This is us ... time for suicide! Restore all tables to
         -- their original status.
         -- ----
-       for v_tab_row in select * from @NAMESPACE at .sl_table loop
+       for v_tab_row in select * from @NAMESPACE at .sl_table order by 
coalesce(tab_lock_order,0),tab_id loop
                 perform @NAMESPACE at .alterTableRestore(v_tab_row.tab_id);
                 perform @NAMESPACE at .tableDropKey(v_tab_row.tab_id);
         end loop;
@@ -1984,7 +1988,7 @@
                         where T.tab_set = p_set_id
                                 and T.tab_reloid = PGC.oid
                                 and PGC.relnamespace = PGN.oid
-                       order by tab_id
+                       order by coalesce(tab_lock_order,0),tab_id
         loop
                 execute ''create trigger "_ at CLUSTERNAME@_lockedset_'' ||
                                 v_tab_row.tab_id ||
@@ -2060,7 +2064,7 @@
                         where T.tab_set = p_set_id
                                 and T.tab_reloid = PGC.oid
                                 and PGC.relnamespace = PGN.oid
-                       order by tab_id
+                       order by coalesce(tab_lock_order,0),tab_id
         loop
                 execute ''drop trigger "_ at CLUSTERNAME@_lockedset_'' ||
                                 v_tab_row.tab_id || ''" on '' || 
v_tab_row.tab_fqname;
@@ -2214,7 +2218,7 @@
         if v_local_node_id = p_old_origin or v_local_node_id = 
p_new_origin then
                 for v_tab_row in select tab_id from @NAMESPACE at .sl_table
                                 where tab_set = p_set_id
-                               order by tab_id
+                               order by coalesce(tab_lock_order,0),tab_id
                 loop
                         perform 
@NAMESPACE at .alterTableRestore(v_tab_row.tab_id);
                 end loop;
@@ -2362,7 +2366,7 @@
         if v_local_node_id = p_old_origin or v_local_node_id = 
p_new_origin then
                 for v_tab_row in select tab_id from @NAMESPACE at .sl_table
                                 where tab_set = p_set_id
-                               order by tab_id
+                               order by coalesce(tab_lock_order,0),tab_id
                 loop
                         perform 
@NAMESPACE at .alterTableForReplication(v_tab_row.tab_id);
                 end loop;
@@ -2441,7 +2445,7 @@
         -- ----
         for v_tab_row in select tab_id from @NAMESPACE at .sl_table
                         where tab_set = p_set_id
-                       order by tab_id
+                       order by coalesce(tab_lock_order,0),tab_id
         loop
                 perform @NAMESPACE at .alterTableRestore(v_tab_row.tab_id);
                 perform @NAMESPACE at .tableDropKey(v_tab_row.tab_id);
@@ -3711,12 +3715,12 @@
                 -- Create a SYNC event, run the script and generate 
the DDL_SCRIPT event
                 -- ----
                 perform @NAMESPACE at .createEvent(''_ at CLUSTERNAME@'', 
''SYNC'', NULL);
-               perform @NAMESPACE at .alterTableRestore(tab_id) from 
@NAMESPACE at .sl_table where tab_set in (select set_id from 
@NAMESPACE at .sl_set where set_origin = 
@NAMESPACE at .getLocalNodeId(''_ at CLUSTERNAME@''));
+               perform @NAMESPACE at .alterTableRestore(tab_id) from 
@NAMESPACE at .sl_table where tab_set in (select set_id from 
@NAMESPACE at .sl_set where set_origin = 
@NAMESPACE at .getLocalNodeId(''_ at CLUSTERNAME@'')) order by 
coalesce(tab_lock_order,0),tab_id;
         else
                 -- ----
                 -- If doing "only on one node" - restore ALL tables 
irrespective of set
                 -- ----
-               perform @NAMESPACE at .alterTableRestore(tab_id) from 
@NAMESPACE at .sl_table;
+               perform @NAMESPACE at .alterTableRestore(tab_id) from 
@NAMESPACE at .sl_table order by coalesce(tab_lock_order,0),tab_id;
         end if;
         return 1;
  end;
@@ -3743,12 +3747,12 @@
  begin
         perform @NAMESPACE at .updateRelname(p_set_id, p_only_on_node);
         if p_only_on_node = -1 then
-               perform @NAMESPACE at .alterTableForReplication(tab_id) 
from @NAMESPACE at .sl_table where tab_set in (select set_id from 
@NAMESPACE at .sl_set where set_origin = 
@NAMESPACE at .getLocalNodeId(''_ at CLUSTERNAME@''));
+               perform @NAMESPACE at .alterTableForReplication(tab_id) 
from @NAMESPACE at .sl_table where tab_set in (select set_id from 
@NAMESPACE at .sl_set where set_origin = 
@NAMESPACE at .getLocalNodeId(''_ at CLUSTERNAME@'')) order by 
coalesce(tab_lock_order,0),tab_id;

                 return  @NAMESPACE at .createEvent(''_ at CLUSTERNAME@'', 
''DDL_SCRIPT'',
                         p_set_id::text, p_script::text, p_only_on_node::text);
         else
-               perform @NAMESPACE at .alterTableForReplication(tab_id) 
from @NAMESPACE at .sl_table;
+               perform @NAMESPACE at .alterTableForReplication(tab_id) 
from @NAMESPACE at .sl_table order by coalesce(tab_lock_order,0),tab_id;
         end if;
         return NULL;
  end;
@@ -3812,7 +3816,7 @@
         -- ----
         -- Restore all original triggers and rules of all sets
         -- ----
-       for v_row in select * from @NAMESPACE at .sl_table
+       for v_row in select * from @NAMESPACE at .sl_table order by 
coalesce(tab_lock_order,0),tab_id
         loop
                 perform @NAMESPACE at .alterTableRestore(v_row.tab_id);
         end loop;
@@ -3843,7 +3847,7 @@
         -- ----
         -- Put all tables back into replicated mode
         -- ----
-       for v_row in select * from @NAMESPACE at .sl_table
+       for v_row in select * from @NAMESPACE at .sl_table order by 
coalesce(tab_lock_order,0),tab_id
         loop
                 perform @NAMESPACE at .alterTableForReplication(v_row.tab_id);
         end loop;
@@ -4363,7 +4367,7 @@
         -- ----
         for v_tab_row in select tab_id from @NAMESPACE at .sl_table
                         where tab_set = p_sub_set
-                       order by tab_id
+                       order by coalesce(tab_lock_order,0),tab_id
         loop
                 perform @NAMESPACE at .alterTableRestore(v_tab_row.tab_id);
                 perform @NAMESPACE at .tableDropKey(v_tab_row.tab_id);
@@ -5912,6 +5921,14 @@
                                         ) without oids'';
                 execute ''insert into @NAMESPACE at .sl_archive_counter
                                         (ac_num, ac_timestamp) 
values (0, ''''epoch''''::timestamp)'';
+       end if;
+
+       -- ----
+       -- Changes for 1.2.13
+       -- ----
+       if p_old IN (''1.0.2'', ''1.0.5'', ''1.0.6'', ''1.1.0'', 
''1.1.1'', ''1.1.2'', ''1.1.3'',''1.1.5'', ''1.1.6'', ''1.1.7'', 
''1.1.8'', ''1.1.9'', ''1.2.0'', ''1.2.1'', ''1.2.2'', ''1.2.3'', 
''1.2.4'', ''1.2.5'', ''1.2.6'', ''1.2.7'', ''1.2.8'', ''1.2.9'', 
''1.2.10'', ''1.2.11'', ''1.2.12'') then
+               -- Add new column sl_table.tab_lock_order
+               execute ''alter table @NAMESPACE at .sl_table add column 
tab_lock_order integer'';
         end if;

         -- In any version, make sure that the xxidin() functions are 
defined STRICT