From: Amitay Isaacs <amitay@gmail.com>
Date: Fri, 30 May 2014 05:36:28 +0000 (+1000)
Subject: ctdb-locking: Remove multiple lock requests per lock context (part 1)
X-Git-Url: http://git.samba.org/?a=commitdiff_plain;h=a89f3508796d6be8efe45ccc1f9ffee7e4d3f4f3;p=obnox%2Fsamba%2Fsamba-obnox.git

ctdb-locking: Remove multiple lock requests per lock context (part 1)

This was a bad idea and caused out of order scheduling of lock requests.

The logic to append lock requests to existing lock context is already
commented.  Remove the commented code and there is no need to check if
lock_ctx is NULL, since we are always creating a new one.

Signed-off-by: Amitay Isaacs <amitay@gmail.com>
Reviewed-by: Volker Lendecke <vl@samba.org>
---

diff --git a/ctdb/server/ctdb_lock.c b/ctdb/server/ctdb_lock.c
index d6beed86b51..28ba79c0ac3 100644
--- a/ctdb/server/ctdb_lock.c
+++ b/ctdb/server/ctdb_lock.c
@@ -903,54 +903,42 @@ static struct lock_request *ctdb_lock_internal(struct ctdb_context *ctdb,
 		return NULL;
 	}
 
-#if 0
-	/* Disable this optimization to ensure first-in-first-out fair
-	 * scheduling of lock requests */
-
-	/* get a context for this key - search only the pending contexts,
-	 * current contexts might in the middle of processing callbacks */
-	lock_ctx = find_lock_context(ctdb->lock_pending, ctdb_db, key, priority, type);
-#endif
-
-	/* No existing context, create one */
+	lock_ctx = talloc_zero(ctdb, struct lock_context);
 	if (lock_ctx == NULL) {
-		lock_ctx = talloc_zero(ctdb, struct lock_context);
-		if (lock_ctx == NULL) {
-			DEBUG(DEBUG_ERR, ("Failed to create a new lock context\n"));
-			return NULL;
-		}
+		DEBUG(DEBUG_ERR, ("Failed to create a new lock context\n"));
+		return NULL;
+	}
 
-		lock_ctx->type = type;
-		lock_ctx->ctdb = ctdb;
-		lock_ctx->ctdb_db = ctdb_db;
-		lock_ctx->key.dsize = key.dsize;
-		if (key.dsize > 0) {
-			lock_ctx->key.dptr = talloc_memdup(lock_ctx, key.dptr, key.dsize);
-			if (lock_ctx->key.dptr == NULL) {
-				DEBUG(DEBUG_ERR, (__location__ "Memory allocation error\n"));
-				talloc_free(lock_ctx);
-				return NULL;
-			}
-			lock_ctx->key_hash = ctdb_hash(&key);
-		} else {
-			lock_ctx->key.dptr = NULL;
+	lock_ctx->type = type;
+	lock_ctx->ctdb = ctdb;
+	lock_ctx->ctdb_db = ctdb_db;
+	lock_ctx->key.dsize = key.dsize;
+	if (key.dsize > 0) {
+		lock_ctx->key.dptr = talloc_memdup(lock_ctx, key.dptr, key.dsize);
+		if (lock_ctx->key.dptr == NULL) {
+			DEBUG(DEBUG_ERR, (__location__ "Memory allocation error\n"));
+			talloc_free(lock_ctx);
+			return NULL;
 		}
-		lock_ctx->priority = priority;
-		lock_ctx->auto_mark = auto_mark;
-
-		lock_ctx->child = -1;
+		lock_ctx->key_hash = ctdb_hash(&key);
+	} else {
+		lock_ctx->key.dptr = NULL;
+	}
+	lock_ctx->priority = priority;
+	lock_ctx->auto_mark = auto_mark;
 
-		DLIST_ADD_END(ctdb->lock_pending, lock_ctx, NULL);
-		ctdb->lock_num_pending++;
-		CTDB_INCREMENT_STAT(ctdb, locks.num_pending);
-		if (ctdb_db) {
-			CTDB_INCREMENT_DB_STAT(ctdb_db, locks.num_pending);
-		}
+	lock_ctx->child = -1;
 
-		/* Start the timer when we activate the context */
-		lock_ctx->start_time = timeval_current();
+	DLIST_ADD_END(ctdb->lock_pending, lock_ctx, NULL);
+	ctdb->lock_num_pending++;
+	CTDB_INCREMENT_STAT(ctdb, locks.num_pending);
+	if (ctdb_db) {
+		CTDB_INCREMENT_DB_STAT(ctdb_db, locks.num_pending);
 	}
 
+	/* Start the timer when we activate the context */
+	lock_ctx->start_time = timeval_current();
+
 	if ((request = talloc_zero(lock_ctx, struct lock_request)) == NULL) {
 		talloc_free(lock_ctx);
 		return NULL;