Christopher Browne cbbrowne at ca.afilias.info
Wed Apr 9 11:44:58 PDT 2008
Vivek Khera <vivek at khera.org> writes:
> On Apr 8, 2008, at 4:29 PM, Jeff Frost wrote:
>> I think if the default event node wasn't always 1, but in fact the
>> current master, then things would be less painful when you end up
>> with a master that isn't node id 1.
>
> I think it would be even better if there was NO default event node. I
> intentionally avoid node number 1 just for this reason -- I can debug
> my scripts much more easily.

Reviewing the parser, there only seem to be the following slonik
commands that use a default of 1:

  INIT CLUSTER has default ID = 1
  STORE NODE has default EVENT NODE = 1
  DROP NODE has default EVENT NODE = 1
  FAILOVER has default BACKUP NODE = 1
  EXECUTE SCRIPT has default EVENT NODE = 1
  UPDATE FUNCTIONS has default ID = 1
  REPAIR CONFIG has default EVENT NODE = 1
  WAIT FOR EVENT has default WAIT ON = 1

It doesn't take a very big patch to implement the requirement that these:
a) Default to -1, and
b) Error out if left as -1

Index: parser.y
===================================================================
RCS file: /home/cvsd/slony1/slony1-engine/src/slonik/parser.y,v
retrieving revision 1.31
diff -c -u -r1.31 parser.y
--- parser.y	14 Feb 2008 22:21:42 -0000	1.31
+++ parser.y	9 Apr 2008 18:41:05 -0000
@@ -149,6 +149,7 @@
 %type <statement>	stmt_set_move_sequence
 %type <statement>	stmt_subscribe_set
 %type <statement>	stmt_unsubscribe_set
 %type <statement>	stmt_lock_set
 %type <statement>	stmt_unlock_set
 %type <statement>	stmt_move_set
@@ -174,6 +175,7 @@
 %token	K_ADMIN
 %token	K_ALL
 %token	K_BACKUP
 %token	K_CLIENT
 %token	K_CLONE
 %token	K_CLUSTER
@@ -229,6 +231,7 @@
 %token	K_SET
 %token	K_STORE
 %token	K_SUBSCRIBE
 %token	K_SUCCESS
 %token	K_SWITCH
 %token	K_TABLE
@@ -565,8 +568,8 @@
 					{
 						SlonikStmt_init_cluster *new;
 						statement_option opt[] = {
-							STMT_OPTION_INT( O_ID, 1 ),
-							STMT_OPTION_STR( O_COMMENT, "Primary Node 1" ),
+							STMT_OPTION_INT( O_ID, -1 ),
+							STMT_OPTION_STR( O_COMMENT, "Initial Node" ),
 							STMT_OPTION_END
 						};
 
@@ -595,7 +598,7 @@
 						statement_option opt[] = {
 							STMT_OPTION_INT( O_ID, -1 ),
 							STMT_OPTION_STR( O_COMMENT, NULL ),
-							STMT_OPTION_INT( O_EVENT_NODE, 1 ),
+							STMT_OPTION_INT( O_EVENT_NODE, -1 ),
 							STMT_OPTION_END
 						};
 
@@ -624,7 +627,7 @@
 						SlonikStmt_drop_node *new;
 						statement_option opt[] = {
 							STMT_OPTION_INT( O_ID, -1 ),
-							STMT_OPTION_INT( O_EVENT_NODE, 1 ),
+							STMT_OPTION_INT( O_EVENT_NODE, -1 ),
 							STMT_OPTION_END
 						};
 
@@ -652,7 +655,7 @@
 						SlonikStmt_failed_node *new;
 						statement_option opt[] = {
 							STMT_OPTION_INT( O_ID, -1 ),
-							STMT_OPTION_INT( O_BACKUP_NODE, 1 ),
+							STMT_OPTION_INT( O_BACKUP_NODE, -1 ),
 							STMT_OPTION_END
 						};
 					{
@@ -1304,7 +1331,7 @@
 						statement_option opt[] = {
 							STMT_OPTION_INT( O_SET_ID, -1 ),
 							STMT_OPTION_STR( O_FILENAME, NULL ),
-							STMT_OPTION_INT( O_EVENT_NODE, 1 ),
+							STMT_OPTION_INT( O_EVENT_NODE, -1 ),
 							STMT_OPTION_INT( O_EXECUTE_ONLY_ON, -1 ),
 							STMT_OPTION_END
 						};
@@ -1335,7 +1362,7 @@
 					{
 						SlonikStmt_update_functions *new;
 						statement_option opt[] = {
-							STMT_OPTION_INT( O_ID, 1 ),
+							STMT_OPTION_INT( O_ID, -1 ),
 							STMT_OPTION_END
 						};
 
@@ -1361,7 +1388,7 @@
 						SlonikStmt_repair_config *new;
 						statement_option opt[] = {
 							STMT_OPTION_INT( O_SET_ID, -1 ),
-							STMT_OPTION_INT( O_EVENT_NODE, 1 ),
+							STMT_OPTION_INT( O_EVENT_NODE, -1 ),
 							STMT_OPTION_INT( O_EXECUTE_ONLY_ON, -1 ),
 							STMT_OPTION_END
 						};
@@ -1393,7 +1420,7 @@
 						statement_option opt[] = {
 							STMT_OPTION_INT( O_ORIGIN, -1 ),
 							STMT_OPTION_INT( O_WAIT_CONFIRMED, -1 ),
-							STMT_OPTION_INT( O_WAIT_ON, 1 ),
+							STMT_OPTION_INT( O_WAIT_ON, -1 ),
 							STMT_OPTION_INT( O_TIMEOUT, 600 ),
 							STMT_OPTION_END
 						};
Index: slonik.c
===================================================================
RCS file: /home/cvsd/slony1/slony1-engine/src/slonik/slonik.c,v
retrieving revision 1.87
diff -c -u -r1.87 slonik.c
--- slonik.c	14 Feb 2008 22:21:42 -0000	1.87
+++ slonik.c	9 Apr 2008 18:41:05 -0000
@@ -29,6 +29,7 @@
 
 #include "postgres.h"
 #include "libpq-fe.h"
+#include "port.h"
 
 #include "slonik.h"
 #include "config.h"
@@ -315,7 +316,9 @@
 
 					if (stmt->ev_origin < 0)
 					{
-						stmt->ev_origin = 1;
+						printf("%s:%d: Error: require EVENT NODE\n", 
+						       hdr->stmt_filename, hdr->stmt_lno);
+						errors++;
 					}
 					if (stmt->no_id == stmt->ev_origin)
 					{
@@ -335,6 +338,12 @@
 					SlonikStmt_drop_node *stmt =
 					(SlonikStmt_drop_node *) hdr;
 
+					if (stmt->ev_origin < 0)
+					{
+						printf("%s:%d: Error: require EVENT NODE\n", 
+						       hdr->stmt_filename, hdr->stmt_lno);
+						errors++;
+					}
 					if (stmt->ev_origin == stmt->no_id)
 					{
 						printf("%s:%d: Error: "
@@ -352,6 +361,12 @@
 					SlonikStmt_failed_node *stmt =
 					(SlonikStmt_failed_node *) hdr;
 
+					if (stmt->backup_node < 0)
+					{
+						printf("%s:%d: Error: require BACKUP NODE\n", 
+						       hdr->stmt_filename, hdr->stmt_lno);
+						errors++;
+					}
 					if (stmt->backup_node == stmt->no_id)
 					{
 						printf("%s:%d: Error: "
@@ -828,6 +843,21 @@
 						errors++;
 				}
 				break;
+                        case STMT_CANCEL_SUBSCRIPTION:
+			        {
+					SlonikStmt_cancel_subscription *stmt =
+						(SlonikStmt_cancel_subscription *) hdr;
+					if (stmt->sub_setid < 0)
+					{
+						printf("%s:%d: Error: "
+							   "set id must be specified\n",
+							   hdr->stmt_filename, hdr->stmt_lno);
+						errors++;
+					}
+					if (script_check_adminfo(hdr, stmt->sub_node) < 0)
+						errors++;
+				}
+				break;
 
 			case STMT_LOCK_SET:
 				{
@@ -925,7 +955,9 @@
 
 					if (stmt->ev_origin < 0)
 					{
-						stmt->ev_origin = 1;
+						printf("%s:%d: Error: require EVENT NODE\n", 
+						       hdr->stmt_filename, hdr->stmt_lno);
+						errors++;
 					}
 					if (stmt->ddl_setid < 0)
 					{
-- 
output = ("cbbrowne" "@" "acm.org")
http://www3.sympatico.ca/cbbrowne/linuxdistributions.html
Why is  it that  when you  transport something by  car, it's  called a
shipment, but when you transport something by ship, it's called cargo?


More information about the Slony1-general mailing list