From 662ed4f3247ec6fdaba53ea15b9d8347f45838b5 Mon Sep 17 00:00:00 2001 From: Matt Jaffee Date: Mon, 20 Apr 2020 22:12:07 -0500 Subject: [PATCH] invert if statements and fix typos --- server.go | 102 ++++++++++++++++++++++++------------------------- transaction.md | 6 +-- 2 files changed, 52 insertions(+), 56 deletions(-) diff --git a/server.go b/server.go index 143e75e2b..b1b25dbe1 100644 --- a/server.go +++ b/server.go @@ -1033,43 +1033,42 @@ func (srv *Server) StartTransaction(ctx context.Context, id string, timeout time if remote && (node.IsCoordinator || len(srv.cluster.Nodes()) == 1) { return Transaction{}, errors.New("got a remote start call to coordinator or single node cluster... shouldn't ever happen") } - // empty string id should generate an id - if !remote { // we are the coordinator, - if id == "" { - id = uuid.NewV4().String() - } - trns, err := srv.holder.StartTransaction(ctx, id, timeout, exclusive) - if err != nil { - return trns, errors.Wrap(err, "starting transaction") - } - err = srv.SendSync( - &TransactionMessage{ - Action: TRANSACTION_START, - Transaction: trns, - }) - if err != nil { - // try to clean up, but ignore errors - _, errLocal := srv.holder.FinishTransaction(ctx, id) - errBroadcast := srv.SendSync( - &TransactionMessage{ - Action: TRANSACTION_FINISH, - Transaction: trns, - }, - ) - if errLocal != nil || errBroadcast != nil { - srv.logger.Printf("error(s) while trying to clean up transaction which failed to start, local: %v, broadcast: %v", - errLocal, - errBroadcast, - ) - } - return trns, errors.Wrap(err, "broadcasting transaction start") - } - return trns, nil - } else { // remote + if remote { return srv.holder.StartTransaction(ctx, id, timeout, exclusive) } + // empty string id should generate an id + if id == "" { + id = uuid.NewV4().String() + } + trns, err := srv.holder.StartTransaction(ctx, id, timeout, exclusive) + if err != nil { + return trns, errors.Wrap(err, "starting transaction") + } + err = srv.SendSync( + &TransactionMessage{ + Action: TRANSACTION_START, + Transaction: trns, + }) + if err != nil { + // try to clean up, but ignore errors + _, errLocal := srv.holder.FinishTransaction(ctx, id) + errBroadcast := srv.SendSync( + &TransactionMessage{ + Action: TRANSACTION_FINISH, + Transaction: trns, + }, + ) + if errLocal != nil || errBroadcast != nil { + srv.logger.Printf("error(s) while trying to clean up transaction which failed to start, local: %v, broadcast: %v", + errLocal, + errBroadcast, + ) + } + return trns, errors.Wrap(err, "broadcasting transaction start") + } + return trns, nil } func (srv *Server) FinishTransaction(ctx context.Context, id string, remote bool) (Transaction, error) { @@ -1081,26 +1080,24 @@ func (srv *Server) FinishTransaction(ctx context.Context, id string, remote bool return Transaction{}, errors.New("got a remote finish call to coordinator or single node cluster... shouldn't ever happen") } - if !remote { - trns, err := srv.holder.FinishTransaction(ctx, id) - if err != nil { - return trns, errors.Wrap(err, "finishing transaction") - } - err = srv.SendSync( - &TransactionMessage{ - Action: TRANSACTION_FINISH, - Transaction: trns, - }, - ) - if err != nil { - srv.logger.Printf("error broadcasting transaction finish: %v", err) - // TODO retry? - } - return trns, nil - } else { // remote + if remote { return srv.holder.FinishTransaction(ctx, id) } - + trns, err := srv.holder.FinishTransaction(ctx, id) + if err != nil { + return trns, errors.Wrap(err, "finishing transaction") + } + err = srv.SendSync( + &TransactionMessage{ + Action: TRANSACTION_FINISH, + Transaction: trns, + }, + ) + if err != nil { + srv.logger.Printf("error broadcasting transaction finish: %v", err) + // TODO retry? + } + return trns, nil } func (srv *Server) Transactions(ctx context.Context) (map[string]Transaction, error) { @@ -1145,9 +1142,8 @@ func (srv *Server) GetTransaction(ctx context.Context, id string, remote bool) ( return Transaction{}, errors.Wrap(err, "contacting remote hosts") } return trns, nil - } else { // remote - return trns, nil } + return trns, nil } // countOpenFiles on operating systems that support lsof. diff --git a/transaction.md b/transaction.md index dd72d4b34..c3bdc27e3 100644 --- a/transaction.md +++ b/transaction.md @@ -92,7 +92,7 @@ anything. When an exclusive transaction is created, it does not necessarily start out in the `active` state. It immediately blocks the starting of new non-exclusive -transactions, but does not transiction to an `active` state until existing +transactions, but does not transition to an `active` state until existing transactions complete. During this time, a GET to it should return: ``` @@ -137,7 +137,7 @@ in sync. If the coordinator doesn't hear back from a node, the request fails. The coordinator only reaches out to active nodes, so if the cluster is in DEGRADED, things can still continue. -If an node is down and comes back up it needs to synchronize its state +If a node is down and comes back up, it needs to synchronize its state with the coordinator (unimplemented). There is a separate TransactionManager and TransactionStore @@ -147,7 +147,7 @@ transactions. The manager handles all the logic (at the node level). Logic related to cluster and remote vs local node is handled by the Server. The Holder contains the TransactionManager, and the Server contains the logic for how to handle external vs intra cluster -requests (remote==true). +requests (remote=true). There is intra-cluster messaging for transactions which is handled with the new TransactionMessage and goes through the usual