Update rebalance/snapshot/heal to use new locking mechanisms

gluster · May 30, 2018 · 4586b4e · 4586b4e
1 parent 5987c9f
commit 4586b4e
Show file tree

Hide file tree

Showing 6 changed files with 44 additions and 64 deletions.
diff --git a/glusterd2/commands/snapshot/snapshot-activate.go b/glusterd2/commands/snapshot/snapshot-activate.go
@@ -153,6 +153,18 @@ func snapshotActivateHandler(w http.ResponseWriter, r *http.Request) {
 	var vol *volume.Volinfo
 
 	snapname := mux.Vars(r)["snapname"]
+
+	txn, err := transaction.NewTxnWithLocks(ctx, snapname)
+	if err != nil {
+		if err == transaction.ErrLockTimeout {
+			restutils.SendHTTPError(ctx, w, http.StatusConflict, err)
+		} else {
+			restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, err)
+		}
+		return
+	}
+	defer txn.Done()
+
 	snapinfo, err := snapshot.GetSnapshot(snapname)
 	if err != nil {
 		restutils.SendHTTPError(ctx, w, http.StatusNotFound, err)
@@ -166,24 +178,16 @@ func snapshotActivateHandler(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	txn := transaction.NewTxn(ctx)
-	defer txn.Cleanup()
 	err = txn.Ctx.Set("snapname", &snapname)
 	if err != nil {
 		log.WithError(err).Error("failed to set snap name in transaction context")
 		restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, err)
 		return
 	}
 
-	lock, unlock, err := transaction.CreateLockSteps(snapname)
-	if err != nil {
-		restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, err)
-		return
-	}
 	//Populating Nodes neeed not be under lock, because snapshot is a read only config
 	txn.Nodes = vol.Nodes()
 	txn.Steps = []*transaction.Step{
-		lock,
 		{
 			DoFunc: "snap-activate.Validate",
 			Nodes:  txn.Nodes,
@@ -198,8 +202,6 @@ func snapshotActivateHandler(w http.ResponseWriter, r *http.Request) {
 			DoFunc: "snap-activate.StoreSnapshot",
 			Nodes:  []uuid.UUID{gdctx.MyUUID},
 		},
-
-		unlock,
 	}
 	err = txn.Ctx.Set("req", &req)
 	if err != nil {

diff --git a/glusterd2/commands/snapshot/snapshot-create.go b/glusterd2/commands/snapshot/snapshot-create.go
@@ -652,38 +652,26 @@ func snapshotCreateHandler(w http.ResponseWriter, r *http.Request) {
 		t := time.Now().UTC()
 		req.SnapName = req.SnapName + t.Format("_GMT_2006_01_02_15_04_05")
 	}
-	volLock, volUnlock := transaction.CreateLockFuncs(req.VolName)
-	// Taking a lock outside the txn as volinfo.Nodes() must also
-	// be populated holding the lock. See issue #510
-	if err := volLock(ctx); err != nil {
+
+	txn, err := transaction.NewTxnWithLocks(ctx, req.VolName, req.SnapName)
+	if err != nil {
 		if err == transaction.ErrLockTimeout {
 			restutils.SendHTTPError(ctx, w, http.StatusConflict, err)
 		} else {
 			restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, err)
 		}
 		return
 	}
-	defer volUnlock(ctx)
+	defer txn.Done()
 
 	vol, e := volume.GetVolume(req.VolName)
 	if e != nil {
 		restutils.SendHTTPError(ctx, w, http.StatusNotFound, err)
 		return
 	}
 
-	txn := transaction.NewTxn(ctx)
-	defer txn.Cleanup()
-
-	snapLock, snapUnlock, err := transaction.CreateLockSteps(req.SnapName)
-	if err != nil {
-		restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, err)
-		return
-	}
-
 	txn.Nodes = vol.Nodes()
-
 	txn.Steps = []*transaction.Step{
-		snapLock,
 		{
 			DoFunc: "snap-create.OriginNodeValidate",
 			Nodes:  []uuid.UUID{gdctx.MyUUID},
@@ -717,7 +705,6 @@ func snapshotCreateHandler(w http.ResponseWriter, r *http.Request) {
 			UndoFunc: "snap-create.UndoStoreSnapshotOnCreate",
 			Nodes:    []uuid.UUID{gdctx.MyUUID},
 		},
-		snapUnlock,
 	}
 	err = txn.Ctx.Set("req", req)
 	if err != nil {

diff --git a/glusterd2/commands/snapshot/snapshot-deactivate.go b/glusterd2/commands/snapshot/snapshot-deactivate.go
@@ -141,23 +141,28 @@ func snapshotDeactivateHandler(w http.ResponseWriter, r *http.Request) {
 	var vol *volume.Volinfo
 
 	snapname := mux.Vars(r)["snapname"]
-	snapinfo, err := snapshot.GetSnapshot(snapname)
+
+	txn, err := transaction.NewTxnWithLocks(ctx, snapname)
 	if err != nil {
-		restutils.SendHTTPError(ctx, w, http.StatusNotFound, err)
+		if err == transaction.ErrLockTimeout {
+			restutils.SendHTTPError(ctx, w, http.StatusConflict, err)
+		} else {
+			restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, err)
+		}
 		return
 	}
+	defer txn.Done()
 
-	vol = &snapinfo.SnapVolinfo
-	txn := transaction.NewTxn(ctx)
-	defer txn.Cleanup()
-	lock, unlock, err := transaction.CreateLockSteps(snapname)
+	snapinfo, err := snapshot.GetSnapshot(snapname)
 	if err != nil {
-		restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, err)
+		restutils.SendHTTPError(ctx, w, http.StatusNotFound, err)
 		return
 	}
+
+	vol = &snapinfo.SnapVolinfo
+
 	txn.Nodes = vol.Nodes()
 	txn.Steps = []*transaction.Step{
-		lock,
 		{
 			DoFunc: "snap-deactivate.Validate",
 			Nodes:  txn.Nodes,
@@ -172,8 +177,6 @@ func snapshotDeactivateHandler(w http.ResponseWriter, r *http.Request) {
 			DoFunc: "snap-deactivate.StoreVolume",
 			Nodes:  []uuid.UUID{gdctx.MyUUID},
 		},
-
-		unlock,
 	}
 	err = txn.Ctx.Set("snapname", &snapname)
 	if err != nil {

diff --git a/plugins/glustershd/rest.go b/plugins/glustershd/rest.go
@@ -67,16 +67,16 @@ func selfhealInfoHandler(w http.ResponseWriter, r *http.Request) {
 	ctx := r.Context()
 	logger := gdctx.GetReqLogger(ctx)
 
-	lock, unlock := transaction.CreateLockFuncs(volname)
-	if err := lock(ctx); err != nil {
+	txn, err := transaction.NewTxnWithLocks(ctx, volname)
+	if err != nil {
 		if err == transaction.ErrLockTimeout {
 			restutils.SendHTTPError(ctx, w, http.StatusConflict, err)
 		} else {
 			restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, err)
 		}
 		return
 	}
-	defer unlock(ctx)
+	defer txn.Done()
 
 	// Validate volume existence
 	volinfo, err := volume.GetVolume(volname)

diff --git a/plugins/rebalance/eventnotify.go b/plugins/rebalance/eventnotify.go
@@ -33,15 +33,13 @@ func HandleEventNotify(status map[string]string) error {
 
 	var rebalinfo *rebalanceapi.RebalInfo
 	var rebalNodeStatus rebalanceapi.RebalNodeStatus
-	ctx := context.TODO()
-
-	lock, unlock := transaction.CreateLockFuncs(volname)
-	if err := lock(ctx); err != nil {
 
+	txn, err := transaction.NewTxnWithLocks(context.TODO(), volname)
+	if err != nil {
 		log.WithError(err).Error("Locking failed. Unable to update store")
 		return err
 	}
-	defer unlock(ctx)
+	defer txn.Done()
 
 	vol, err := volume.GetVolume(volname)
 	if err != nil {

diff --git a/plugins/rebalance/rest.go b/plugins/rebalance/rest.go
@@ -56,16 +56,16 @@ func rebalanceStartHandler(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	lock, unlock := transaction.CreateLockFuncs(volname)
-	if err := lock(ctx); err != nil {
+	txn, err := transaction.NewTxnWithLocks(ctx, volname)
+	if err != nil {
 		if err == transaction.ErrLockTimeout {
 			restutils.SendHTTPError(ctx, w, http.StatusConflict, err)
 		} else {
 			restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, err)
 		}
 		return
 	}
-	defer unlock(ctx)
+	defer txn.Done()
 
 	vol, err := volume.GetVolume(volname)
 	if err != nil {
@@ -85,9 +85,6 @@ func rebalanceStartHandler(w http.ResponseWriter, r *http.Request) {
 
 	// TODO: Check for remove-brick
 
-	txn := transaction.NewTxn(ctx)
-	defer txn.Cleanup()
-
 	// Start the rebalance process on all nodes
 	// Only this node will save the rebalinfo in the store
 
@@ -151,16 +148,16 @@ func rebalanceStopHandler(w http.ResponseWriter, r *http.Request) {
 	// collect inputs from url
 	volname := mux.Vars(r)["volname"]
 
-	lock, unlock := transaction.CreateLockFuncs(volname)
-	if err := lock(ctx); err != nil {
+	txn, err := transaction.NewTxnWithLocks(ctx, volname)
+	if err != nil {
 		if err == transaction.ErrLockTimeout {
 			restutils.SendHTTPError(ctx, w, http.StatusConflict, err)
 		} else {
 			restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, err)
 		}
 		return
 	}
-	defer unlock(ctx)
+	defer txn.Done()
 
 	// Validate rebalance command
 	vol, err := volume.GetVolume(volname)
@@ -181,10 +178,6 @@ func rebalanceStopHandler(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	//A simple transaction to stop rebalance
-	txn := transaction.NewTxn(ctx)
-	defer txn.Cleanup()
-
 	txn.Nodes = vol.Nodes()
 	txn.Steps = []*transaction.Step{
 		{
@@ -236,19 +229,16 @@ func rebalanceStatusHandler(w http.ResponseWriter, r *http.Request) {
 	// collect inputs from url
 	volname := mux.Vars(r)["volname"]
 
-	txn := transaction.NewTxn(ctx)
-	defer txn.Cleanup()
-
-	lock, unlock := transaction.CreateLockFuncs(volname)
-	if err := lock(ctx); err != nil {
+	txn, err := transaction.NewTxnWithLocks(ctx, volname)
+	if err != nil {
 		if err == transaction.ErrLockTimeout {
 			restutils.SendHTTPError(ctx, w, http.StatusConflict, err)
 		} else {
 			restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, err)
 		}
 		return
 	}
-	defer unlock(ctx)
+	defer txn.Done()
 
 	// Validate rebalance command
 	vol, err := volume.GetVolume(volname)