Hi Andrew, Lars,

Thanks for the comments!

For the convenience of further development, before we reach the
consensus on the cib syntax, I temporarily implemented the syntax as the
following -- It's somewhat different from my previous idea. :-)
Please see also the attached patch which includes the schema, the
internal data structure and the functions for unpacking configuration.

The definitions of tickets:
<cib ...>
  <configuration>
     ...
    <tickets>
      <ticket id="ticketA" loss-policy="stop"/>
      <ticket id="ticketB" loss-policy="fence"/>
      <ticket id="ticketC" loss-policy="freeze"/>
    </tickets>
...

The state of tickets:

...
  <status>
    <cluster_state>
      <transient_attributes id="cluster">
        <instance_attributes id="status-cluster">
          <nvpair id="status-cluster-granted-ticket-ticketA"
name="granted-ticket-ticketA" value="true"/>
          <nvpair id="status-cluster-last-granted-ticket-ticketA"
name="last-granted-ticket-ticketA" value="1305259696"/>
          <nvpair id="status-cluster-granted-ticket-ticketB"
name="granted-ticket-ticketB" value="1"/>
          <nvpair id="status-cluster-last-granted-ticket-ticketB"
name="last-granted-ticket-ticketB" value="1305259760"/>
        </instance_attributes>
      </transient_attributes>
    </cluster_state>
    <node_state...
...

Even though I've done some code for it, it's still open for discussion.
So if there's anything you think is inappropriate, or I misunderstood
any of your points in the long mails, please let me know. ;-)

The "last-granted" time would be automatically set whenever the ticket
is granted. I'm not sure that we need to store the expiration time in
cib and even in PE data, given handling expiration in PE is not
preferred. Unless the facilities really need to store the expiration
time in cib. Anyway, That's trivial if it's stored in "cluster_state".
That would not even affect the schema.

Other replies and issues below:

On 05/12/11 19:47, Lars Marowsky-Bree wrote:
> On 2011-04-29T03:33:00, "Gao,Yan" <y...@novell.com> wrote:
> 
>>> Yes; a ticket section, just like that.
>> All right. How about the schema:
>>     <element name="configuration">
>>       <interleave>
>> ...
>>         <element name="tickets">
>>           <zeroOrMore>
>>             <element name="ticket_set">
>>               <externalRef href="nvset.rng"/>
>>             </element>
>>           </zeroOrMore>
>>         </element>
>> ...
> 
> Makes sense to me.
Because we would need to set "loss-policy" and probably the
"owned-policy"/"bid" which you mentioned, one nvpair for each ticket is
not sufficient and clear.

> 
>>> Personally, I lean towards this. (Andrew has expressed a wish to do
>>> without the "rsc_" prefix, so lets drop this ;-)
>> Well then, how about "ticket_dep" or "ticket_req"?
> 
> The first sounds a bit better to me.
Still not quite sure if we have to introduce an new kind of constraint,
or just improve the "colocation"/"order" which might reuse some existing
code...

> 
>>> Not sure the kind="Deadman" is actually required, but it probably makes
>>> sense to be able to switch off the big hammer for debugging purposes.
>>> ;-)
>> I was thinking it's for switching on/off "immediately fence once the
>> dependency is no longer satisfied".
> 
> Agreed. I was saying that this is only for debugging purposes.
This might not be so useful if we can set "loss-policy" from ticket
definition.

> 
>>>
>>> I don't see why any resource would depend on several tickets; but I can
>>> see a use case for wanting to depend on _not_ owning a ticket, similar
>>> to the node attributes. And the resource would need a role, obviously.
>> OK. The schema I can imagine:
>>
>>   <define name="element-ticket_dep">
>>     <element name="ticket_dep">
>>       <attribute name="id"><data type="ID"/></attribute>
>>       <choice>
>>         <oneOrMore>
>>           <ref name="element-resource-set"/>
>>         </oneOrMore>
>>         <group>
>>           <attribute name="rsc"><data type="IDREF"/></attribute>
>>           <optional>
>>             <attribute name="rsc-role">
>>               <ref name="attribute-roles"/>
>>             </attribute>
>>           </optional>
>>         </group>
>>       </choice>
>>       <attribute name="ticket"><text/></attribute>
> 
> Actually, if we were to define a new type for the ticket list, we could
> reference the id of the ticket element here and only allow configured
> tickets.
Right.

> 
>>> A site that does not own a ticket but has a non-zero value for it
>>> defined will request the ticket from the CTR; the CTR will grant it to
>>> the site with the highest bid (but not to a site with 0)
>> The site with the highest "bid" is being revoked the ticket.
> 
> Hm? No, I thought the site with the highest bid would be granted the
> ticket, not have it revoked.

Sorry, I misunderstood that. Any state change should be due to the
change of "bid"s, right?

As far as I understand, a "bid" value determines the behaviors of both
the cluster transition and the CTR. For example:
A site has the ticket, now its ticket is about to be revoked. There're
two possibilities:

- There's a higher bid from another site now:
   CTR sets "granted-ticket-ticketA=false" for this site. Pengine
invokes the specified "loss-policy". When the transition is successfully
done, CTR sets "granted-ticket-ticketA=true" for the site with the
higher bid. -- Can CTR know if/when the transition is successfully done?

2. The site changes its bid to "0" for any reason to give up the ticket.
   Pengine stops the relevant resources. When the transition is
successfully done, CTR is aware of that and sets
"granted-ticket-ticketA=false" for this site and then sets
"granted-ticket-ticketA=true" for the site with the higher bid.

So CTR should be able to coordinate these actions?

Should the "bid" go into "ticket" definition (that could cover the
owned-policy=(start|stop) you mentioned, right?) ? Or probably into
"cluster_state"? Or as an "instance_attribute" of a CTR?

> 
>> Should it clear the "bid" also? Otherwise it will get the ticket again
>> soon after?
> 
> My thinking was that the "bid" is actually an on-going event/finite
> state machine. Whenever a state change occurs - connection dropped
> somewhere, bid value changed, a site becomes non-quorate and gives up
> its ticket, etc - the CTRs that are still alive re-evaluate the grants
> and decide anew where the tickets go.
That means the CTRs would need to record the state of every site along
with the current bid values to determine its next actions? Sounds a bit
complex. If that is unavoidable, the finite state machine should be
really reliable and cover all possible states.

> 
>>> (Tangent - ownership appears to belong to the status section; the value
>>> seems belongs to the cib->ticket section(?).)
>> Perhaps. Although there's no appropriate place to set a cluster-wide
>> attribute in the status section so far.
> 
> Right, and it is actually not so easy, since the status section is
> reconstructed from each node at times.
I think the introduced "cluster_state" could be created when the first
ticket is granted.

> 
>> Other solutions are:
>> A "ticket" is not a nvpair. It is
>>
>> - An object with "ownership" and "bid" attributes.
>> Or:
>> - A nvpair-set which includes the "ownership" and "bid" nvpairs.
> 
> Both might work. The first - an element with several attributes - might
> work best, I think, since it's a bit cleaner, and would allow us to do
> more validation when dependencies are defined.
Indeed.

Thanks,
  Yan
-- 
Gao,Yan <y...@novell.com>
Software Engineer
China Server Team, SUSE.
diff -r d406ba8f8101 include/crm/msg_xml.h
--- a/include/crm/msg_xml.h	Tue May 03 17:10:26 2011 +0200
+++ b/include/crm/msg_xml.h	Fri May 13 19:41:13 2011 +0800
@@ -126,6 +126,7 @@
 #define XML_CIB_TAG_OPCONFIG		"op_defaults"
 #define XML_CIB_TAG_RSCCONFIG   	"rsc_defaults"
 #define XML_CIB_TAG_ACLS   		"acls"
+#define XML_CIB_TAG_TICKETS   		"tickets"
 
 #define XML_CIB_TAG_STATE         	"node_state"
 #define XML_CIB_TAG_NODE          	"node"
@@ -291,6 +292,11 @@
 #define XML_ACL_ATTR_XPATH		"xpath"
 #define XML_ACL_ATTR_ATTRIBUTE		"attribute"
 
+#define XML_CIB_TAG_TICKET   		"ticket"
+#define XML_TICKET_ATTR_LOSS_POLICY	"loss-policy"
+
+#define XML_CIB_TAG_CLUSTER_STATE	"cluster_state"
+
 #include <crm/common/xml.h> 
 
 #define ID(x) crm_element_value(x, XML_ATTR_ID)
diff -r d406ba8f8101 include/crm/pengine/status.h
--- a/include/crm/pengine/status.h	Tue May 03 17:10:26 2011 +0200
+++ b/include/crm/pengine/status.h	Fri May 13 19:41:13 2011 +0800
@@ -26,6 +26,7 @@ typedef struct node_s node_t;
 typedef struct pe_action_s action_t;
 typedef struct pe_action_s pe_action_t;
 typedef struct resource_s resource_t;
+typedef struct ticket_s ticket_t;
 
 typedef enum no_quorum_policy_e {
 	no_quorum_freeze,
@@ -89,6 +90,8 @@ typedef struct pe_working_set_s
 
 		GHashTable *config_hash;
 		GHashTable *domains;
+		GHashTable *tickets;
+		GHashTable *attrs;
 		
 		GListPtr nodes;
 		GListPtr resources;
@@ -287,6 +290,19 @@ typedef struct notify_data_s {
 		
 } notify_data_t;
 
+enum loss_ticket_policy_e {
+	loss_ticket_fence,
+	loss_ticket_stop,
+	loss_ticket_freeze
+};
+
+struct ticket_s {
+	const char *id;
+	enum loss_ticket_policy_e loss_policy;
+	gboolean granted;
+	time_t last_granted;
+};
+
 gboolean cluster_status(pe_working_set_t *data_set);
 extern void set_working_set_defaults(pe_working_set_t *data_set);
 extern void cleanup_calculations(pe_working_set_t *data_set);
diff -r d406ba8f8101 lib/cib/cib_utils.c
--- a/lib/cib/cib_utils.c	Tue May 03 17:10:26 2011 +0200
+++ b/lib/cib/cib_utils.c	Fri May 13 19:41:13 2011 +0800
@@ -52,6 +52,7 @@ struct config_root_s known_paths[] = {
     { XML_CIB_TAG_STATUS,       "/cib",               "//cib/status" },
     { XML_CIB_TAG_CONFIGURATION,"/cib",               "//cib/configuration" },
     { XML_CIB_TAG_CRMCONFIG,    "/cib/configuration", "//cib/configuration/crm_config" },
+    { XML_CIB_TAG_TICKETS,      "/cib/configuration", "//cib/configuration/tickets" },
     { XML_CIB_TAG_NODES,        "/cib/configuration", "//cib/configuration/nodes" },
     { XML_CIB_TAG_DOMAINS,      "/cib/configuration", "//cib/configuration/domains" },
     { XML_CIB_TAG_RESOURCES,    "/cib/configuration", "//cib/configuration/resources" },
diff -r d406ba8f8101 lib/pengine/status.c
--- a/lib/pengine/status.c	Tue May 03 17:10:26 2011 +0200
+++ b/lib/pengine/status.c	Fri May 13 19:41:13 2011 +0800
@@ -59,6 +59,8 @@ cluster_status(pe_working_set_t *data_se
 {
 	xmlNode * config          = get_object_root(
 		XML_CIB_TAG_CRMCONFIG,   data_set->input);
+	xmlNode * cib_tickets     = get_object_root(
+		XML_CIB_TAG_TICKETS,     data_set->input);
 	xmlNode * cib_nodes       = get_object_root(
 		XML_CIB_TAG_NODES,       data_set->input);
 	xmlNode * cib_resources   = get_object_root(
@@ -107,6 +109,7 @@ cluster_status(pe_working_set_t *data_se
 			 " - fencing and resource management disabled");
 	}
 	
+	unpack_tickets(cib_tickets, data_set);
  	unpack_nodes(cib_nodes, data_set);
 	unpack_domains(cib_domains, data_set);
  	unpack_resources(cib_resources, data_set);
@@ -188,6 +191,14 @@ cleanup_calculations(pe_working_set_t *d
 	if(data_set->config_hash != NULL) {
 		g_hash_table_destroy(data_set->config_hash);
 	}
+
+	if(data_set->tickets) {
+		g_hash_table_destroy(data_set->tickets);
+	}
+
+	if(data_set->attrs) {
+		g_hash_table_destroy(data_set->attrs);
+	}
 	
 	crm_free(data_set->dc_uuid);
 	
diff -r d406ba8f8101 lib/pengine/unpack.c
--- a/lib/pengine/unpack.c	Tue May 03 17:10:26 2011 +0200
+++ b/lib/pengine/unpack.c	Fri May 13 19:41:13 2011 +0800
@@ -192,6 +192,52 @@ unpack_config(xmlNode *config, pe_workin
     return TRUE;
 }
 
+static void destroy_ticket(gpointer data)
+{
+    ticket_t *ticket = data;
+    crm_free(ticket);
+}
+
+gboolean
+unpack_tickets(xmlNode *xml_tickets, pe_working_set_t *data_set)
+{
+    xmlNode *xml_ticket = NULL;
+
+    crm_info("Unpacking tickets");
+    data_set->tickets = g_hash_table_new_full(
+	crm_str_hash, g_str_equal, g_hash_destroy_str, destroy_ticket);
+    
+    for(xml_ticket = __xml_first_child(xml_tickets); xml_ticket != NULL; xml_ticket = __xml_next(xml_ticket)) {
+	if(crm_str_eq((const char *)xml_ticket->name, XML_CIB_TAG_TICKET, TRUE)) {
+	    ticket_t *ticket = NULL;
+	    const char *value = NULL; 
+
+	    crm_malloc0(ticket, sizeof(ticket_t));
+	    if(ticket == NULL) {
+		return FALSE;
+	    }
+
+	    ticket->id = crm_element_value(xml_ticket, XML_ATTR_ID);
+
+	    value = crm_element_value(xml_ticket, XML_TICKET_ATTR_LOSS_POLICY);
+	    if(safe_str_eq(value, "fence")) {
+		crm_debug("On loss of ticket '%s': Fence the relevant nodes", ticket->id);
+		ticket->loss_policy = loss_ticket_fence;
+	    } else if(safe_str_eq(value, "freeze")) {
+		crm_debug("On loss of ticket '%s': Freeze the relevant resources", ticket->id);
+		ticket->loss_policy = loss_ticket_freeze;
+	    } else {
+		crm_debug("On loss of ticket '%s': Stop the relevant resources", ticket->id);
+		ticket->loss_policy = loss_ticket_stop;
+	    }
+
+	    g_hash_table_insert(data_set->tickets, crm_strdup(ticket->id), ticket);
+	}
+    }
+    
+    return TRUE;
+}
+
 gboolean
 unpack_nodes(xmlNode * xml_nodes, pe_working_set_t *data_set)
 {
@@ -397,6 +443,37 @@ unpack_resources(xmlNode * xml_resources
     return TRUE;
 }
 
+static void
+get_ticket_state(gpointer key, gpointer value, gpointer user_data)
+{
+    const char *ticket_id = key;
+    ticket_t *ticket = value;
+    GHashTable *attrs = user_data;
+    char *attr_key = NULL;
+    const char *attr_value = NULL;
+
+    attr_key = crm_concat("granted-ticket", ticket_id, '-');
+    attr_value = g_hash_table_lookup(attrs, attr_key);
+
+    if (crm_is_true(attr_value)) {
+	ticket->granted = TRUE;
+	crm_info("We have ticket '%s'", ticket->id);
+    } else {
+	ticket->granted = FALSE;
+	crm_info("We do not have ticket '%s'", ticket->id);
+    }
+
+    crm_free(attr_key);
+
+
+    attr_key = crm_concat("last-granted-ticket", ticket_id, '-');
+    attr_value = g_hash_table_lookup(attrs, attr_key);
+    if(attr_value) {
+ 	ticket->last_granted = crm_parse_int(attr_value, 0);
+    }
+
+    crm_free(attr_key);
+}
 
 /* remove nodes that are down, stopping */
 /* create +ve rsc_to_node constraints between resources and the nodes they are running on */
@@ -413,7 +490,23 @@ unpack_status(xmlNode * status, pe_worki
     node_t  *this_node   = NULL;
 	
     crm_debug_3("Beginning unpack");
+
+    data_set->attrs = g_hash_table_new_full(
+	    crm_str_hash, g_str_equal, g_hash_destroy_str, g_hash_destroy_str);
+
     for(node_state = __xml_first_child(status); node_state != NULL; node_state = __xml_next(node_state)) {
+	if(crm_str_eq((const char *)node_state->name, XML_CIB_TAG_CLUSTER_STATE, TRUE)) {
+    	    xmlNode *cluster_state = node_state;
+
+	    attrs = find_xml_node(cluster_state, XML_TAG_TRANSIENT_NODEATTRS, FALSE);
+
+	    unpack_instance_attributes(
+			data_set->input, attrs, XML_TAG_ATTR_SETS, NULL,
+			data_set->attrs, NULL, TRUE, data_set->now);
+
+	    g_hash_table_foreach(data_set->tickets, get_ticket_state, data_set->attrs);
+	}
+
 	if(crm_str_eq((const char *)node_state->name, XML_CIB_TAG_STATE, TRUE)) {
 	    id    = crm_element_value(node_state, XML_ATTR_ID);
 	    uname = crm_element_value(node_state, XML_ATTR_UNAME);
diff -r d406ba8f8101 lib/pengine/unpack.h
--- a/lib/pengine/unpack.h	Tue May 03 17:10:26 2011 +0200
+++ b/lib/pengine/unpack.h	Fri May 13 19:41:13 2011 +0800
@@ -23,6 +23,8 @@ extern gboolean unpack_resources(
 
 extern gboolean unpack_config(xmlNode *config, pe_working_set_t *data_set);
 
+extern gboolean unpack_tickets(xmlNode *tickets, pe_working_set_t *data_set);
+
 extern gboolean unpack_nodes(xmlNode *xml_nodes, pe_working_set_t *data_set);
 
 extern gboolean unpack_domains(xmlNode *xml_domains, pe_working_set_t *data_set);
diff -r d406ba8f8101 xml/pacemaker-1.1.rng
--- a/xml/pacemaker-1.1.rng	Tue May 03 17:10:26 2011 +0200
+++ b/xml/pacemaker-1.1.rng	Fri May 13 19:41:13 2011 +0800
@@ -20,6 +20,9 @@
 	  </zeroOrMore>
 	</element>
 	<optional>
+	  <ref name="element-tickets"/>
+	</optional>
+	<optional>
 	  <element name="rsc_defaults">
 	    <zeroOrMore>
 	      <element name="meta_attributes">
@@ -103,6 +106,25 @@
     </optional>
   </define>
 
+  <define name="element-tickets">
+    <element name="tickets">
+      <zeroOrMore>
+	<element name="ticket">
+	  <attribute name="id"><data type="ID"/></attribute>
+	  <optional>
+	    <attribute name="loss-policy">
+	      <choice>
+		<value>fence</value>
+		<value>stop</value>
+		<value>freeze</value>
+	      </choice>
+	    </attribute>
+	  </optional>
+	</element>
+      </zeroOrMore>
+    </element>
+  </define>
+
   <define name="element-nodes">
     <element name="nodes">
       <zeroOrMore>
_______________________________________________
Pacemaker mailing list: Pacemaker@oss.clusterlabs.org
http://oss.clusterlabs.org/mailman/listinfo/pacemaker

Project Home: http://www.clusterlabs.org
Getting started: http://www.clusterlabs.org/doc/Cluster_from_Scratch.pdf
Bugs: http://developerbugs.linux-foundation.org/enter_bug.cgi?product=Pacemaker

Reply via email to