[Xen-changelog] [xen stable-4.5] oxenstored: replay transaction upon conflict

[patchbot@xxxxxxx]

commit 988929a2cea69dc1003a6eadd3525cc4499793a7
Author:     Jonathan Davies <jonathan.davies@xxxxxxxxxx>
AuthorDate: Thu Mar 23 17:30:42 2017 +0000
Commit:     Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
CommitDate: Wed Apr 5 15:22:33 2017 +0100

    oxenstored: replay transaction upon conflict
    
    The existing transaction merge algorithm keeps track of the least upper 
bound
    (longest common prefix) of all the nodes which have been read and written, 
and
    will re-combine two stores which have disjoint upper bounds. This works 
well for
    small transactions but causes unnecessary conflicts for ones that span a 
large
    subtree, such as the following ones used by the xapi toolstack:
    
     * VM start: creates /vm/... /vss/... /local/domain/...
       The least upper bound of this transaction is / and so all
       these transactions conflict with everything.
    
     * Device hotplug: creates /local/domain/0/... /local/domain/n/...
       The least upper bound of this transaction is /local/domain so
       all these transactions conflict with each other.
    
    If the existing merge algorithm cannot merge and commit, we attempt
    a /replay/ of the failed transaction against the new store.
    
    When we replay the requests we check whether the response sent to the 
client is
    the same as during the first attempt at the transaction. If the responses 
are
    all the same then the transaction replay can be committed. If any differ 
then
    the transaction replay must be aborted and the client must retry.
    
    This algorithm uses the intuition that the transactions made by the 
toolstack
    are designed to be for separate domains, and should fundamentally not 
conflict
    in the sense that they don't read or write any shared keys. By replaying the
    transaction on the server side we do what the client would have to do 
anyway,
    only we can do it quickly without allowing any other requests to interfere.
    
    Performing 300 parallel simulated VM start and shutdowns without this code:
    
    300 parallel starts and shutdowns: 268.92
    
    Performing 300 parallel simulated VM start and shutdowns with this code:
    
    300 parallel starts and shutdowns: 3.80
    
    Reported-by: Juergen Gross <jgross@xxxxxxxx>
    Signed-off-by: Dave Scott <dave@xxxxxxxxxx>
    Signed-off-by: Jonathan Davies <jonathan.davies@xxxxxxxxxx>
    Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
    Reviewed-by: Jon Ludlam <jonathan.ludlam@xxxxxxxxxx>
    Reviewed-by: Euan Harris <euan.harris@xxxxxxxxxx>
    Acked-by: David Scott <dave@xxxxxxxxxx>
---
 tools/ocaml/xenstored/connection.ml |  5 ++++-
 tools/ocaml/xenstored/packet.ml     |  5 +++++
 tools/ocaml/xenstored/process.ml    | 33 +++++++++++++++++++++++++++++++++
 3 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/tools/ocaml/xenstored/connection.ml 
b/tools/ocaml/xenstored/connection.ml
index b4dc9cb..9eaf415 100644
--- a/tools/ocaml/xenstored/connection.ml
+++ b/tools/ocaml/xenstored/connection.ml
@@ -226,7 +226,10 @@ let end_transaction con tid commit =
        let trans = Hashtbl.find con.transactions tid in
        Hashtbl.remove con.transactions tid;
        Logging.end_transaction ~tid ~con:(get_domstr con);
-       if commit then Transaction.commit ~con:(get_domstr con) trans else true
+       match commit with
+       | None -> true
+       | Some transaction_replay_f ->
+               Transaction.commit ~con:(get_domstr con) trans || 
transaction_replay_f con trans
 
 let get_transaction con tid =
        Hashtbl.find con.transactions tid
diff --git a/tools/ocaml/xenstored/packet.ml b/tools/ocaml/xenstored/packet.ml
index 22cae1d..aeae0a4 100644
--- a/tools/ocaml/xenstored/packet.ml
+++ b/tools/ocaml/xenstored/packet.ml
@@ -9,3 +9,8 @@ type response =
        | Ack of (unit -> unit)  (* function is the action to execute after 
sending the ack *)
        | Reply of string
        | Error of string
+
+let response_equal a b =
+       match (a, b) with
+       | (Ack _, Ack _) -> true (* just consider the response, not the 
post-response action *)
+       | (x, y) -> x = y
diff --git a/tools/ocaml/xenstored/process.ml b/tools/ocaml/xenstored/process.ml
index 67cd880..8fbb6b6 100644
--- a/tools/ocaml/xenstored/process.ml
+++ b/tools/ocaml/xenstored/process.ml
@@ -275,6 +275,38 @@ let input_handle_error ~cons ~doms ~fct ~con ~t ~req =
        | (Failure "int_of_string")    -> reply_error "EINVAL"
        | Define.Unknown_operation     -> reply_error "ENOSYS"
 
+(* Replay a stored transaction against a fresh store, check the responses are
+   all equivalent: if so, commit the transaction. Otherwise send the abort to
+   the client. *)
+let transaction_replay c t doms cons =
+       match t.Transaction.ty with
+       | Transaction.No ->
+               error "attempted to replay a non-full transaction";
+               false
+       | Transaction.Full(id, oldroot, cstore) ->
+               let tid = Connection.start_transaction c cstore in
+               let new_t = Transaction.make tid cstore in
+               let con = sprintf "r(%d):%s" id (Connection.get_domstr c) in
+               let perform_exn (request, response) =
+                       let fct = function_of_type_simple_op request.Packet.ty 
in
+                       let response' = input_handle_error ~cons ~doms ~fct 
~con:c ~t:new_t ~req:request in
+                       if not(Packet.response_equal response response') then 
raise Transaction_again in
+               finally
+               (fun () ->
+                       try
+                               Logging.start_transaction ~con ~tid;
+                               List.iter perform_exn 
(Transaction.get_operations t);
+                               Logging.end_transaction ~con ~tid;
+
+                               Transaction.commit ~con new_t
+                       with e ->
+                               info "transaction_replay %d caught: %s" tid 
(Printexc.to_string e);
+                               false
+                       )
+               (fun () ->
+                       Connection.end_transaction c tid None
+               )
+
 let do_watch con t domains cons data =
        let (node, token) = 
                match (split None '\000' data) with
@@ -307,6 +339,7 @@ let do_transaction_end con t domains cons data =
                | _        -> raise Invalid_Cmd_Args
                in
        let success =
+               let commit = if commit then Some (fun con trans -> 
transaction_replay con trans domains cons) else None in
                Connection.end_transaction con (Transaction.get_id t) commit in
        if not success then
                raise Transaction_again;
--
generated by git-patchbot for /home/xen/git/xen.git#stable-4.5

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxx
https://lists.xenproject.org/xen-changelog