Cleanup patches for dbwrap_ctdb

Volker Lendecke Volker.Lendecke at SerNet.DE
Tue Mar 26 04:11:54 MDT 2013


Hi!

Attached find some cleanup patches for dbwrap_ctdb. Some
time ago, the dbwrap internal API was converted from a fetch
operation to a parse_record operation to avoid malloc/free
where possible. The ctdb backend implements the
parse_record operation using a fetch operation internally,
which is a bit counter-productive. The attached patchset
fixes this.

Please review & push to master.

Thanks,

Volker

-- 
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:kontakt at sernet.de
-------------- next part --------------
From c019175f5b1872f27c0385524837340c58d2c478 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 23 Nov 2012 17:54:57 +0100
Subject: [PATCH 1/4] ctdb-conn: Add ctdbd_parse

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/include/ctdbd_conn.h |    5 +++
 source3/lib/ctdbd_conn.c     |   71 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 76 insertions(+), 0 deletions(-)

diff --git a/source3/include/ctdbd_conn.h b/source3/include/ctdbd_conn.h
index 5778a92..295d41e 100644
--- a/source3/include/ctdbd_conn.h
+++ b/source3/include/ctdbd_conn.h
@@ -65,6 +65,11 @@ NTSTATUS ctdbd_migrate(struct ctdbd_connection *conn, uint32_t db_id,
 NTSTATUS ctdbd_fetch(struct ctdbd_connection *conn, uint32_t db_id,
 		     TDB_DATA key, TALLOC_CTX *mem_ctx, TDB_DATA *data,
 		     bool local_copy);
+NTSTATUS ctdbd_parse(struct ctdbd_connection *conn, uint32_t db_id,
+		     TDB_DATA key, bool local_copy,
+		     void (*parser)(TDB_DATA key, TDB_DATA data,
+				    void *private_data),
+		     void *private_data);
 
 NTSTATUS ctdbd_traverse(uint32_t db_id,
 			void (*fn)(TDB_DATA key, TDB_DATA data,
diff --git a/source3/lib/ctdbd_conn.c b/source3/lib/ctdbd_conn.c
index 2cf5e47..d77b03d 100644
--- a/source3/lib/ctdbd_conn.c
+++ b/source3/lib/ctdbd_conn.c
@@ -1494,6 +1494,77 @@ NTSTATUS ctdbd_fetch(struct ctdbd_connection *conn, uint32_t db_id,
 	return status;
 }
 
+/*
+ * Fetch a record and parse it
+ */
+NTSTATUS ctdbd_parse(struct ctdbd_connection *conn, uint32_t db_id,
+		     TDB_DATA key, bool local_copy,
+		     void (*parser)(TDB_DATA key, TDB_DATA data,
+				    void *private_data),
+		     void *private_data)
+{
+	struct ctdb_req_call req;
+	struct ctdb_reply_call *reply;
+	NTSTATUS status;
+	uint32_t flags;
+
+#ifdef HAVE_CTDB_WANT_READONLY_DECL
+	flags = local_copy ? CTDB_WANT_READONLY : 0;
+#else
+	flags = 0;
+#endif
+
+	ZERO_STRUCT(req);
+
+	req.hdr.length = offsetof(struct ctdb_req_call, data) + key.dsize;
+	req.hdr.ctdb_magic   = CTDB_MAGIC;
+	req.hdr.ctdb_version = CTDB_VERSION;
+	req.hdr.operation    = CTDB_REQ_CALL;
+	req.hdr.reqid        = ctdbd_next_reqid(conn);
+	req.flags            = flags;
+	req.callid           = CTDB_FETCH_FUNC;
+	req.db_id            = db_id;
+	req.keylen           = key.dsize;
+
+	status = ctdb_packet_send(
+		conn->pkt, 2,
+		data_blob_const(&req, offsetof(struct ctdb_req_call, data)),
+		data_blob_const(key.dptr, key.dsize));
+
+	if (!NT_STATUS_IS_OK(status)) {
+		DEBUG(3, ("ctdb_packet_send failed: %s\n", nt_errstr(status)));
+		return status;
+	}
+
+	status = ctdb_packet_flush(conn->pkt);
+
+	if (!NT_STATUS_IS_OK(status)) {
+		DEBUG(3, ("write to ctdbd failed: %s\n", nt_errstr(status)));
+		cluster_fatal("cluster dispatch daemon control write error\n");
+	}
+
+	status = ctdb_read_req(conn, req.hdr.reqid, NULL, (void *)&reply);
+
+	if (!NT_STATUS_IS_OK(status)) {
+		DEBUG(0, ("ctdb_read_req failed: %s\n", nt_errstr(status)));
+		goto fail;
+	}
+
+	if (reply->hdr.operation != CTDB_REPLY_CALL) {
+		DEBUG(0, ("received invalid reply\n"));
+		status = NT_STATUS_INTERNAL_ERROR;
+		goto fail;
+	}
+
+	parser(key, make_tdb_data(&reply->data[0], reply->datalen),
+	       private_data);
+
+	status = NT_STATUS_OK;
+ fail:
+	TALLOC_FREE(reply);
+	return status;
+}
+
 struct ctdbd_traverse_state {
 	void (*fn)(TDB_DATA key, TDB_DATA data, void *private_data);
 	void *private_data;
-- 
1.7.3.4


From 4b14199f6c3a1f94f84e2f399cd8ce5cdb012036 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sat, 24 Nov 2012 14:14:37 +0000
Subject: [PATCH 2/4] dbwrap-ctdb: Use ctdbd_parse in db_ctdb_parse_record

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/lib/dbwrap/dbwrap_ctdb.c |   11 ++---------
 1 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/source3/lib/dbwrap/dbwrap_ctdb.c b/source3/lib/dbwrap/dbwrap_ctdb.c
index 399c850..691acdb 100644
--- a/source3/lib/dbwrap/dbwrap_ctdb.c
+++ b/source3/lib/dbwrap/dbwrap_ctdb.c
@@ -1211,7 +1211,6 @@ static NTSTATUS db_ctdb_parse_record(struct db_context *db, TDB_DATA key,
 		db->private_data, struct db_ctdb_ctx);
 	struct db_ctdb_parse_record_state state;
 	NTSTATUS status;
-	TDB_DATA data;
 
 	state.parser = parser;
 	state.private_data = private_data;
@@ -1249,14 +1248,8 @@ static NTSTATUS db_ctdb_parse_record(struct db_context *db, TDB_DATA key,
 		return NT_STATUS_OK;
 	}
 
-	status = ctdbd_fetch(messaging_ctdbd_connection(), ctx->db_id, key,
-			     talloc_tos(), &data, state.ask_for_readonly_copy);
-	if (!NT_STATUS_IS_OK(status)) {
-		return status;
-	}
-	parser(key, data, private_data);
-	TALLOC_FREE(data.dptr);
-	return NT_STATUS_OK;
+	return ctdbd_parse(messaging_ctdbd_connection(), ctx->db_id, key,
+			   state.ask_for_readonly_copy, parser, private_data);
 }
 
 struct traverse_state {
-- 
1.7.3.4


From 66db3e0f529dfc27f3fbfbcfd2bcca1fda247e5d Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sat, 24 Nov 2012 14:15:38 +0000
Subject: [PATCH 3/4] ctdb-conn: remove ctdbd_fetch

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/include/ctdbd_conn.h |    3 --
 source3/lib/ctdbd_conn.c     |   81 ------------------------------------------
 2 files changed, 0 insertions(+), 84 deletions(-)

diff --git a/source3/include/ctdbd_conn.h b/source3/include/ctdbd_conn.h
index 295d41e..64cb1d5 100644
--- a/source3/include/ctdbd_conn.h
+++ b/source3/include/ctdbd_conn.h
@@ -62,9 +62,6 @@ NTSTATUS ctdbd_db_attach(struct ctdbd_connection *conn, const char *name,
 NTSTATUS ctdbd_migrate(struct ctdbd_connection *conn, uint32_t db_id,
 		       TDB_DATA key);
 
-NTSTATUS ctdbd_fetch(struct ctdbd_connection *conn, uint32_t db_id,
-		     TDB_DATA key, TALLOC_CTX *mem_ctx, TDB_DATA *data,
-		     bool local_copy);
 NTSTATUS ctdbd_parse(struct ctdbd_connection *conn, uint32_t db_id,
 		     TDB_DATA key, bool local_copy,
 		     void (*parser)(TDB_DATA key, TDB_DATA data,
diff --git a/source3/lib/ctdbd_conn.c b/source3/lib/ctdbd_conn.c
index d77b03d..1481a9c 100644
--- a/source3/lib/ctdbd_conn.c
+++ b/source3/lib/ctdbd_conn.c
@@ -1414,87 +1414,6 @@ NTSTATUS ctdbd_migrate(struct ctdbd_connection *conn, uint32_t db_id,
 }
 
 /*
- * remotely fetch a record (read-only)
- */
-NTSTATUS ctdbd_fetch(struct ctdbd_connection *conn, uint32_t db_id,
-		     TDB_DATA key, TALLOC_CTX *mem_ctx, TDB_DATA *data,
-		     bool local_copy)
-{
-	struct ctdb_req_call req;
-	struct ctdb_reply_call *reply;
-	NTSTATUS status;
-	uint32_t flags;
-
-#ifdef HAVE_CTDB_WANT_READONLY_DECL
-	flags = local_copy ? CTDB_WANT_READONLY : 0;
-#else
-	flags = 0;
-#endif
-
-	ZERO_STRUCT(req);
-
-	req.hdr.length = offsetof(struct ctdb_req_call, data) + key.dsize;
-	req.hdr.ctdb_magic   = CTDB_MAGIC;
-	req.hdr.ctdb_version = CTDB_VERSION;
-	req.hdr.operation    = CTDB_REQ_CALL;
-	req.hdr.reqid        = ctdbd_next_reqid(conn);
-	req.flags            = flags;
-	req.callid           = CTDB_FETCH_FUNC;
-	req.db_id            = db_id;
-	req.keylen           = key.dsize;
-
-	status = ctdb_packet_send(
-		conn->pkt, 2,
-		data_blob_const(&req, offsetof(struct ctdb_req_call, data)),
-		data_blob_const(key.dptr, key.dsize));
-
-	if (!NT_STATUS_IS_OK(status)) {
-		DEBUG(3, ("ctdb_packet_send failed: %s\n", nt_errstr(status)));
-		return status;
-	}
-
-	status = ctdb_packet_flush(conn->pkt);
-
-	if (!NT_STATUS_IS_OK(status)) {
-		DEBUG(3, ("write to ctdbd failed: %s\n", nt_errstr(status)));
-		cluster_fatal("cluster dispatch daemon control write error\n");
-	}
-
-	status = ctdb_read_req(conn, req.hdr.reqid, NULL, (void *)&reply);
-
-	if (!NT_STATUS_IS_OK(status)) {
-		DEBUG(0, ("ctdb_read_req failed: %s\n", nt_errstr(status)));
-		goto fail;
-	}
-
-	if (reply->hdr.operation != CTDB_REPLY_CALL) {
-		DEBUG(0, ("received invalid reply\n"));
-		status = NT_STATUS_INTERNAL_ERROR;
-		goto fail;
-	}
-
-	data->dsize = reply->datalen;
-	if (data->dsize == 0) {
-		data->dptr = NULL;
-		goto done;
-	}
-
-	data->dptr = (uint8 *)talloc_memdup(mem_ctx, &reply->data[0],
-					    reply->datalen);
-	if (data->dptr == NULL) {
-		DEBUG(0, ("talloc failed\n"));
-		status = NT_STATUS_NO_MEMORY;
-		goto fail;
-	}
-
- done:
-	status = NT_STATUS_OK;
- fail:
-	TALLOC_FREE(reply);
-	return status;
-}
-
-/*
  * Fetch a record and parse it
  */
 NTSTATUS ctdbd_parse(struct ctdbd_connection *conn, uint32_t db_id,
-- 
1.7.3.4


From a1fb9213d46c4db016646a3f913c09603d106d9c Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sat, 24 Nov 2012 14:42:06 +0000
Subject: [PATCH 4/4] dbwrap-ctdb: Avoid a talloc_stackframe()

We have only a single allocation in this routine, so I think we can live
without a stackframe.

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/lib/dbwrap/dbwrap_ctdb.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/source3/lib/dbwrap/dbwrap_ctdb.c b/source3/lib/dbwrap/dbwrap_ctdb.c
index 691acdb..e55689c 100644
--- a/source3/lib/dbwrap/dbwrap_ctdb.c
+++ b/source3/lib/dbwrap/dbwrap_ctdb.c
@@ -140,15 +140,13 @@ static NTSTATUS db_ctdb_ltdb_store(struct db_ctdb_ctx *db,
 				   struct ctdb_ltdb_header *header,
 				   TDB_DATA data)
 {
-	TALLOC_CTX *tmp_ctx = talloc_stackframe();
 	TDB_DATA rec;
 	int ret;
 
 	rec.dsize = data.dsize + sizeof(struct ctdb_ltdb_header);
-	rec.dptr = (uint8_t *)talloc_size(tmp_ctx, rec.dsize);
+	rec.dptr = (uint8_t *)talloc_size(talloc_tos(), rec.dsize);
 
 	if (rec.dptr == NULL) {
-		talloc_free(tmp_ctx);
 		return NT_STATUS_NO_MEMORY;
 	}
 
@@ -157,7 +155,7 @@ static NTSTATUS db_ctdb_ltdb_store(struct db_ctdb_ctx *db,
 
 	ret = tdb_store(db->wtdb->tdb, key, rec, TDB_REPLACE);
 
-	talloc_free(tmp_ctx);
+	talloc_free(rec.dptr);
 
 	return (ret == 0) ? NT_STATUS_OK
 			  : tdb_error_to_ntstatus(db->wtdb->tdb);
-- 
1.7.3.4



More information about the samba-technical mailing list