Chrissie/others,

Dave reported a bug today which is basically that sync is broken.  I
found the root cause to be some integration issues between quorum and
sync.

After thinking about it today, I came to the conclusion that sync should
always happen regardless of quorum state.  As the code is now, sync only
happens when quorum is achieved.  This leaves incorrect system state and
is a violation of how the sync engine is designed to work.

What I propose is to provide the current quorum state into the sync_init
callback so service engines can make the policy call on what they  want
to do based upon current quorum.  

Any objections to this method?

I've attached a patch which at least fixes sync to work properly and is
a complete forward port of the working sync engine in openais whitetank.

Regards
-steve
Index: exec/apidef.c
===================================================================
--- exec/apidef.c	(revision 1897)
+++ exec/apidef.c	(working copy)
@@ -102,7 +102,7 @@
 	.tpg_groups_mcast = (typedef_tpg_groups_mcast)totempg_groups_mcast_groups,
 	.tpg_groups_reserve = NULL,
 	.tpg_groups_release = NULL,
-	.sync_request = sync_request,
+	.sync_request = NULL, //sync_request,
 	.quorum_is_quorate = corosync_quorum_is_quorate,
 	.quorum_register_callback = corosync_quorum_register_callback,
 	.quorum_unregister_callback = corosync_quorum_unregister_callback,
Index: exec/quorum.c
===================================================================
--- exec/quorum.c	(revision 1897)
+++ exec/quorum.c	(working copy)
@@ -98,6 +98,14 @@
 	}
 }
 
+static void do_nothing (
+        unsigned int *view_list,
+        int view_list_entries,
+        int primary_designated,
+        struct memb_ring_id *ring_id)
+{
+}
+
 int corosync_quorum_initialize (struct quorum_callin_functions *fns,
 				sync_callback_fn_t *sync_callback_fn)
 {
@@ -105,7 +113,7 @@
 		return -1;
 
 	corosync_quorum_fns = fns;
-	*sync_callback_fn = sync_primary_callback_fn;
+	*sync_callback_fn = do_nothing;
 	return 0;
 }
 
Index: exec/sync.c
===================================================================
--- exec/sync.c	(revision 1897)
+++ exec/sync.c	(working copy)
@@ -32,9 +32,6 @@
  * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
  * THE POSSIBILITY OF SUCH DAMAGE.
  */
-
-#include <config.h>
-
 #include <sys/types.h>
 #include <sys/socket.h>
 #include <sys/un.h>
@@ -52,22 +49,21 @@
 #include <netinet/in.h>
 #include <arpa/inet.h>
 
+#include <config.h>
+ 
 #include <corosync/corotypes.h>
 #include <corosync/swab.h>
 #include <corosync/totem/totempg.h>
 #include <corosync/totem/totem.h>
 #include <corosync/lcr/lcr_ifact.h>
 #include <corosync/engine/logsys.h>
+#include <corosync/ipc_gen.h>
 #include "quorum.h"
-
-#include "main.h"
 #include "sync.h"
 
-
 LOGSYS_DECLARE_SUBSYS ("SYNC", LOG_INFO);
 
 #define MESSAGE_REQ_SYNC_BARRIER 0
-#define MESSAGE_REQ_SYNC_REQUEST 1
 
 struct barrier_data {
 	unsigned int nodeid;
@@ -76,6 +72,8 @@
 
 static struct memb_ring_id *sync_ring_id;
 
+static int vsf_none = 0;
+
 static int (*sync_callbacks_retrieve) (int sync_id, struct sync_callbacks *callack);
 
 static struct sync_callbacks sync_callbacks;
@@ -87,7 +85,6 @@
 static int sync_recovery_index = 0;
 
 static void *sync_callback_token_handle = 0;
-static void *sync_request_token_handle;
 
 static struct barrier_data barrier_data_confchg[PROCESSOR_COUNT_MAX];
 
@@ -95,6 +92,8 @@
 
 static struct barrier_data barrier_data_process[PROCESSOR_COUNT_MAX];
 
+static struct openais_vsf_iface_ver0 *vsf_iface;
+
 static int sync_barrier_send (struct memb_ring_id *ring_id);
 
 static int sync_start_process (enum totem_callback_token_type type, void *data);
@@ -116,33 +115,25 @@
 	unsigned int *joined_list, int joined_list_entries,
 	struct memb_ring_id *ring_id);
 
+static void sync_primary_callback_fn (
+        unsigned int *view_list,
+        int view_list_entries,
+        int primary_designated,
+        struct memb_ring_id *ring_id);
+
+
 static struct totempg_group sync_group = {
     .group      = "sync",
     .group_len  = 4
 };
 
 static hdb_handle_t sync_group_handle;
-static char *service_name;
-static unsigned int current_members[PROCESSOR_COUNT_MAX];
-static unsigned int current_members_cnt;
 
 struct req_exec_sync_barrier_start {
 	mar_req_header_t header;
 	struct memb_ring_id ring_id;
 };
 
-struct sync_request {
-	uint32_t name_len;
-	char name[0] __attribute__((aligned(8)));
-};
-
-typedef struct sync_msg {
-	mar_req_header_t header;
-	struct memb_ring_id ring_id;
-	struct sync_request sync_request;
-} sync_msg_t;
-
-
 /*
  * Send a barrier data structure
  */
@@ -261,7 +252,6 @@
 int sync_register (
 	int (*callbacks_retrieve) (int sync_id, struct sync_callbacks *callack),
 	void (*synchronization_completed) (void))
-
 {
 	unsigned int res;
 
@@ -283,13 +273,14 @@
 		log_printf (LOG_LEVEL_ERROR, "Couldn't join group.\n");
 		return (-1);
 	}
-
+		
 	sync_callbacks_retrieve = callbacks_retrieve;
 	sync_synchronization_completed = synchronization_completed;
 	return (0);
 }
 
-void sync_primary_callback_fn (
+
+static void sync_primary_callback_fn (
 	unsigned int *view_list,
 	int view_list_entries,
 	int primary_designated,
@@ -297,6 +288,13 @@
 {
 	int i;
 
+	if (primary_designated) {
+		log_printf (LOG_LEVEL_NOTICE, "This node is within the primary component and will provide service.\n");
+	} else {
+		log_printf (LOG_LEVEL_NOTICE, "This node is within the non-primary component and will NOT provide any services.\n");
+		return;
+	}
+
 	/*
 	 * Execute configuration change for synchronization service
 	 */
@@ -334,15 +332,15 @@
 {
 	struct req_exec_sync_barrier_start *req_exec_sync_barrier_start =
 		(struct req_exec_sync_barrier_start *)iovec[0].iov_base;
-	sync_msg_t *msg = (sync_msg_t *)iovec[0].iov_base;
-
+	unsigned int barrier_completed;
 	int i;
 
+log_printf (LOG_LEVEL_NOTICE, "confchg entries %d\n", barrier_data_confchg_entries);
 	if (endian_conversion_required) {
 		sync_endian_convert (req_exec_sync_barrier_start);
 	}
 
-	int barrier_completed = 1;
+	barrier_completed = 1;
 
 	memcpy (&deliver_ring_id, &req_exec_sync_barrier_start->ring_id,
 		sizeof (struct memb_ring_id));
@@ -355,43 +353,13 @@
 		return;
 	}
 
-	if (msg->header.id == MESSAGE_REQ_SYNC_REQUEST) {
-		if (endian_conversion_required) {
-			swab_mar_uint32_t (&msg->sync_request.name_len);
-		}	
-		/*
-		 * If there is an ongoing sync, abort it. A requested sync is
-		 * only allowed to abort other requested synchronizations,
-		 * not full synchronizations.
-		 */
-		if (sync_processing && sync_callbacks.sync_abort) {
-			sync_callbacks.sync_abort();
-			sync_callbacks.sync_activate = NULL;
-			sync_processing = 0;
-			assert (service_name != NULL);
-			free (service_name);
-			service_name = NULL;
-		}
-
-		service_name = malloc (msg->sync_request.name_len);
-		strcpy (service_name, msg->sync_request.name);
-
-		/*
-		 * Start requested synchronization
-		 */
-		sync_primary_callback_fn (current_members, current_members_cnt,	1,
-			sync_ring_id);
-
-		return;
-	}
-
 	/*
 	 * Set completion for source_addr's address
 	 */
 	for (i = 0; i < barrier_data_confchg_entries; i++) {
 		if (nodeid == barrier_data_process[i].nodeid) {
 			barrier_data_process[i].completed = 1;
-			log_printf (LOG_LEVEL_DEBUG,
+			log_printf (LOG_LEVEL_NOTICE,
 				"Barrier Start Recieved From %d\n",
 				barrier_data_process[i].nodeid);
 			break;
@@ -402,7 +370,7 @@
 	 * Test if barrier is complete
 	 */
 	for (i = 0; i < barrier_data_confchg_entries; i++) {
-		log_printf (LOG_LEVEL_DEBUG,
+		log_printf (LOG_LEVEL_NOTICE,
 			"Barrier completion status for nodeid %d = %d. \n", 
 			barrier_data_process[i].nodeid,
 			barrier_data_process[i].completed);
@@ -411,7 +379,7 @@
 		}
 	}
 	if (barrier_completed) {
-		log_printf (LOG_LEVEL_DEBUG,
+		log_printf (LOG_LEVEL_NOTICE,
 			"Synchronization barrier completed\n");
 	}
 	/*
@@ -420,7 +388,7 @@
 	if (barrier_completed && sync_callbacks.sync_activate) {
 		sync_callbacks.sync_activate ();
 	
-		log_printf (LOG_LEVEL_DEBUG,
+		log_printf (LOG_LEVEL_NOTICE,
 			"Committing synchronization for (%s)\n",
 			sync_callbacks.name);
 	}
@@ -438,7 +406,7 @@
 		 * if sync service found, execute it
 		 */
 		if (sync_processing && sync_callbacks.sync_init) {
-			log_printf (LOG_LEVEL_DEBUG,
+			log_printf (LOG_LEVEL_NOTICE,
 				"Synchronization actions starting for (%s)\n",
 				sync_callbacks.name);
 			sync_service_init (&deliver_ring_id);
@@ -454,7 +422,6 @@
 	unsigned int *joined_list, int joined_list_entries,
 	struct memb_ring_id *ring_id)
 {
-	int i;
 	sync_ring_id = ring_id;
 
 	if (configuration_type != TOTEM_CONFIGURATION_REGULAR) {
@@ -464,107 +431,20 @@
 		sync_callbacks.sync_abort ();
 		sync_callbacks.sync_activate = NULL;
 	}
-	/*
-	 * Save current members and ring ID for later use
-	 */
-	for (i = 0; i < member_list_entries; i++) {
-		current_members[i] = member_list[i];
-	}
-	current_members_cnt = member_list_entries;
 
-	/*
-	 * If no virtual synchrony filter configured, then start
-	 * synchronization process
-	 */
-	if (quorum_none() == 1) {
-		sync_primary_callback_fn (
-			member_list,
-			member_list_entries,
-			1,
-			ring_id);
-	}
+	sync_primary_callback_fn (
+		member_list,
+		member_list_entries,
+		1,
+		ring_id);
 }
-/**
- * TOTEM callback function used to multicast a sync_request
- * message
- * @param type
- * @param _name
- *
- * @return int
- */
-static int sync_request_send (
-	enum totem_callback_token_type type, void *_name)
-{
-	int res;
-	char *name = _name;
-	sync_msg_t msg;
-	struct iovec iovec[2];
-	int name_len;
 
-	ENTER();
-
-	name_len = strlen (name) + 1;
-	msg.header.size = sizeof (msg) + name_len;
-	msg.header.id = MESSAGE_REQ_SYNC_REQUEST;
-
-	if (sync_ring_id == NULL) {
-		log_printf (LOG_LEVEL_ERROR,
-			"%s sync_ring_id is NULL.\n", __func__);
-		return 1;
-	}
-	memcpy (&msg.ring_id, sync_ring_id,	sizeof (struct memb_ring_id));
-	msg.sync_request.name_len = name_len;
-
-	iovec[0].iov_base = (char *)&msg;
-	iovec[0].iov_len = sizeof (msg);
-	iovec[1].iov_base = _name;
-	iovec[1].iov_len = name_len;
-
-	res = totempg_groups_mcast_joined (
-		sync_group_handle, iovec, 2, TOTEMPG_AGREED);
-
-	if (res == 0) {
-		/*
-		 * We managed to multicast the message so delete the token callback
-		 * for the sync request.
-		 */
-		totempg_callback_token_destroy (&sync_request_token_handle);
-	}
-
-	/*
-	 * if we failed to multicast the message, this function will be called
-	 * again.
-	 */
-
-	return (0);
-}
-
 int sync_in_process (void)
 {
 	return (sync_processing);
 }
 
-/**
- * Execute synchronization upon request for the named service
- * @param name
- *
- * @return int
- */
-int sync_request (char *name)
+int sync_primary_designated (void)
 {
-	assert (name != NULL);
-
-	ENTER();
-
-	if (sync_processing) {
-		return -1;
-	}
-
-	totempg_callback_token_create (&sync_request_token_handle,
-		TOTEM_CALLBACK_TOKEN_SENT, 0, /* don't delete after callback */
-		sync_request_send, name);
-
-	LEAVE();
-
-	return 0;
+	return (1);
 }
Index: exec/sync.h
===================================================================
--- exec/sync.h	(revision 1897)
+++ exec/sync.h	(working copy)
@@ -64,11 +64,4 @@
  */
 extern int sync_request (char *name);
 
-extern void sync_primary_callback_fn (
-	unsigned int *view_list,
-	int view_list_entries,
-	int primary_designated,
-	struct memb_ring_id *ring_id);
-
-
 #endif /* SYNC_H_DEFINED */
_______________________________________________
Openais mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/openais

Reply via email to