printing: traverse_read the printer list for share updates

The printcap update procedure involves the background printer process
obtaining the printcap information from the printing backend, writing
this to printer_list.tdb, and then notifying all smbd processes of the
new list. The processes then all attempt to simultaneously traverse
printer_list.tdb, in order to update their local share lists.

With a large number of printers, and a large number of per-client smbd
processes, this traversal results in significant lock contention, mostly
due to the fact that the traversal is unnecessarily done with an
exclusive (write) lock on the printer_list.tdb database.

This commit changes the share update code path to perform a read-only
traversal.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=10652

Reported-by: Alex K <korobkin+samba@gmail.com>
Reported-by: Franz Pförtsch <franz.pfoertsch@brose.com>
Signed-off-by: David Disseldorp <ddiss@samba.org>
Reviewed-by: Andreas Schneider <asn@samba.org>

5 files changed, 17 insertions, 12 deletions

diff --git a/source3/printing/load.c b/source3/printing/load.c
index 136d055088..2ba3b2e106 100644
--- a/source3/printing/load.c
+++ b/source3/printing/load.c
@@ -71,5 +71,5 @@ void load_printers(struct tevent_context *ev,
 
 	/* load all printcap printers */
 	if (lp_load_printers() && lp_servicenumber(PRINTERS_NAME) >= 0)
-		pcap_printer_fn(lp_add_one_printer, NULL);
+		pcap_printer_read_fn(lp_add_one_printer, NULL);
 }
diff --git a/source3/printing/pcap.c b/source3/printing/pcap.c
index dd7ba62459..25dd4c70cd 100644
--- a/source3/printing/pcap.c
+++ b/source3/printing/pcap.c
@@ -229,11 +229,11 @@ void pcap_printer_fn_specific(const struct pcap_cache *pc,
 	return;
 }
 
-void pcap_printer_fn(void (*fn)(const char *, const char *, const char *, void *), void *pdata)
+void pcap_printer_read_fn(void (*fn)(const char *, const char *, const char *, void *), void *pdata)
 {
 	NTSTATUS status;
 
-	status = printer_list_run_fn(fn, pdata);
+	status = printer_list_read_run_fn(fn, pdata);
 	if (!NT_STATUS_IS_OK(status)) {
 		DEBUG(3, ("Failed to run fn for all printers!\n"));
 	}
diff --git a/source3/printing/pcap.h b/source3/printing/pcap.h
index 70562137ac..6c062c3294 100644
--- a/source3/printing/pcap.h
+++ b/source3/printing/pcap.h
@@ -39,7 +39,7 @@ bool pcap_cache_add(const char *name, const char *comment, const char *location)
 bool pcap_cache_loaded(void);
 bool pcap_cache_replace(const struct pcap_cache *cache);
 void pcap_printer_fn_specific(const struct pcap_cache *, void (*fn)(const char *, const char *, const char *, void *), void *);
-void pcap_printer_fn(void (*fn)(const char *, const char *, const char *, void *), void *);
+void pcap_printer_read_fn(void (*fn)(const char *, const char *, const char *, void *), void *);
 
 void pcap_cache_reload(struct tevent_context *ev,
 		       struct messaging_context *msg_ctx,
diff --git a/source3/printing/printer_list.c b/source3/printing/printer_list.c
index 4a66b96e19..9a9fa0b962 100644
--- a/source3/printing/printer_list.c
+++ b/source3/printing/printer_list.c
@@ -284,7 +284,8 @@ done:
 typedef int (printer_list_trv_fn_t)(struct db_record *, void *);
 
 static NTSTATUS printer_list_traverse(printer_list_trv_fn_t *fn,
-						void *private_data)
+				      void *private_data,
+				      bool read_only)
 {
 	struct db_context *db;
 	NTSTATUS status;
@@ -294,7 +295,11 @@ static NTSTATUS printer_list_traverse(printer_list_trv_fn_t *fn,
 		return NT_STATUS_INTERNAL_DB_CORRUPTION;
 	}
 
-	status = dbwrap_traverse(db, fn, private_data, NULL);
+	if (read_only) {
+		status = dbwrap_traverse_read(db, fn, private_data, NULL);
+	} else {
+		status = dbwrap_traverse(db, fn, private_data, NULL);
+	}
 
 	return status;
 }
@@ -364,7 +369,7 @@ NTSTATUS printer_list_clean_old(void)
 
 	state.status = NT_STATUS_OK;
 
-	status = printer_list_traverse(printer_list_clean_fn, &state);
+	status = printer_list_traverse(printer_list_clean_fn, &state, false);
 	if (NT_STATUS_EQUAL(status, NT_STATUS_UNSUCCESSFUL) &&
 	    !NT_STATUS_IS_OK(state.status)) {
 		status = state.status;
@@ -417,8 +422,8 @@ static int printer_list_exec_fn(struct db_record *rec, void *private_data)
 	return 0;
 }
 
-NTSTATUS printer_list_run_fn(void (*fn)(const char *, const char *, const char *, void *),
-			     void *private_data)
+NTSTATUS printer_list_read_run_fn(void (*fn)(const char *, const char *, const char *, void *),
+				  void *private_data)
 {
 	struct printer_list_exec_state state;
 	NTSTATUS status;
@@ -427,7 +432,7 @@ NTSTATUS printer_list_run_fn(void (*fn)(const char *, const char *, const char *
 	state.private_data = private_data;
 	state.status = NT_STATUS_OK;
 
-	status = printer_list_traverse(printer_list_exec_fn, &state);
+	status = printer_list_traverse(printer_list_exec_fn, &state, true);
 	if (NT_STATUS_EQUAL(status, NT_STATUS_UNSUCCESSFUL) &&
 	    !NT_STATUS_IS_OK(state.status)) {
 		status = state.status;
diff --git a/source3/printing/printer_list.h b/source3/printing/printer_list.h
index fb2e007ae6..b12c1923e7 100644
--- a/source3/printing/printer_list.h
+++ b/source3/printing/printer_list.h
@@ -100,6 +100,6 @@ NTSTATUS printer_list_mark_reload(void);
  */
 NTSTATUS printer_list_clean_old(void);
 
-NTSTATUS printer_list_run_fn(void (*fn)(const char *, const char *, const char *, void *),
-			     void *private_data);
+NTSTATUS printer_list_read_run_fn(void (*fn)(const char *, const char *, const char *, void *),
+				  void *private_data);
 #endif /* _PRINTER_LIST_H_ */