[PATCH] Change samba process model default to prefork

Tim Beale timbeale at catalyst.net.nz
Thu Jan 31 04:42:49 UTC 2019


As discussed here:
https://lists.samba.org/archive/samba-technical/2019-January/132269.html

The attached patches flip the process model default for v4.11.

CI link: https://gitlab.com/catalyst-samba/samba/pipelines/45518445

As for what does the 'standard' process-model mean... I agree the term
made more sense when it was the default. But I couldn't think of a
better name. And if we renamed it, then we should probably go through
and also rename all the underlying code as well. I suspect most users
probably won't try to deviate from the default process model (seeing as
we never documented how). So as far as samba usability goes, I'm not
sure that renaming this parameter is top of the list.

-------------- next part --------------
From eac0686e592e1fa3d220ad577f78caf69a8993c1 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Thu, 31 Jan 2019 12:08:00 +1300
Subject: [PATCH 1/4] samba: Change default process model to prefork

Prefork is the more sensible default option now, as it better
handles a large number of client connections.

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
 WHATSNEW.txt                  | 13 +++++++++++++
 docs-xml/manpages/samba.8.xml |  4 ++--
 source4/smbd/server.c         |  2 +-
 3 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/WHATSNEW.txt b/WHATSNEW.txt
index a3ed921..85c417a 100644
--- a/WHATSNEW.txt
+++ b/WHATSNEW.txt
@@ -16,6 +16,19 @@ UPGRADING
 NEW FEATURES/CHANGES
 ====================
 
+Default samba process model
+---------------------------
+
+The default for the --model argument passed to the samba executable has changed
+from 'standard' to 'prefork'. This means a difference in the number of samba
+child processes that are created to handle client connections. The previous
+default would create a separate process for every LDAP or NETLOGON client
+connection. For a network with a lot of persistent client connections, this
+could result in significant memory overhead.  Now, with the new default of
+'prefork', the LDAP, NETLOGON, and KDC services will create a fixed number of
+worker processes at startup and share the client connections amongst these
+workers. The number of worker processes can be configured by the 'prefork
+children' setting in the smb.conf (the default is 4).
 
 REMOVED FEATURES
 ================
diff --git a/docs-xml/manpages/samba.8.xml b/docs-xml/manpages/samba.8.xml
index 0d542b2..8d548d8 100644
--- a/docs-xml/manpages/samba.8.xml
+++ b/docs-xml/manpages/samba.8.xml
@@ -119,8 +119,8 @@
 			for each new client connection.</para></listitem>
 
 			<listitem><para><emphasis>prefork</emphasis></para>
-			<para>A process is started for each Samba service, and a
-			fixed number of worker processes are started for those
+			<para>The default. A process is started for each Samba service,
+			and a fixed number of worker processes are started for those
 			services that support it (currently LDAP, NETLOGON, and KDC).
 			The client connections are then shared amongst the worker
 			processes.
diff --git a/source4/smbd/server.c b/source4/smbd/server.c
index cf86049..ea421b3 100644
--- a/source4/smbd/server.c
+++ b/source4/smbd/server.c
@@ -452,7 +452,7 @@ static int binary_smbd_main(const char *binary_name,
 	init_module_fn *shared_init;
 	uint16_t stdin_event_flags;
 	NTSTATUS status;
-	const char *model = "standard";
+	const char *model = "prefork";
 	int max_runtime = 0;
 	struct stat st;
 	enum {
-- 
2.7.4


From 73f0c419b445e45f496bb691cdacfe7ebdfad00e Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Thu, 31 Jan 2019 13:12:43 +1300
Subject: [PATCH 2/4] selftest: Make process_model argument optional in
 check_or_start()

It's more realistic to *not* always specify a process-model, and rely on
the samba code to use the correct default. This patch changes selftest
so we only use the -M process-model option if a particular process_model
was specified. Otherwise the testenv will use whatever the default is.

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
 selftest/target/Samba4.pm | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/selftest/target/Samba4.pm b/selftest/target/Samba4.pm
index b662776..25fde3f 100755
--- a/selftest/target/Samba4.pm
+++ b/selftest/target/Samba4.pm
@@ -161,10 +161,13 @@ sub check_or_start($$$)
 			@preargs = split(/ /,$ENV{SAMBA_VALGRIND});
 		}
 
+		if (defined($process_model)) {
+			push @optargs, ("-M", $process_model);
+		}
 		close($env_vars->{STDIN_PIPE});
 		open STDIN, ">&", $STDIN_READER or die "can't dup STDIN_READER to STDIN: $!";
 
-		exec(@preargs, Samba::bindir_path($self, "samba"), "-M", $process_model, "-i", "--no-process-group", "--maximum-runtime=$self->{server_maxtime}", $env_vars->{CONFIGURATION}, @optargs) or die("Unable to start samba: $!");
+		exec(@preargs, Samba::bindir_path($self, "samba"), "-i", "--no-process-group", "--maximum-runtime=$self->{server_maxtime}", $env_vars->{CONFIGURATION}, @optargs) or die("Unable to start samba: $!");
 	}
 	$env_vars->{SAMBA_PID} = $pid;
 	print "DONE ($pid)\n";
-- 
2.7.4


From b6e51ed6cb21a3aecef900e3cf854ce15a09e7db Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Thu, 31 Jan 2019 12:45:31 +1300
Subject: [PATCH 3/4] selftest: Convert backup/restore testenvs to use default

These testenvs shouldn't be dependent on the process model at all, so we
should be able to convert them to the new default without any
repercussions.

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
 selftest/target/Samba4.pm | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/selftest/target/Samba4.pm b/selftest/target/Samba4.pm
index 25fde3f..59accbd 100755
--- a/selftest/target/Samba4.pm
+++ b/selftest/target/Samba4.pm
@@ -2894,7 +2894,7 @@ sub setup_backupfromdc
 		return undef;
 	}
 
-	if (not defined($self->check_or_start($env, "standard"))) {
+	if (not defined($self->check_or_start($env))) {
 	    return undef;
 	}
 
@@ -3104,7 +3104,7 @@ sub setup_restoredc
 	}
 
 	# start samba for the restored DC
-	if (not defined($self->check_or_start($env, "standard"))) {
+	if (not defined($self->check_or_start($env))) {
 	    return undef;
 	}
 
@@ -3146,7 +3146,7 @@ sub setup_renamedc
 	}
 
 	# start samba for the restored DC
-	if (not defined($self->check_or_start($env, "standard"))) {
+	if (not defined($self->check_or_start($env))) {
 	    return undef;
 	}
 
@@ -3195,7 +3195,7 @@ sub setup_offlinebackupdc
 	Samba::mk_krb5_conf($ctx);
 
 	# start samba for the restored DC
-	if (not defined($self->check_or_start($env, "standard"))) {
+	if (not defined($self->check_or_start($env))) {
 	    return undef;
 	}
 
@@ -3250,7 +3250,7 @@ sub setup_labdc
 	}
 
 	# start samba for the restored DC
-	if (not defined($self->check_or_start($env, "standard"))) {
+	if (not defined($self->check_or_start($env))) {
 	    return undef;
 	}
 
@@ -3348,7 +3348,7 @@ sub setup_customdc
 	Samba::mk_krb5_conf($ctx);
 
 	# start samba for the restored DC
-	if (not defined($self->check_or_start($env, "standard"))) {
+	if (not defined($self->check_or_start($env))) {
 	    return undef;
 	}
 
-- 
2.7.4


From 06df16f7460a22369330c17b3b3448cb7bc7b410 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Thu, 31 Jan 2019 13:30:07 +1300
Subject: [PATCH 4/4] man pages: Add note about standard process model

Calling this model the 'standard' model made a lot more sense when it
was the default. Add a small note explaining that it has this name for
historical reasons.

(The term 'standard' may have originally been chosen for some other
reason. However, it's hard to find the rationale behind the term from
back in 2005)

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
 docs-xml/manpages/samba.8.xml | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/docs-xml/manpages/samba.8.xml b/docs-xml/manpages/samba.8.xml
index 8d548d8..35b3e9f 100644
--- a/docs-xml/manpages/samba.8.xml
+++ b/docs-xml/manpages/samba.8.xml
@@ -116,7 +116,12 @@
 			<para>A process is created for each Samba service,
 			and for those services that support it (currently only
 			LDAP and NETLOGON) a new processes is started
-			for each new client connection.</para></listitem>
+			for each new client connection.</para>
+
+			<para>Historically, this was the 'standard' way Samba behaved
+			up until v4.10. Note that this model can be resource intensive
+			if you have a large number of client connections.</para>
+			</listitem>
 
 			<listitem><para><emphasis>prefork</emphasis></para>
 			<para>The default. A process is started for each Samba service,
-- 
2.7.4



More information about the samba-technical mailing list