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>
(cherry picked from commit 1e83435eac2cef03fccb4cf69ef5e0bfbd710410)

diff --git a/source3/printing/load.c b/source3/printing/load.c
index 136d055088fd260af73dddd9d6b2384c3355face..2ba3b2e106d854f4eb5901361aa351655ec4f804 100644 (file)
--- 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 dd7ba624590124cace8d73b97f2595b66256c168..25dd4c70cde5a766c6cf4d2dbec659c12e832f36 100644 (file)
--- 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 70562137ac87c4666cc5c45b96e8a474e00285a4..6c062c3294ed9616909977beab1a8172ebfc8c8b 100644 (file)
--- 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 0fc94733a08b47803cc04c7bae724fb0968ffe10..2355e29b495cf9b890582ffea2d5d814ad971c57 100644 (file)
--- a/source3/printing/printer_list.c
+++ b/source3/printing/printer_list.c
@@ -279,7 +279,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;
@@ -289,7 +290,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;
 }
@@ -359,7 +364,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;
@@ -412,8 +417,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;
@@ -422,7 +427,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 fb2e007ae6c43bea3e1ab806388f76d3aead9731..b12c1923e7295a27c258d278f869be99759348e8 100644 (file)
--- 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_ */