Browse Source

tdb2: don't cancel transaction when tdb_transaction_prepare_commit fails

And don't double-log.  Both of these cause problems if we want to do
tdb_transaction_prepare_commit non-blocking (and have it fail so we can
try again).
Rusty Russell 15 years ago
parent
commit
0693529224
3 changed files with 7 additions and 31 deletions
  1. 1 10
      ccan/tdb2/lock.c
  2. 3 0
      ccan/tdb2/tdb2.h
  3. 3 21
      ccan/tdb2/transaction.c

+ 1 - 10
ccan/tdb2/lock.c

@@ -555,23 +555,14 @@ again:
 	/* Lock hashes, gradually. */
 	ecode = tdb_lock_gradual(tdb, ltype, flags, TDB_HASH_LOCK_START,
 				 TDB_HASH_LOCK_RANGE);
-	if (ecode != TDB_SUCCESS) {
-		if (!(flags & TDB_LOCK_PROBE)) {
-			tdb_logerr(tdb, ecode, TDB_LOG_ERROR,
-				   "tdb_allrecord_lock hashes failed");
-		}
+	if (ecode != TDB_SUCCESS)
 		return ecode;
-	}
 
 	/* Lock free tables: there to end of file. */
 	ecode = tdb_brlock(tdb, ltype,
 			   TDB_HASH_LOCK_START + TDB_HASH_LOCK_RANGE,
 			   0, flags);
 	if (ecode != TDB_SUCCESS) {
-		if (!(flags & TDB_LOCK_PROBE)) {
-			tdb_logerr(tdb, ecode, TDB_LOG_ERROR,
-				 "tdb_allrecord_lock freetables failed");
-		}
 		tdb_brunlock(tdb, ltype, TDB_HASH_LOCK_START,
 			     TDB_HASH_LOCK_RANGE);
 		return ecode;

+ 3 - 0
ccan/tdb2/tdb2.h

@@ -288,6 +288,9 @@ enum TDB_ERROR tdb_transaction_commit(struct tdb_context *tdb);
  * tdb_transaction_commit): if this succeeds then a transaction will only
  * fail if the write() or fsync() calls fail.
  *
+ * If this fails you must still call tdb_transaction_cancel() to cancel
+ * the transaction.
+ *
  * See Also:
  *	tdb_transaction_commit()
  */

+ 3 - 21
ccan/tdb2/transaction.c

@@ -921,10 +921,6 @@ static enum TDB_ERROR _tdb_transaction_prepare_commit(struct tdb_context *tdb)
 	/* upgrade the main transaction lock region to a write lock */
 	ecode = tdb_allrecord_upgrade(tdb);
 	if (ecode != TDB_SUCCESS) {
-		tdb_logerr(tdb, ecode, TDB_LOG_ERROR,
-			 "tdb_transaction_prepare_commit:"
-			 " failed to upgrade hash locks");
-		_tdb_transaction_cancel(tdb);
 		return ecode;
 	}
 
@@ -932,10 +928,6 @@ static enum TDB_ERROR _tdb_transaction_prepare_commit(struct tdb_context *tdb)
 	   during the commit */
 	ecode = tdb_lock_open(tdb, TDB_LOCK_WAIT|TDB_LOCK_NOCHECK);
 	if (ecode != TDB_SUCCESS) {
-		tdb_logerr(tdb, ecode, TDB_LOG_ERROR,
-			   "tdb_transaction_prepare_commit:"
-			   " failed to get open lock");
-		_tdb_transaction_cancel(tdb);
 		return ecode;
 	}
 
@@ -946,10 +938,6 @@ static enum TDB_ERROR _tdb_transaction_prepare_commit(struct tdb_context *tdb)
 						   &tdb->transaction
 						   ->magic_offset);
 		if (ecode != TDB_SUCCESS) {
-			tdb_logerr(tdb, ecode, TDB_LOG_ERROR,
-				 "tdb_transaction_prepare_commit:"
-				 " failed to setup recovery data");
-			_tdb_transaction_cancel(tdb);
 			return ecode;
 		}
 	}
@@ -965,10 +953,6 @@ static enum TDB_ERROR _tdb_transaction_prepare_commit(struct tdb_context *tdb)
 		tdb->file->map_size = tdb->transaction->old_map_size;
 		ecode = methods->expand_file(tdb, add);
 		if (ecode != TDB_SUCCESS) {
-			tdb_logerr(tdb, ecode, TDB_LOG_ERROR,
-				 "tdb_transaction_prepare_commit:"
-				 " expansion failed");
-			_tdb_transaction_cancel(tdb);
 			return ecode;
 		}
 	}
@@ -1016,8 +1000,10 @@ enum TDB_ERROR tdb_transaction_commit(struct tdb_context *tdb)
 
 	if (!tdb->transaction->prepared) {
 		ecode = _tdb_transaction_prepare_commit(tdb);
-		if (ecode != TDB_SUCCESS)
+		if (ecode != TDB_SUCCESS) {
+			_tdb_transaction_cancel(tdb);
 			return tdb->last_error = ecode;
+		}
 	}
 
 	methods = tdb->transaction->io_methods;
@@ -1040,10 +1026,6 @@ enum TDB_ERROR tdb_transaction_commit(struct tdb_context *tdb)
 		ecode = methods->twrite(tdb, offset,
 					tdb->transaction->blocks[i], length);
 		if (ecode != TDB_SUCCESS) {
-			tdb_logerr(tdb, ecode, TDB_LOG_ERROR,
-				   "tdb_transaction_commit:"
-				   " write failed during commit");
-
 			/* we've overwritten part of the data and
 			   possibly expanded the file, so we need to
 			   run the crash recovery code */