[PATCH] Improve replication test speed by using immediate events

Andrew Bartlett abartlet at samba.org
Fri Jul 21 09:10:27 UTC 2017


On Fri, 2017-07-21 at 18:40 +1200, Andrew Bartlett via samba-technical
wrote:
> On Fri, 2017-07-21 at 08:34 +0200, Stefan Metzmacher wrote:
> > Hi Andrew,
> > 
> > > > https://git.samba.org/?p=abartlet/samba.git/.git;a=shortlog;h=refs/heads/faster-drs-krb5-ccache-p-im
> > > 
> > > We don't need to constantly call tevent_create_immediate(service),
> > > we should do it only once on startup.
> > > 
> > > tevent_schedule_immediate() can be called again and again.
> > > 
> > > That way dreplsrv_pendingops_schedule_pull_now() don't
> > > need to return WERROR, but void.
> > 
> > Please also create a bugreport so we can backport that to 4.7.
> 
> Thank you so much for the feedback and guidance!
> 
> I'll do as requested, and I've filed:
> https://bugzilla.samba.org/show_bug.cgi?id=12921

See attached.  

Also, interestingly, with this I've been able to see the BAD_NET_RESP
error and got the logs to go with it.  If they are actually related,
then Tim's GET_TGT work will fix it.  (I'll write another mail about
that). 

I have this under another private autobuild just to be sure.

Thanks,

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 8e8fb8ef1c71bf0d08c9d26676ac3963a1d22f3d Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Fri, 21 Jul 2017 17:52:04 +1200
Subject: [PATCH] s4-drepl: Use tevent_schedule_immediate() in DsReplicaSync
 handler

When we are sent a DsReplicaSync() we should work on inbound replication
(ideally from the requested source, but so far we just start the whole queue)
right away, not after 1 second.

We should also target inbound replication, not any outbound replication
notification that may happen to be due.

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12921
---
 source4/dsdb/repl/drepl_periodic.c | 60 ++++++--------------------------------
 source4/dsdb/repl/drepl_service.c  | 11 ++++++-
 source4/dsdb/repl/drepl_service.h  | 18 ++++--------
 3 files changed, 24 insertions(+), 65 deletions(-)

diff --git a/source4/dsdb/repl/drepl_periodic.c b/source4/dsdb/repl/drepl_periodic.c
index d6b9467..64a53e0 100644
--- a/source4/dsdb/repl/drepl_periodic.c
+++ b/source4/dsdb/repl/drepl_periodic.c
@@ -134,63 +134,21 @@ void dreplsrv_run_pending_ops(struct dreplsrv_service *s)
 	}
 }
 
-static void dreplsrv_pending_run(struct dreplsrv_service *service);
-
-static void dreplsrv_pending_handler_te(struct tevent_context *ev, struct tevent_timer *te,
-					struct timeval t, void *ptr)
+static void dreplsrv_pending_pull_handler_im(struct tevent_context *ev,
+					     struct tevent_immediate *im,
+					     void *ptr)
 {
 	struct dreplsrv_service *service = talloc_get_type(ptr, struct dreplsrv_service);
 
-	service->pending.te = NULL;
-
-	dreplsrv_pending_run(service);
+	dreplsrv_run_pull_ops(service);
 }
 
-WERROR dreplsrv_pendingops_schedule(struct dreplsrv_service *service, uint32_t next_interval)
+void dreplsrv_pendingops_schedule_pull_now(struct dreplsrv_service *service)
 {
-	TALLOC_CTX *tmp_mem;
-	struct tevent_timer *new_te;
-	struct timeval next_time;
+	tevent_schedule_immediate(service->pending.im, service->task->event_ctx,
+				  dreplsrv_pending_pull_handler_im,
+				  service);
 
-	/* prevent looping */
-	if (next_interval == 0) {
-		next_interval = 1;
-	}
-
-	next_time = timeval_current_ofs(next_interval, 50);
-
-	if (service->pending.te) {
-		/*
-		 * if the timestamp of the new event is higher,
-		 * as current next we don't need to reschedule
-		 */
-		if (timeval_compare(&next_time, &service->pending.next_event) > 0) {
-			return WERR_OK;
-		}
-	}
-
-	/* reset the next scheduled timestamp */
-	service->pending.next_event = next_time;
-
-	new_te = tevent_add_timer(service->task->event_ctx, service,
-				  service->pending.next_event,
-			         dreplsrv_pending_handler_te, service);
-	W_ERROR_HAVE_NO_MEMORY(new_te);
-
-	tmp_mem = talloc_new(service);
-	DEBUG(4,("dreplsrv_pending_schedule(%u) %sscheduled for: %s\n",
-		next_interval,
-		(service->pending.te?"re":""),
-		nt_time_string(tmp_mem, timeval_to_nttime(&next_time))));
-	talloc_free(tmp_mem);
-
-	talloc_free(service->pending.te);
-	service->pending.te = new_te;
-
-	return WERR_OK;
+	return;
 }
 
-static void dreplsrv_pending_run(struct dreplsrv_service *service)
-{
-	dreplsrv_run_pending_ops(service);
-}
diff --git a/source4/dsdb/repl/drepl_service.c b/source4/dsdb/repl/drepl_service.c
index 39791b4..5b28363 100644
--- a/source4/dsdb/repl/drepl_service.c
+++ b/source4/dsdb/repl/drepl_service.c
@@ -339,7 +339,7 @@ static NTSTATUS drepl_replica_sync(struct irpc_message *msg,
 	 * schedule replication event to force
 	 * replication as soon as possible
 	 */
-	dreplsrv_pendingops_schedule(service, 0);
+	dreplsrv_pendingops_schedule_pull_now(service);
 
 done:
 	return NT_STATUS_OK;
@@ -486,6 +486,15 @@ static void dreplsrv_task_init(struct task_server *task)
 		return;
 	}
 
+	service->pending.im = tevent_create_immediate(service);
+	if (service->pending.im == NULL) {
+		task_server_terminate(task,
+				      "dreplsrv: Failed to create immediate "
+				      "task for future DsReplicaSync\n",
+				      true);
+		return;
+	}
+
 	/* if we are a RODC then we do not send DSReplicaSync*/
 	if (!service->am_rodc) {
 		service->notify.interval = lpcfg_parm_int(task->lp_ctx, NULL, "dreplsrv",
diff --git a/source4/dsdb/repl/drepl_service.h b/source4/dsdb/repl/drepl_service.h
index 6b85ad6..4b0e43f 100644
--- a/source4/dsdb/repl/drepl_service.h
+++ b/source4/dsdb/repl/drepl_service.h
@@ -187,21 +187,13 @@ struct dreplsrv_service {
 		struct tevent_timer *te;
 	} periodic;
 
-	/* some stuff for running only the pendings ops */
+	/* some stuff for running only the incoming notify ops */
 	struct {
-		/*
-		 * the interval between notify runs
-		 */
-		uint32_t interval;
-
-		/*
-		 * the timestamp for the next event,
-		 * this is the timstamp passed to event_add_timed()
+		/* 
+		 * here we have a reference to the immidicate event that was
+		 * scheduled from the DsReplicaSync
 		 */
-		struct timeval next_event;
-
-		/* here we have a reference to the timed event the schedules the notifies */
-		struct tevent_timer *te;
+		struct tevent_immediate *im;
 	} pending;
 
 	/* some stuff for notify processing */
-- 
2.9.4



More information about the samba-technical mailing list