[WIP][PATCH] Remove nested transactions by default

Andrew Bartlett abartlet at samba.org
Wed Jun 28 06:23:22 UTC 2017


I had left this aside, but being on the home stretch for the locking
patches I wanted to raise the issue of the LDB transaction semantics.

ldb's current transaction semantics are (frankly) a little odd.

If we have a nested transaction, eg:
- start transaction
  - start transaction
  - operation
  - cancel transaction 
- end transaction

Then the cancel transaction is just ignored!

Internally there are also many layers of reference counting that while
currently correct feel ripe for challenges in the future. 

This patch set removes the ability to nest transactions by default,
using it only for a special case in the join. 

I don't want to derail the locking patches, but I also thought 'if we
are going to break the behaviours, why not get rid of this'. 

Thoughts welcome.  I suspect we should just leave this for after 4.7,
but it is presented for comment. 

Andrew Bartlett
-- 
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba
-------------- next part --------------
From 88a3f4eab5a507b24dafbdba648605bd608d9e9d Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Thu, 27 Apr 2017 08:18:38 +1200
Subject: [PATCH 01/25] ldb_tdb: Use TDB_DISALLOW_NESTING

The top level LDB code and the ldb_tdb code already keeps a reference
count of so-called nested transactions so do not allow tdb to do also.

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 lib/ldb/ldb_tdb/ldb_tdb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/ldb/ldb_tdb/ldb_tdb.c b/lib/ldb/ldb_tdb/ldb_tdb.c
index 2ac1967..1176dd5 100644
--- a/lib/ldb/ldb_tdb/ldb_tdb.c
+++ b/lib/ldb/ldb_tdb/ldb_tdb.c
@@ -1612,7 +1612,7 @@ static int ltdb_connect(struct ldb_context *ldb, const char *url,
 		path = url;
 	}
 
-	tdb_flags = TDB_DEFAULT | TDB_SEQNUM;
+	tdb_flags = TDB_DEFAULT | TDB_SEQNUM | TDB_DISALLOW_NESTING;
 
 	/* check for the 'nosync' option */
 	if (flags & LDB_FLG_NOSYNC) {
-- 
2.9.4


From ddcdbccde661de9ef77ef45f554c3e00dc18f97a Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Thu, 27 Apr 2017 08:34:56 +1200
Subject: [PATCH 02/25] tdb: Add new function tdb_transaction_active()

This will allow callers to avoid their own reference counting of transactions.

Additionally, this will always line up with the acutal transaction state, even
in the error cases where tdb can cancel the transaction

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 lib/tdb/common/transaction.c |  8 ++++++++
 lib/tdb/include/tdb.h        | 18 ++++++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/lib/tdb/common/transaction.c b/lib/tdb/common/transaction.c
index 420e754..9b975ea 100644
--- a/lib/tdb/common/transaction.c
+++ b/lib/tdb/common/transaction.c
@@ -412,6 +412,14 @@ static const struct tdb_methods transaction_methods = {
 	transaction_expand_file,
 };
 
+/*
+ * Is a transaction currently active on this context?
+ *
+ */
+_PUBLIC_ bool tdb_transaction_active(struct tdb_context *tdb)
+{
+	return (tdb->transaction != NULL);
+}
 
 /*
   start a tdb transaction. No token is returned, as only a single
diff --git a/lib/tdb/include/tdb.h b/lib/tdb/include/tdb.h
index e86d267..906ddd9 100644
--- a/lib/tdb/include/tdb.h
+++ b/lib/tdb/include/tdb.h
@@ -651,6 +651,24 @@ tdb_log_func tdb_log_fn(struct tdb_context *tdb);
 void *tdb_get_logging_private(struct tdb_context *tdb);
 
 /**
+ * @brief Is a transaction active?
+ *
+ * It is helpful for the application to know if a transaction is
+ * active, rather than needing to maintain an application-level reference
+ * count.
+ *
+ * @param[in]  tdb      The database to start the transaction.
+ *
+ * @return              true if there is a transaction active, false otherwise
+ *
+ * @see tdb_transaction_start()
+ * @see tdb_transaction_prepare_commit()
+ * @see tdb_transaction_commit()
+ * @see tdb_transaction_cancel()
+ */
+bool tdb_transaction_active(struct tdb_context *tdb);
+	
+/**
  * @brief Start a transaction.
  *
  * All operations after the transaction start can either be committed with
-- 
2.9.4


From fd0bd28c6ad8074f7696256f1840ce290f19e92f Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Thu, 27 Apr 2017 08:49:42 +1200
Subject: [PATCH 03/25] ldb_tdb: use tdb_transaction_active() to determine if
 we are in a transaction

This avoids using the ldb_tdb level reference count

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 lib/ldb/ldb_tdb/ldb_search.c | 2 +-
 lib/ldb/ldb_tdb/ldb_tdb.c    | 7 ++++---
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/lib/ldb/ldb_tdb/ldb_search.c b/lib/ldb/ldb_tdb/ldb_search.c
index 53355e0..d33fe40 100644
--- a/lib/ldb/ldb_tdb/ldb_search.c
+++ b/lib/ldb/ldb_tdb/ldb_search.c
@@ -489,7 +489,7 @@ static int ltdb_search_full(struct ltdb_context *ctx)
 	int ret;
 
 	ctx->error = LDB_SUCCESS;
-	if (ltdb->in_transaction != 0) {
+	if (tdb_transaction_active(ltdb->tdb)) {
 		ret = tdb_traverse(ltdb->tdb, search_func, ctx);
 	} else {
 		ret = tdb_traverse_read(ltdb->tdb, search_func, ctx);
diff --git a/lib/ldb/ldb_tdb/ldb_tdb.c b/lib/ldb/ldb_tdb/ldb_tdb.c
index 1176dd5..c1156ca 100644
--- a/lib/ldb/ldb_tdb/ldb_tdb.c
+++ b/lib/ldb/ldb_tdb/ldb_tdb.c
@@ -100,7 +100,7 @@ int ltdb_lock_read(struct ldb_module *module)
 	struct ltdb_private *ltdb = talloc_get_type(data, struct ltdb_private);
 	int ret = 0;
 
-	if (ltdb->in_transaction == 0 &&
+	if (tdb_transaction_active(ltdb->tdb) == false &&
 	    ltdb->read_lock_count == 0) {
 		ret = tdb_lockall_read(ltdb->tdb);
 	}
@@ -117,7 +117,8 @@ int ltdb_unlock_read(struct ldb_module *module)
 {
 	void *data = ldb_module_get_private(module);
 	struct ltdb_private *ltdb = talloc_get_type(data, struct ltdb_private);
-	if (ltdb->in_transaction == 0 && ltdb->read_lock_count == 1) {
+	if (tdb_transaction_active(ltdb->tdb) == false
+	    && ltdb->read_lock_count == 1) {
 		tdb_unlockall_read(ltdb->tdb);
 		ltdb->read_lock_count--;
 		return 0;
@@ -223,7 +224,7 @@ static int ltdb_modified(struct ldb_module *module, struct ldb_dn *dn)
 
 	/* only allow modifies inside a transaction, otherwise the
 	 * ldb is unsafe */
-	if (ltdb->in_transaction == 0) {
+	if (tdb_transaction_active(ltdb->tdb) == false) {
 		ldb_set_errstring(ldb_module_get_ctx(module), "ltdb modify without transaction");
 		return LDB_ERR_OPERATIONS_ERROR;
 	}
-- 
2.9.4


From e3f9e2151cc7d8d382b64d763350b64d7b02a903 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Thu, 27 Apr 2017 08:51:08 +1200
Subject: [PATCH 04/25] tdb: Improve documentation for tdb_transaction_start()

It now references the TDB_ALLOW_NESTING and TDB_DISALLOW_NESTING flags

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 lib/tdb/include/tdb.h | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/lib/tdb/include/tdb.h b/lib/tdb/include/tdb.h
index 906ddd9..89bc583 100644
--- a/lib/tdb/include/tdb.h
+++ b/lib/tdb/include/tdb.h
@@ -674,10 +674,13 @@ bool tdb_transaction_active(struct tdb_context *tdb);
  * All operations after the transaction start can either be committed with
  * tdb_transaction_commit() or cancelled with tdb_transaction_cancel().
  *
- * If you call tdb_transaction_start() again on the same tdb context while a
- * transaction is in progress, then the same transaction buffer is re-used. The
- * number of tdb_transaction_{commit,cancel} operations must match the number
- * of successful tdb_transaction_start() calls.
+ * If (the default) TDB_ALLOW_NESTING was specified or
+ * TDB_DISALLOW_NESTING was not specified as a flag via tdb_open() or
+ * tdb_open_ex(), you call tdb_transaction_start() again on the same
+ * tdb context while a transaction is in progress, then the same
+ * transaction buffer is re-used. The number of
+ * tdb_transaction_{commit,cancel} operations must match the number of
+ * successful tdb_transaction_start() calls.
  *
  * Note that transactions are by default disk synchronous, and use a recover
  * area in the database to automatically recover the database on the next open
-- 
2.9.4


From 728ba368cc220d4cf00836f98d25d72ee04a5c82 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Sun, 30 Apr 2017 08:32:30 +1000
Subject: [PATCH 05/25] tdb: version 1.3.14

* allow tdb_traverse_read before tdb_transaction[_prepare]_commit()
* add tdb_transaction_active()

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 lib/tdb/ABI/tdb-1.3.14.sigs | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/tdb/ABI/tdb-1.3.14.sigs b/lib/tdb/ABI/tdb-1.3.14.sigs
index 48f4278..61ce5e6 100644
--- a/lib/tdb/ABI/tdb-1.3.14.sigs
+++ b/lib/tdb/ABI/tdb-1.3.14.sigs
@@ -54,6 +54,7 @@ tdb_setalarm_sigptr: void (struct tdb_context *, volatile sig_atomic_t *)
 tdb_store: int (struct tdb_context *, TDB_DATA, TDB_DATA, int)
 tdb_storev: int (struct tdb_context *, TDB_DATA, const TDB_DATA *, int, int)
 tdb_summary: char *(struct tdb_context *)
+tdb_transaction_active: bool (struct tdb_context *)
 tdb_transaction_cancel: int (struct tdb_context *)
 tdb_transaction_commit: int (struct tdb_context *)
 tdb_transaction_prepare_commit: int (struct tdb_context *)
-- 
2.9.4


From be013240a89adf7237ceb85e2d06ec0806bc4d0a Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Sun, 30 Apr 2017 09:06:29 +1000
Subject: [PATCH 06/25] pyldb: Add ldb.FLG_DONT_CREATE_DB constant to ldb
 python bindings

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 lib/ldb/pyldb.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/ldb/pyldb.c b/lib/ldb/pyldb.c
index b65e255..cb821b9 100644
--- a/lib/ldb/pyldb.c
+++ b/lib/ldb/pyldb.c
@@ -4201,6 +4201,7 @@ static PyObject* module_init(void)
 	ADD_LDB_INT(FLG_NOSYNC);
 	ADD_LDB_INT(FLG_RECONNECT);
 	ADD_LDB_INT(FLG_NOMMAP);
+	ADD_LDB_INT(FLG_DONT_CREATE_DB);
 
 	/* Historical misspelling */
 	PyModule_AddIntConstant(m, "ERR_ALIAS_DEREFERINCING_PROBLEM", LDB_ERR_ALIAS_DEREFERENCING_PROBLEM);
-- 
2.9.4


From 8a839ace09b2c31d155aa843fb0f8b46f48e2462 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Mon, 1 May 2017 15:28:22 +1000
Subject: [PATCH 07/25] ldb: Only call start_transaction() in
 ldb_autotransaction_request() if needed

This avoids needing the LDB_FLG_FAKE_TXN_NESTING flag to be introduced
that permits emulating nested transactions

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 lib/ldb/common/ldb.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/lib/ldb/common/ldb.c b/lib/ldb/common/ldb.c
index a4d9977..74cc5b1 100644
--- a/lib/ldb/common/ldb.c
+++ b/lib/ldb/common/ldb.c
@@ -566,10 +566,14 @@ static int ldb_autotransaction_request(struct ldb_context *ldb,
 				       struct ldb_request *req)
 {
 	int ret;
-
-	ret = ldb_transaction_start(ldb);
-	if (ret != LDB_SUCCESS) {
-		return ret;
+	bool started_transaction = false;
+	
+	if (ldb->transaction_active == 0) {
+		ret = ldb_transaction_start(ldb);
+		if (ret != LDB_SUCCESS) {
+			return ret;
+		}
+		started_transaction = true;
 	}
 
 	ret = ldb_request(ldb, req);
@@ -577,11 +581,13 @@ static int ldb_autotransaction_request(struct ldb_context *ldb,
 		ret = ldb_wait(req->handle, LDB_WAIT_ALL);
 	}
 
-	if (ret == LDB_SUCCESS) {
-		return ldb_transaction_commit(ldb);
+	if (started_transaction) {
+		if (ret == LDB_SUCCESS) {
+			return ldb_transaction_commit(ldb);
+		}
+		ldb_transaction_cancel(ldb);
 	}
-	ldb_transaction_cancel(ldb);
-
+	
 	return ret;
 }
 
-- 
2.9.4


From 54ec23ef9d3096ebf6014ce43e4cb85021112349 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Mon, 1 May 2017 15:29:32 +1000
Subject: [PATCH 08/25] pyldb: Add ldb.FLG_ENABLE_TRACING constant to ldb
 python bindings

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 lib/ldb/pyldb.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/ldb/pyldb.c b/lib/ldb/pyldb.c
index cb821b9..72b6c3b 100644
--- a/lib/ldb/pyldb.c
+++ b/lib/ldb/pyldb.c
@@ -4201,6 +4201,7 @@ static PyObject* module_init(void)
 	ADD_LDB_INT(FLG_NOSYNC);
 	ADD_LDB_INT(FLG_RECONNECT);
 	ADD_LDB_INT(FLG_NOMMAP);
+	ADD_LDB_INT(FLG_ENABLE_TRACING);
 	ADD_LDB_INT(FLG_DONT_CREATE_DB);
 
 	/* Historical misspelling */
-- 
2.9.4


From d8ced9b7a8999be2e3088800eb5cc44191e507d6 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Mon, 1 May 2017 17:55:38 +1000
Subject: [PATCH 09/25] ldb: make ldb_autotransaction_request non-static

This allows us to do a similar job to dsdb_autotransaction_request but without
nested transactions

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 lib/ldb/common/ldb.c      |   8 ++--
 lib/ldb/include/ldb.h     |   5 +++
 lib/ldb/ldb_tdb/ldb_tdb.c |  17 +++++++-
 lib/ldb/pyldb.c           | 105 ++++++++++++++++++++++++++++------------------
 4 files changed, 90 insertions(+), 45 deletions(-)

diff --git a/lib/ldb/common/ldb.c b/lib/ldb/common/ldb.c
index 74cc5b1..18e7efe 100644
--- a/lib/ldb/common/ldb.c
+++ b/lib/ldb/common/ldb.c
@@ -561,9 +561,11 @@ int ldb_transaction_cancel_noerr(struct ldb_context *ldb)
 }
 
 
-/* autostarts a transaction if none active */
-static int ldb_autotransaction_request(struct ldb_context *ldb,
-				       struct ldb_request *req)
+/*
+ * like ldb_request, but autostarts a transaction if none active
+ */
+int ldb_autotransaction_request(struct ldb_context *ldb,
+				struct ldb_request *req)
 {
 	int ret;
 	bool started_transaction = false;
diff --git a/lib/ldb/include/ldb.h b/lib/ldb/include/ldb.h
index 14cec0e..15d5354 100644
--- a/lib/ldb/include/ldb.h
+++ b/lib/ldb/include/ldb.h
@@ -1030,6 +1030,11 @@ struct ldb_request {
 };
 
 int ldb_request(struct ldb_context *ldb, struct ldb_request *request);
+/*
+ * like ldb_request, but autostarts a transaction if none active
+ */
+int ldb_autotransaction_request(struct ldb_context *ldb,
+				struct ldb_request *req);
 int ldb_request_done(struct ldb_request *req, int status);
 bool ldb_request_is_done(struct ldb_request *req);
 
diff --git a/lib/ldb/ldb_tdb/ldb_tdb.c b/lib/ldb/ldb_tdb/ldb_tdb.c
index c1156ca..65c48b2 100644
--- a/lib/ldb/ldb_tdb/ldb_tdb.c
+++ b/lib/ldb/ldb_tdb/ldb_tdb.c
@@ -85,6 +85,8 @@ int ltdb_err_map(enum TDB_ERROR tdb_code)
 		return LDB_ERR_NO_SUCH_OBJECT;
 	case TDB_ERR_RDONLY:
 		return LDB_ERR_INSUFFICIENT_ACCESS_RIGHTS;
+	case TDB_ERR_NESTING:
+		return LDB_ERR_BUSY;
 	default:
 		break;
 	}
@@ -1145,7 +1147,20 @@ static int ltdb_start_trans(struct ldb_module *module)
 	struct ltdb_private *ltdb = talloc_get_type(data, struct ltdb_private);
 
 	if (tdb_transaction_start(ltdb->tdb) != 0) {
-		return ltdb_err_map(tdb_error(ltdb->tdb));
+		int tdb_ret = tdb_error(ltdb->tdb);
+		int ret = ltdb_err_map(tdb_ret);
+		if (tdb_ret == TDB_ERR_NESTING) {
+			ldb_asprintf_errstring(ldb_module_get_ctx(module),
+					       "Failure starting tdb transaction: "
+					       "Nested transactions not supported by TDB");
+		} else {
+			ldb_asprintf_errstring(ldb_module_get_ctx(module),
+					       "Failure during tdb_transaction_start(): "
+					       "%s -> %s",
+					       tdb_errorstr(ltdb->tdb),
+					       ldb_strerror(ret));
+		}
+		return ret;
 	}
 
 	ltdb->in_transaction++;
diff --git a/lib/ldb/pyldb.c b/lib/ldb/pyldb.c
index 72b6c3b..8a0e5c2 100644
--- a/lib/ldb/pyldb.c
+++ b/lib/ldb/pyldb.c
@@ -1196,6 +1196,7 @@ static PyObject *py_ldb_modify(PyLdbObject *self, PyObject *args, PyObject *kwar
 	TALLOC_CTX *mem_ctx;
 	bool validate=true;
 	const char * const kwnames[] = { "message", "controls", "validate", NULL };
+	bool started_transaction = false;
 
 	if (!PyArg_ParseTupleAndKeywords(args, kwargs, "O|Ob",
 					 discard_const_p(char *, kwnames),
@@ -1248,22 +1249,27 @@ static PyObject *py_ldb_modify(PyLdbObject *self, PyObject *args, PyObject *kwar
 	/* do request and autostart a transaction */
 	/* Then let's LDB handle the message error in case of pb as they are meaningful */
 
-	ret = ldb_transaction_start(ldb_ctx);
-	if (ret != LDB_SUCCESS) {
-		talloc_free(mem_ctx);
-		PyErr_SetLdbError(PyExc_LdbError, ret, ldb_ctx);
-		return NULL;
+	if (ldb_ctx->transaction_active == 0) {
+		ret = ldb_transaction_start(ldb_ctx);
+		if (ret != LDB_SUCCESS) {
+			talloc_free(mem_ctx);
+			PyErr_SetLdbError(PyExc_LdbError, ret, ldb_ctx);
+			return NULL;
+		}
+		started_transaction = true;
 	}
-
+	
 	ret = ldb_request(ldb_ctx, req);
 	if (ret == LDB_SUCCESS) {
 		ret = ldb_wait(req->handle, LDB_WAIT_ALL);
 	}
 
-	if (ret == LDB_SUCCESS) {
-		ret = ldb_transaction_commit(ldb_ctx);
-	} else {
-		ldb_transaction_cancel(ldb_ctx);
+	if (started_transaction) {
+		if (ret == LDB_SUCCESS) {
+			ret = ldb_transaction_commit(ldb_ctx);
+		} else {
+			ldb_transaction_cancel(ldb_ctx);
+		}
 	}
 
 	talloc_free(mem_ctx);
@@ -1345,6 +1351,7 @@ static PyObject *py_ldb_add(PyLdbObject *self, PyObject *args, PyObject *kwargs)
 	TALLOC_CTX *mem_ctx;
 	struct ldb_control **parsed_controls;
 	const char * const kwnames[] = { "message", "controls", NULL };
+	bool started_transaction = false;
 
 	if (!PyArg_ParseTupleAndKeywords(args, kwargs, "O|O",
 					 discard_const_p(char *, kwnames),
@@ -1403,24 +1410,28 @@ static PyObject *py_ldb_add(PyLdbObject *self, PyObject *args, PyObject *kwargs)
         /* do request and autostart a transaction */
 	/* Then let's LDB handle the message error in case of pb as they are meaningful */
 
-	ret = ldb_transaction_start(ldb_ctx);
-	if (ret != LDB_SUCCESS) {
-		talloc_free(mem_ctx);
-		PyErr_SetLdbError(PyExc_LdbError, ret, ldb_ctx);
-		return NULL;
+	if (ldb_ctx->transaction_active == 0) {
+		ret = ldb_transaction_start(ldb_ctx);
+		if (ret != LDB_SUCCESS) {
+			talloc_free(mem_ctx);
+			PyErr_SetLdbError(PyExc_LdbError, ret, ldb_ctx);
+			return NULL;
+		}
+		started_transaction = true;
 	}
-
+	
 	ret = ldb_request(ldb_ctx, req);
 	if (ret == LDB_SUCCESS) {
 		ret = ldb_wait(req->handle, LDB_WAIT_ALL);
 	}
 
-	if (ret == LDB_SUCCESS) {
-		ret = ldb_transaction_commit(ldb_ctx);
-	} else {
-		ldb_transaction_cancel(ldb_ctx);
+	if (started_transaction) {
+		if (ret == LDB_SUCCESS) {
+			ret = ldb_transaction_commit(ldb_ctx);
+		} else {
+			ldb_transaction_cancel(ldb_ctx);
+		}
 	}
-
 	talloc_free(mem_ctx);
 	PyErr_LDB_ERROR_IS_ERR_RAISE(PyExc_LdbError, ret, ldb_ctx);
 
@@ -1438,6 +1449,7 @@ static PyObject *py_ldb_delete(PyLdbObject *self, PyObject *args, PyObject *kwar
 	TALLOC_CTX *mem_ctx;
 	struct ldb_control **parsed_controls;
 	const char * const kwnames[] = { "dn", "controls", NULL };
+	bool started_transaction = false;
 
 	if (!PyArg_ParseTupleAndKeywords(args, kwargs, "O|O",
 					 discard_const_p(char *, kwnames),
@@ -1479,24 +1491,29 @@ static PyObject *py_ldb_delete(PyLdbObject *self, PyObject *args, PyObject *kwar
 	/* do request and autostart a transaction */
 	/* Then let's LDB handle the message error in case of pb as they are meaningful */
 
-	ret = ldb_transaction_start(ldb_ctx);
-	if (ret != LDB_SUCCESS) {
-		talloc_free(mem_ctx);
-		PyErr_SetLdbError(PyExc_LdbError, ret, ldb_ctx);
-		return NULL;
+	if (ldb_ctx->transaction_active == 0) {
+		ret = ldb_transaction_start(ldb_ctx);
+		if (ret != LDB_SUCCESS) {
+			talloc_free(mem_ctx);
+			PyErr_SetLdbError(PyExc_LdbError, ret, ldb_ctx);
+			return NULL;
+		}
+		started_transaction = true;
 	}
-
+	
 	ret = ldb_request(ldb_ctx, req);
 	if (ret == LDB_SUCCESS) {
 		ret = ldb_wait(req->handle, LDB_WAIT_ALL);
 	}
 
-	if (ret == LDB_SUCCESS) {
-		ret = ldb_transaction_commit(ldb_ctx);
-	} else {
-		ldb_transaction_cancel(ldb_ctx);
+	if (started_transaction) {
+		if (ret == LDB_SUCCESS) {
+			ret = ldb_transaction_commit(ldb_ctx);
+		} else {
+			ldb_transaction_cancel(ldb_ctx);
+		}
 	}
-
+	
 	talloc_free(mem_ctx);
 	PyErr_LDB_ERROR_IS_ERR_RAISE(PyExc_LdbError, ret, ldb_ctx);
 
@@ -1514,6 +1531,7 @@ static PyObject *py_ldb_rename(PyLdbObject *self, PyObject *args, PyObject *kwar
 	struct ldb_context *ldb_ctx;
 	struct ldb_request *req;
 	const char * const kwnames[] = { "dn1", "dn2", "controls", NULL };
+	bool started_transaction = false;
 
 	ldb_ctx = pyldb_Ldb_AsLdbContext(self);
 
@@ -1563,11 +1581,14 @@ static PyObject *py_ldb_rename(PyLdbObject *self, PyObject *args, PyObject *kwar
 	/* do request and autostart a transaction */
 	/* Then let's LDB handle the message error in case of pb as they are meaningful */
 
-	ret = ldb_transaction_start(ldb_ctx);
-	if (ret != LDB_SUCCESS) {
-		talloc_free(mem_ctx);
-		PyErr_SetLdbError(PyExc_LdbError, ret, ldb_ctx);
-		return NULL;
+	if (ldb_ctx->transaction_active == 0) {
+		ret = ldb_transaction_start(ldb_ctx);
+		if (ret != LDB_SUCCESS) {
+			talloc_free(mem_ctx);
+			PyErr_SetLdbError(PyExc_LdbError, ret, ldb_ctx);
+			return NULL;
+		}
+		started_transaction = true;
 	}
 
 	ret = ldb_request(ldb_ctx, req);
@@ -1575,10 +1596,12 @@ static PyObject *py_ldb_rename(PyLdbObject *self, PyObject *args, PyObject *kwar
 		ret = ldb_wait(req->handle, LDB_WAIT_ALL);
 	}
 
-	if (ret == LDB_SUCCESS) {
-		ret = ldb_transaction_commit(ldb_ctx);
-	} else {
-		ldb_transaction_cancel(ldb_ctx);
+	if (started_transaction) {
+		if (ret == LDB_SUCCESS) {
+			ret = ldb_transaction_commit(ldb_ctx);
+		} else {
+			ldb_transaction_cancel(ldb_ctx);
+		}
 	}
 
 	talloc_free(mem_ctx);
-- 
2.9.4


From dfd7cf9dab6739844d43fa53d7a9963f32a5357f Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Mon, 1 May 2017 17:57:46 +1000
Subject: [PATCH 10/25] dsdb: Use ldb_autotransaction_request rather than
 dsdb_autotransaction_request

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 source4/auth/sam.c                  |  4 ++--
 source4/dsdb/common/util.c          | 34 ++++------------------------------
 source4/rpc_server/lsa/dcesrv_lsa.c |  2 +-
 3 files changed, 7 insertions(+), 33 deletions(-)

diff --git a/source4/auth/sam.c b/source4/auth/sam.c
index 49e34ba..aa06bf2 100644
--- a/source4/auth/sam.c
+++ b/source4/auth/sam.c
@@ -728,7 +728,7 @@ NTSTATUS authsam_update_bad_pwd_count(struct ldb_context *sam_ctx,
 			goto done;
 		}
 
-		ret = dsdb_autotransaction_request(sam_ctx, req);
+		ret = ldb_autotransaction_request(sam_ctx, req);
 		talloc_free(req);
 	}
 
@@ -1027,7 +1027,7 @@ NTSTATUS authsam_logon_success_accounting(struct ldb_context *sam_ctx,
 			goto done;
 		}
 
-		ret = dsdb_autotransaction_request(sam_ctx, req);
+		ret = ldb_autotransaction_request(sam_ctx, req);
 		talloc_free(req);
 	}
 
diff --git a/source4/dsdb/common/util.c b/source4/dsdb/common/util.c
index 7a12c71..6b0f31b 100644
--- a/source4/dsdb/common/util.c
+++ b/source4/dsdb/common/util.c
@@ -1103,32 +1103,6 @@ int samdb_msg_set_uint(struct ldb_context *sam_ldb, TALLOC_CTX *mem_ctx,
 }
 
 /*
- * Handle ldb_request in transaction
- */
-int dsdb_autotransaction_request(struct ldb_context *sam_ldb,
-				 struct ldb_request *req)
-{
-	int ret;
-
-	ret = ldb_transaction_start(sam_ldb);
-	if (ret != LDB_SUCCESS) {
-		return ret;
-	}
-
-	ret = ldb_request(sam_ldb, req);
-	if (ret == LDB_SUCCESS) {
-		ret = ldb_wait(req->handle, LDB_WAIT_ALL);
-	}
-
-	if (ret == LDB_SUCCESS) {
-		return ldb_transaction_commit(sam_ldb);
-	}
-	ldb_transaction_cancel(sam_ldb);
-
-	return ret;
-}
-
-/*
   return a default security descriptor
 */
 struct security_descriptor *samdb_default_security_descriptor(TALLOC_CTX *mem_ctx)
@@ -2277,7 +2251,7 @@ static NTSTATUS samdb_set_password_internal(struct ldb_context *ldb, TALLOC_CTX
 		return NT_STATUS_NO_MEMORY;
 	}
 
-	ret = dsdb_autotransaction_request(ldb, req);
+	ret = ldb_autotransaction_request(ldb, req);
 
 	if (req->context != NULL) {
 		struct ldb_control *control = talloc_get_type_abort(req->context,
@@ -4396,7 +4370,7 @@ int dsdb_add(struct ldb_context *ldb, const struct ldb_message *message,
 		return ret;
 	}
 
-	ret = dsdb_autotransaction_request(ldb, req);
+	ret = ldb_autotransaction_request(ldb, req);
 
 	talloc_free(req);
 	return ret;
@@ -4426,7 +4400,7 @@ int dsdb_modify(struct ldb_context *ldb, const struct ldb_message *message,
 		return ret;
 	}
 
-	ret = dsdb_autotransaction_request(ldb, req);
+	ret = ldb_autotransaction_request(ldb, req);
 
 	talloc_free(req);
 	return ret;
@@ -4456,7 +4430,7 @@ int dsdb_delete(struct ldb_context *ldb, struct ldb_dn *dn,
 		return ret;
 	}
 
-	ret = dsdb_autotransaction_request(ldb, req);
+	ret = ldb_autotransaction_request(ldb, req);
 
 	talloc_free(req);
 	return ret;
diff --git a/source4/rpc_server/lsa/dcesrv_lsa.c b/source4/rpc_server/lsa/dcesrv_lsa.c
index 3cccd57..78c4595 100644
--- a/source4/rpc_server/lsa/dcesrv_lsa.c
+++ b/source4/rpc_server/lsa/dcesrv_lsa.c
@@ -1013,7 +1013,7 @@ static NTSTATUS add_trust_user(TALLOC_CTX *mem_ctx,
 		return NT_STATUS_NO_MEMORY;
 	}
 
-	ret = dsdb_autotransaction_request(sam_ldb, req);
+	ret = ldb_autotransaction_request(sam_ldb, req);
 	if (ret != LDB_SUCCESS) {
 		DEBUG(0,("Failed to create user record %s: %s\n",
 			 ldb_dn_get_linearized(msg->dn),
-- 
2.9.4


From 7ced4d2ac196a8cdca52aa39fdbb000c028bdb5f Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Mon, 1 May 2017 19:25:43 +1000
Subject: [PATCH 11/25] ldb: Use ldb_autotransaction_request() in ldbadd,
 ldbdel, ldbrename and ldbmodify

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 lib/ldb/tools/ldbutil.c | 34 +++-------------------------------
 1 file changed, 3 insertions(+), 31 deletions(-)

diff --git a/lib/ldb/tools/ldbutil.c b/lib/ldb/tools/ldbutil.c
index 9ff7fad..050d5c8 100644
--- a/lib/ldb/tools/ldbutil.c
+++ b/lib/ldb/tools/ldbutil.c
@@ -35,34 +35,6 @@
 #include "ldbutil.h"
 
 
-/* autostarts a transacion if none active */
-static int ldb_do_autotransaction(struct ldb_context *ldb,
-				       struct ldb_request *req)
-{
-	int ret;
-
-	ret = ldb_transaction_start(ldb);
-	if (ret != LDB_SUCCESS) {
-		return ret;
-	}
-
-	ret = ldb_request(ldb, req);
-	if (ret == LDB_SUCCESS) {
-		ret = ldb_wait(req->handle, LDB_WAIT_ALL);
-	}
-
-	if (ret == LDB_SUCCESS) {
-		return ldb_transaction_commit(ldb);
-	}
-	ldb_transaction_cancel(ldb);
-
-	if (ldb_errstring(ldb) == NULL) {
-		/* no error string was setup by the backend */
-		ldb_asprintf_errstring(ldb, "%s (%d)", ldb_strerror(ret), ret);
-	}
-
-	return ret;
-}
 /*
   Same as ldb_add but accept control
 */
@@ -88,7 +60,7 @@ int ldb_add_ctrl(struct ldb_context *ldb,
 	if (ret != LDB_SUCCESS) return ret;
 
 	/* do request and autostart a transaction */
-	ret = ldb_do_autotransaction(ldb, req);
+	ret = ldb_autotransaction_request(ldb, req);
 
 	talloc_free(req);
 	return ret;
@@ -113,7 +85,7 @@ int ldb_delete_ctrl(struct ldb_context *ldb, struct ldb_dn *dn,
 	if (ret != LDB_SUCCESS) return ret;
 
 	/* do request and autostart a transaction */
-	ret = ldb_do_autotransaction(ldb, req);
+	ret = ldb_autotransaction_request(ldb, req);
 
 	talloc_free(req);
 	return ret;
@@ -145,7 +117,7 @@ int ldb_modify_ctrl(struct ldb_context *ldb,
         if (ret != LDB_SUCCESS) return ret;
 
         /* do request and autostart a transaction */
-        ret = ldb_do_autotransaction(ldb, req);
+        ret = ldb_autotransaction_request(ldb, req);
 
         talloc_free(req);
         return ret;
-- 
2.9.4


From 0eb476bbdedca5b5559cfd4716d9fa28a02686cf Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Mon, 1 May 2017 19:26:32 +1000
Subject: [PATCH 12/25] samdb: Do not nest transactions in samdb.py

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 python/samba/samdb.py | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/python/samba/samdb.py b/python/samba/samdb.py
index 6fe680d..76187af 100644
--- a/python/samba/samdb.py
+++ b/python/samba/samdb.py
@@ -456,10 +456,11 @@ member: %s
             if ldbmessage2:
                 self.modify(ldbmessage2)
 
-            # Sets the password for it
+            # Sets the password for it, but skip the nested transaction
             if setpassword:
                 self.setpassword("(samAccountName=%s)" % ldb.binary_encode(username), password,
-                                 force_password_change_at_next_login_req)
+                                 force_password_change_at_next_login_req,
+                                 need_transaction=False)
         except:
             self.transaction_cancel()
             raise
@@ -489,7 +490,8 @@ member: %s
             self.transaction_commit()
 
     def setpassword(self, search_filter, password,
-            force_change_at_next_login=False, username=None):
+                    force_change_at_next_login=False, username=None,
+                    need_transaction=True):
         """Sets the password for a user
 
         :param search_filter: LDAP filter to find the user (eg
@@ -497,7 +499,8 @@ member: %s
         :param password: Password for the user
         :param force_change_at_next_login: Force password change
         """
-        self.transaction_start()
+        if need_transaction:
+            self.transaction_start()
         try:
             res = self.search(base=self.domain_dn(), scope=ldb.SCOPE_SUBTREE,
                               expression=search_filter, attrs=[])
@@ -523,10 +526,12 @@ unicodePwd:: %s
             #  modify the userAccountControl to remove the disabled bit
             self.enable_account(search_filter)
         except:
-            self.transaction_cancel()
+            if need_transaction:
+                self.transaction_cancel()
             raise
         else:
-            self.transaction_commit()
+            if need_transaction:
+                self.transaction_commit()
 
     def setexpiry(self, search_filter, expiry_seconds, no_expiry_req=False):
         """Sets the account expiry for a user
-- 
2.9.4


From 245a87b02d76ed2c50eb976fc6b132ca218a36de Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Mon, 1 May 2017 19:48:12 +1000
Subject: [PATCH 13/25] ldb: Remove nested transactions from ldbadd

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 lib/ldb/tools/ldbadd.c | 17 -----------------
 1 file changed, 17 deletions(-)

diff --git a/lib/ldb/tools/ldbadd.c b/lib/ldb/tools/ldbadd.c
index e6cea29..747c975 100644
--- a/lib/ldb/tools/ldbadd.c
+++ b/lib/ldb/tools/ldbadd.c
@@ -65,13 +65,6 @@ static int process_file(struct ldb_context *ldb, FILE *f, unsigned int *count)
 		return LDB_ERR_OPERATIONS_ERROR;
 	}
 
-	fun_ret = ldb_transaction_start(ldb);
-	if (fun_ret != LDB_SUCCESS) {
-		fprintf(stderr, "ERR: (%s) on transaction start\n",
-			ldb_errstring(ldb));
-		return fun_ret;
-	}
-
 	while ((ldif = ldb_ldif_read_file_state(ldb, &state))) {
 		if (ldif->changetype != LDB_CHANGETYPE_ADD &&
 		    ldif->changetype != LDB_CHANGETYPE_NONE) {
@@ -113,16 +106,6 @@ static int process_file(struct ldb_context *ldb, FILE *f, unsigned int *count)
 		fun_ret = LDB_ERR_OPERATIONS_ERROR;
 	}
 
-	if (fun_ret == LDB_SUCCESS) {
-		fun_ret = ldb_transaction_commit(ldb);
-		if (fun_ret != LDB_SUCCESS) {
-			fprintf(stderr, "ERR: (%s) on transaction commit\n",
-				ldb_errstring(ldb));
-		}
-	} else {
-		ldb_transaction_cancel(ldb);
-	}
-
 	return fun_ret;
 }
 
-- 
2.9.4


From 02dc02fb2d63653dc6a74d3a885d1ffc25847312 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Mon, 1 May 2017 20:20:35 +1000
Subject: [PATCH 14/25] ldb: Remove in_transaction from ldb_tdb

We no longer need to track this, the tdb_transaction_start() call
prevents us from seeing nested transactions.

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 lib/ldb/ldb_tdb/ldb_tdb.c | 14 ++++++--------
 lib/ldb/ldb_tdb/ldb_tdb.h |  2 --
 2 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/lib/ldb/ldb_tdb/ldb_tdb.c b/lib/ldb/ldb_tdb/ldb_tdb.c
index 65c48b2..75460a1 100644
--- a/lib/ldb/ldb_tdb/ldb_tdb.c
+++ b/lib/ldb/ldb_tdb/ldb_tdb.c
@@ -1163,8 +1163,6 @@ static int ltdb_start_trans(struct ldb_module *module)
 		return ret;
 	}
 
-	ltdb->in_transaction++;
-
 	ltdb_index_transaction_start(module);
 
 	return LDB_SUCCESS;
@@ -1176,20 +1174,22 @@ static int ltdb_prepare_commit(struct ldb_module *module)
 	void *data = ldb_module_get_private(module);
 	struct ltdb_private *ltdb = talloc_get_type(data, struct ltdb_private);
 
-	if (ltdb->in_transaction != 1) {
-		return LDB_SUCCESS;
+	if (tdb_transaction_active(ltdb->tdb) == false) {
+		ldb_set_errstring(ldb_module_get_ctx(module),
+				  "ltdb_prepare_commit() called "
+				  "without transaction active");
+		
+		return LDB_ERR_OPERATIONS_ERROR;
 	}
 
 	ret = ltdb_index_transaction_commit(module);
 	if (ret != LDB_SUCCESS) {
 		tdb_transaction_cancel(ltdb->tdb);
-		ltdb->in_transaction--;
 		return ret;
 	}
 
 	if (tdb_transaction_prepare_commit(ltdb->tdb) != 0) {
 		ret = ltdb_err_map(tdb_error(ltdb->tdb));
-		ltdb->in_transaction--;
 		ldb_asprintf_errstring(ldb_module_get_ctx(module),
 				       "Failure during tdb_transaction_prepare_commit(): %s -> %s",
 				       tdb_errorstr(ltdb->tdb),
@@ -1215,7 +1215,6 @@ static int ltdb_end_trans(struct ldb_module *module)
 		}
 	}
 
-	ltdb->in_transaction--;
 	ltdb->prepared_commit = false;
 
 	if (tdb_transaction_commit(ltdb->tdb) != 0) {
@@ -1235,7 +1234,6 @@ static int ltdb_del_trans(struct ldb_module *module)
 	void *data = ldb_module_get_private(module);
 	struct ltdb_private *ltdb = talloc_get_type(data, struct ltdb_private);
 
-	ltdb->in_transaction--;
 
 	if (ltdb_index_transaction_cancel(module) != 0) {
 		tdb_transaction_cancel(ltdb->tdb);
diff --git a/lib/ldb/ldb_tdb/ldb_tdb.h b/lib/ldb/ldb_tdb/ldb_tdb.h
index 26ae68e..41dc50e 100644
--- a/lib/ldb/ldb_tdb/ldb_tdb.h
+++ b/lib/ldb/ldb_tdb/ldb_tdb.h
@@ -22,8 +22,6 @@ struct ltdb_private {
 		bool attribute_indexes;
 	} *cache;
 
-	int in_transaction;
-
 	bool check_base;
 	bool disallow_dn_filter;
 	struct ltdb_idxptr *idxptr;
-- 
2.9.4


From c0c19dc875c51d82768202705425f9912f4f254c Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Wed, 3 May 2017 06:33:32 +0200
Subject: [PATCH 15/25] dsdb/repl: Remove nested transaction calls from
 dsdb_origin_objects_commit()

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 source4/dsdb/repl/replicated_objects.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/source4/dsdb/repl/replicated_objects.c b/source4/dsdb/repl/replicated_objects.c
index 637f7fa..98a16b0 100644
--- a/source4/dsdb/repl/replicated_objects.c
+++ b/source4/dsdb/repl/replicated_objects.c
@@ -1066,6 +1066,11 @@ static WERROR dsdb_origin_object_convert(struct ldb_context *ldb,
 	return WERR_OK;
 }
 
+/*
+ * The caller is required to ensuere there is a transaction in place
+ * for this operation
+ */
+
 WERROR dsdb_origin_objects_commit(struct ldb_context *ldb,
 				  TALLOC_CTX *mem_ctx,
 				  const struct drsuapi_DsReplicaObjectListItem *first_object,
@@ -1096,11 +1101,6 @@ WERROR dsdb_origin_objects_commit(struct ldb_context *ldb,
 		return WERR_OK;
 	}
 
-	ret = ldb_transaction_start(ldb);
-	if (ret != LDB_SUCCESS) {
-		return WERR_DS_INTERNAL_FAILURE;
-	}
-
 	objects	= talloc_array(mem_ctx, struct ldb_message *,
 			       num_objects);
 	if (objects == NULL) {
@@ -1210,11 +1210,6 @@ WERROR dsdb_origin_objects_commit(struct ldb_context *ldb,
 		}
 	}
 
-	ret = ldb_transaction_commit(ldb);
-	if (ret != LDB_SUCCESS) {
-		return WERR_DS_INTERNAL_FAILURE;
-	}
-
 	talloc_free(objects);
 
 	*_num = num_objects;
-- 
2.9.4


From 3936c49a98ade56a490d58154dc84208e257f67c Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Wed, 3 May 2017 07:50:28 +0200
Subject: [PATCH 16/25] samba-tool drs replicate: Remove nested transaction
 from --local

We do not need to take out an additonal transaction here, the C code
handles this for us.

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 python/samba/netcmd/drs.py | 2 --
 1 file changed, 2 deletions(-)

diff --git a/python/samba/netcmd/drs.py b/python/samba/netcmd/drs.py
index b9b876a..4b62898 100644
--- a/python/samba/netcmd/drs.py
+++ b/python/samba/netcmd/drs.py
@@ -273,7 +273,6 @@ def drs_local_replicate(self, SOURCE_DC, NC, full_sync=False, single_object=Fals
         exop = drsuapi.DRSUAPI_EXOP_REPL_OBJ
         full_sync = True
 
-    self.samdb.transaction_start()
     repl = drs_utils.drs_Replicate("ncacn_ip_tcp:%s[seal]" % self.server, self.lp,
                                    self.creds, self.local_samdb, dest_dsa_invocation_id)
 
@@ -287,7 +286,6 @@ def drs_local_replicate(self, SOURCE_DC, NC, full_sync=False, single_object=Fals
                                                   exop=exop)
     except Exception, e:
         raise CommandError("Error replicating DN %s" % NC, e)
-    self.samdb.transaction_commit()
 
     if full_sync:
         self.message("Full Replication of all %d objects and %d links from %s to %s was successful."
-- 
2.9.4


From e1dce03abcea19e0e23674aeaf08a695e2ccb003 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Wed, 3 May 2017 07:53:32 +0200
Subject: [PATCH 17/25] samba-tool rodc preload: Remove nested transaction

We do not need to take out an additonal transaction here, the C code handles this

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 python/samba/netcmd/rodc.py | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/python/samba/netcmd/rodc.py b/python/samba/netcmd/rodc.py
index e7fbcdc..d92fe2e 100644
--- a/python/samba/netcmd/rodc.py
+++ b/python/samba/netcmd/rodc.py
@@ -131,18 +131,15 @@ class cmd_rodc_preload(Command):
 
             self.outf.write("Replicating DN %s\n" % dn)
 
-            local_samdb.transaction_start()
             try:
                 repl.replicate(dn, source_dsa_invocation_id, destination_dsa_guid,
                                exop=drsuapi.DRSUAPI_EXOP_REPL_SECRET, rodc=True)
             except Exception, e:
-                local_samdb.transaction_cancel()
                 if not ignore_errors:
                     raise CommandError("Error replicating DN %s" % dn)
                 errors.append(ReplicationError("Error replicating DN %s" % dn))
                 continue
 
-            local_samdb.transaction_commit()
 
         if len(errors) > 0:
             print "\nPreload encountered problematic users:"
-- 
2.9.4


From 35dfa82146b911aa0d4f3f79603b707966a7dad6 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Sun, 30 Apr 2017 08:09:59 +1000
Subject: [PATCH 18/25] ldb: Add flag LDB_FLG_FAKE_TXN_NESTING

This will allow the fake nesting of transactions.  Instead it will be up to
the backend to nest if possible.  The caller will need to choose a
backend that matches the desired behaivour.

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 lib/ldb/include/ldb.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/lib/ldb/include/ldb.h b/lib/ldb/include/ldb.h
index 15d5354..5e446c6 100644
--- a/lib/ldb/include/ldb.h
+++ b/lib/ldb/include/ldb.h
@@ -280,6 +280,18 @@ struct ldb_utf8_fns {
 */
 #define LDB_FLG_DONT_CREATE_DB 64
 
+/**
+   Flags to tell LDB to fake nested transactions
+
+   With this flag ldb will emulate nested transactions via a reference
+   count.  ldb_tdb does not have real nested transactions, so with this
+   backend transaction nesting will otherwise fail.
+   
+   However the difference between real and emulated nested
+   transactions is visible to the application.
+*/
+#define LDB_FLG_FAKE_TXN_NESTING 128
+
 /*
    structures for ldb_parse_tree handling code
 */
-- 
2.9.4


From bc88667fdfbded22ff20d611513f10c86b180a84 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Mon, 1 May 2017 16:09:00 +1000
Subject: [PATCH 19/25] pyldb: Add ldb.FLG_DONT_FAKE_TXN_NESTING constant to
 ldb python bindings

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 lib/ldb/pyldb.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/ldb/pyldb.c b/lib/ldb/pyldb.c
index 8a0e5c2..4a2a80b 100644
--- a/lib/ldb/pyldb.c
+++ b/lib/ldb/pyldb.c
@@ -4226,6 +4226,7 @@ static PyObject* module_init(void)
 	ADD_LDB_INT(FLG_NOMMAP);
 	ADD_LDB_INT(FLG_ENABLE_TRACING);
 	ADD_LDB_INT(FLG_DONT_CREATE_DB);
+	ADD_LDB_INT(FLG_FAKE_TXN_NESTING);
 
 	/* Historical misspelling */
 	PyModule_AddIntConstant(m, "ERR_ALIAS_DEREFERINCING_PROBLEM", LDB_ERR_ALIAS_DEREFERENCING_PROBLEM);
-- 
2.9.4


From b28d2567a233607edb80107e241a7cb91c382bab Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Wed, 28 Jun 2017 17:32:46 +1200
Subject: [PATCH 20/25] provision: Pass down ldb flags into SamDB() and
 ldb.connect()

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 python/samba/provision/__init__.py | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/python/samba/provision/__init__.py b/python/samba/provision/__init__.py
index 2387931..8254c12 100644
--- a/python/samba/provision/__init__.py
+++ b/python/samba/provision/__init__.py
@@ -1182,7 +1182,8 @@ def create_default_gpo(sysvolpath, dnsdomain, policyguid, policyguid_dc):
 
 
 def setup_samdb(path, session_info, provision_backend, lp, names,
-        logger, fill, serverrole, schema, am_rodc=False):
+                logger, fill, serverrole, schema, am_rodc=False,
+                samdb_flags=0):
     """Setup a complete SAM Database.
 
     :note: This will wipe the main SAM database file!
@@ -1197,7 +1198,8 @@ def setup_samdb(path, session_info, provision_backend, lp, names,
     # quite yet
     samdb = SamDB(session_info=session_info, url=None, auto_connect=False,
                   credentials=provision_backend.credentials, lp=lp,
-                  global_schema=False, am_rodc=am_rodc)
+                  global_schema=False, am_rodc=am_rodc,
+                  flags=samdb_flags)
 
     logger.info("Pre-loading the Samba 4 and AD schema")
 
@@ -1211,7 +1213,7 @@ def setup_samdb(path, session_info, provision_backend, lp, names,
     # And now we can connect to the DB - the schema won't be loaded from the
     # DB
     try:
-        samdb.connect(path)
+        samdb.connect(path, flags=samdb_flags)
     except ldb.LdbError, (num, string_error):
         if (num == ldb.ERR_INSUFFICIENT_ACCESS_RIGHTS):
             raise ProvisioningError("Permission denied connecting to %s, are you running as root?" % path)
@@ -1942,7 +1944,8 @@ def provision(logger, session_info, smbconf=None,
         sitename=None, ol_mmr_urls=None, ol_olc=None, slapd_path=None,
         useeadb=False, am_rodc=False, lp=None, use_ntvfs=False,
         use_rfc2307=False, maxuid=None, maxgid=None, skip_sysvolacl=True,
-        ldap_backend_forced_uri=None, nosync=False, ldap_dryrun_mode=False, ldap_backend_extra_port=None):
+        ldap_backend_forced_uri=None, nosync=False, ldap_dryrun_mode=False, ldap_backend_extra_port=None,
+        samdb_flags=0):
     """Provision samba4
 
     :note: caution, this wipes all existing data!
@@ -2141,7 +2144,8 @@ def provision(logger, session_info, smbconf=None,
         samdb = setup_samdb(paths.samdb, session_info,
                             provision_backend, lp, names, logger=logger,
                             serverrole=serverrole,
-                            schema=schema, fill=samdb_fill, am_rodc=am_rodc)
+                            schema=schema, fill=samdb_fill, am_rodc=am_rodc,
+                            samdb_flags=samdb_flags)
 
         if serverrole == "active directory domain controller":
             if paths.netlogon is None:
-- 
2.9.4


From 9784df35394b126962da5ebd918c1d3a7ae874a1 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Wed, 28 Jun 2017 17:33:46 +1200
Subject: [PATCH 21/25] join.py: Allow fake transaction nesting for the join

We use this to have a transaction open over the full join, not just each chunk

This is needed to resolve all the links cross-partition in a single prepare_commit()

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 python/samba/join.py | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/python/samba/join.py b/python/samba/join.py
index fa87f0b..e3f46e7 100644
--- a/python/samba/join.py
+++ b/python/samba/join.py
@@ -837,7 +837,8 @@ class dc_join(object):
                 hostname=ctx.myname, domainsid=ctx.domsid,
                 machinepass=ctx.acct_pass, serverrole="active directory domain controller",
                 sitename=ctx.site, lp=ctx.lp, ntdsguid=ctx.ntds_guid,
-                use_ntvfs=ctx.use_ntvfs, dns_backend=ctx.dns_backend)
+                use_ntvfs=ctx.use_ntvfs, dns_backend=ctx.dns_backend,
+                samdb_flags=ldb.FLG_FAKE_TXN_NESTING)
         print "Provision OK for domain DN %s" % presult.domaindn
         ctx.local_samdb = presult.samdb
         ctx.lp          = presult.lp
@@ -856,7 +857,8 @@ class dc_join(object):
         ctx.samdb = SamDB(url=ctx.local_samdb.url,
                           session_info=system_session(),
                           lp=ctx.local_samdb.lp,
-                          global_schema=False)
+                          global_schema=False,
+                          flags=ldb.FLG_FAKE_TXN_NESTING)
         ctx.samdb.set_invocation_id(str(ctx.invocation_id))
         ctx.local_samdb = ctx.samdb
 
-- 
2.9.4


From 39ffbb0f8764cd80224933ed7d20d9a9f7278f4c Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Wed, 28 Jun 2017 17:37:29 +1200
Subject: [PATCH 22/25] ldb: Honour new flag LDB_FLG_FAKE_TXN_NESTING

This will allow the fake nesting of transactions.  If not specified it
will be up to the backend to nest if possible.  The caller will need
to choose a backend that matches the desired behaivour, however
ldb_tdb does not support nested transactions.

This is a behaviour change, previously this was allowed without the
flag being specified.

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 lib/ldb/common/ldb.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/lib/ldb/common/ldb.c b/lib/ldb/common/ldb.c
index 18e7efe..99a01e7 100644
--- a/lib/ldb/common/ldb.c
+++ b/lib/ldb/common/ldb.c
@@ -358,7 +358,8 @@ int ldb_transaction_start(struct ldb_context *ldb)
 		  ldb->transaction_active);
 
 	/* explicit transaction active, count nested requests */
-	if (ldb->transaction_active) {
+	if ((ldb->flags & LDB_FLG_FAKE_TXN_NESTING)
+	    && ldb->transaction_active) {
 		ldb->transaction_active++;
 		return LDB_SUCCESS;
 	}
@@ -403,10 +404,12 @@ int ldb_transaction_prepare_commit(struct ldb_context *ldb)
 	if (ldb->prepare_commit_done) {
 		return LDB_SUCCESS;
 	}
-
-	/* commit only when all nested transactions are complete */
-	if (ldb->transaction_active > 1) {
-		return LDB_SUCCESS;
+		
+	if (ldb->flags & LDB_FLG_FAKE_TXN_NESTING) {
+		/* commit only when all nested transactions are complete */
+		if (ldb->transaction_active > 1) {
+			return LDB_SUCCESS;
+		}
 	}
 
 	ldb->prepare_commit_done = true;
@@ -468,7 +471,8 @@ int ldb_transaction_commit(struct ldb_context *ldb)
 		  ldb->transaction_active);
 
 	/* commit only when all nested transactions are complete */
-	if (ldb->transaction_active > 0) {
+	if ((ldb->flags & LDB_FLG_FAKE_TXN_NESTING) &&
+	    ldb->transaction_active > 0) {
 		return LDB_SUCCESS;
 	}
 
@@ -518,7 +522,8 @@ int ldb_transaction_cancel(struct ldb_context *ldb)
 		  ldb->transaction_active);
 
 	/* really cancel only if all nested transactions are complete */
-	if (ldb->transaction_active > 0) {
+	if (ldb->flags & LDB_FLG_FAKE_TXN_NESTING &&
+	    ldb->transaction_active > 0) {
 		return LDB_SUCCESS;
 	}
 
-- 
2.9.4


From 780bbd865585b35d53d55af7c4d4380b145b89f6 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Wed, 28 Jun 2017 16:33:51 +1200
Subject: [PATCH 23/25] ABI for ldb without nested transactions

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 lib/ldb/ABI/ldb-1.2.0.sigs | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/ldb/ABI/ldb-1.2.0.sigs b/lib/ldb/ABI/ldb-1.2.0.sigs
index 1be2ae7..25aa51b 100644
--- a/lib/ldb/ABI/ldb-1.2.0.sigs
+++ b/lib/ldb/ABI/ldb-1.2.0.sigs
@@ -6,6 +6,7 @@ ldb_attr_dn: int (const char *)
 ldb_attr_in_list: int (const char * const *, const char *)
 ldb_attr_list_copy: const char **(TALLOC_CTX *, const char * const *)
 ldb_attr_list_copy_add: const char **(TALLOC_CTX *, const char * const *, const char *)
+ldb_autotransaction_request: int (struct ldb_context *, struct ldb_request *)
 ldb_base64_decode: int (char *)
 ldb_base64_encode: char *(TALLOC_CTX *, const char *, int)
 ldb_binary_decode: struct ldb_val (TALLOC_CTX *, const char *)
-- 
2.9.4


From 8f604a397dd6581e879309789b33813289c02efd Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Wed, 3 May 2017 06:34:01 +0200
Subject: [PATCH 24/25] drsuapi: Improve debugging in DsAddEntry()

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 source4/rpc_server/drsuapi/addentry.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/source4/rpc_server/drsuapi/addentry.c b/source4/rpc_server/drsuapi/addentry.c
index d071830..7cfb3fd 100644
--- a/source4/rpc_server/drsuapi/addentry.c
+++ b/source4/rpc_server/drsuapi/addentry.c
@@ -186,6 +186,8 @@ WERROR dcesrv_drsuapi_DsAddEntry(struct dcesrv_call_state *dce_call, TALLOC_CTX
 	case 2:
 		ret = ldb_transaction_start(b_state->sam_ctx);
 		if (ret != LDB_SUCCESS) {
+			DBG_ERR("DsAddEntry start transaction failed: %s\n",
+				ldb_errstring(b_state->sam_ctx));
 			return WERR_DS_DRA_INTERNAL_ERROR;
 		}
 
-- 
2.9.4


From 69ad4e58861cdcf3ed6294bc47c99c09187a8751 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Wed, 28 Jun 2017 17:34:05 +1200
Subject: [PATCH 25/25] dsdb: Improve debugging on start transacton failure

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 source4/dsdb/repl/replicated_objects.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/source4/dsdb/repl/replicated_objects.c b/source4/dsdb/repl/replicated_objects.c
index 98a16b0..ffc3290 100644
--- a/source4/dsdb/repl/replicated_objects.c
+++ b/source4/dsdb/repl/replicated_objects.c
@@ -842,7 +842,8 @@ WERROR dsdb_replicated_objects_commit(struct ldb_context *ldb,
 	 */
 	ret = ldb_transaction_start(ldb);
 	if (ret != LDB_SUCCESS) {
-		DEBUG(0,(__location__ " Failed to start transaction\n"));
+		DEBUG(0,(__location__ " Failed to start transaction: %s\n",
+			 ldb_errstring(ldb)));
 		return WERR_FOOBAR;
 	}
 
-- 
2.9.4



More information about the samba-technical mailing list