[389-devel] Please review: reduce calls to csn_as_string and slapi_log_error

2011-11-16 Thread Rich Megginson


From d5312b2bd400695875c1fe68a6a54c612f09eec9 Mon Sep 17 00:00:00 2001
From: Rich Megginson rmegg...@redhat.com
Date: Tue, 15 Nov 2011 20:26:46 -0700
Subject: [PATCH] reduce calls to csn_as_string and slapi_log_error

csn_as_string is an expensive operation - we should reduce calls to it
many of these calls are in calls to slapi_log_error at the
REPL log level where we want to print the csn string to the error log
When we call slapi_log_error(REPL,...), any nested functions are called
first, which means that even if the log level is REPL and nothing is logged,
a lot of computation may occur for nothing.  We should protect these calls
using slapi_is_loglevel_set().
---
 ldap/servers/plugins/replication/cl5_api.c |  147 
 ldap/servers/plugins/replication/csnpl.c   |6 +-
 .../plugins/replication/repl5_inc_protocol.c   |   53 
 ldap/servers/plugins/replication/repl5_plugins.c   |   12 +-
 ldap/servers/plugins/replication/repl5_replica.c   |   34 +++--
 ldap/servers/plugins/replication/repl5_ruv.c   |   41 --
 6 files changed, 177 insertions(+), 116 deletions(-)

diff --git a/ldap/servers/plugins/replication/cl5_api.c b/ldap/servers/plugins/replication/cl5_api.c
index 2a834d7..0ea90d1 100644
--- a/ldap/servers/plugins/replication/cl5_api.c
+++ b/ldap/servers/plugins/replication/cl5_api.c
@@ -3532,9 +3532,11 @@ static void _cl5TrimFile (Object *obj, long *numToTrim)
 }
 else
 {
-	slapi_log_error (SLAPI_LOG_REPL, NULL,
-		Changelog purge skipped anchor csn %s\n,
-		csn_as_string (maxcsn, PR_FALSE, strCSN));
+	if (slapi_is_loglevel_set(SLAPI_LOG_REPL)) {
+		slapi_log_error (SLAPI_LOG_REPL, NULL,
+		 Changelog purge skipped anchor csn %s\n,
+		 csn_as_string (maxcsn, PR_FALSE, strCSN));
+	}
 
 	/* extra read to skip the current record */
 	cl5_operation_parameters_done (op);
@@ -5020,10 +5022,13 @@ static int _cl5PositionCursorForReplay (ReplicaId consumerRID, const RUV *consum
 PR_ASSERT (supplierRuv);
 
 	agmt_name = get_thread_private_agmtname();
-slapi_log_error(SLAPI_LOG_REPL, NULL, _cl5PositionCursorForReplay (%s): Consumer RUV:\n, agmt_name);
-ruv_dump (consumerRuv, agmt_name, NULL);
-slapi_log_error(SLAPI_LOG_REPL, NULL, _cl5PositionCursorForReplay (%s): Supplier RUV:\n, agmt_name);
-ruv_dump (supplierRuv, agmt_name, NULL);
+
+	if (slapi_is_loglevel_set(SLAPI_LOG_REPL)) {
+		slapi_log_error(SLAPI_LOG_REPL, NULL, _cl5PositionCursorForReplay (%s): Consumer RUV:\n, agmt_name);
+		ruv_dump (consumerRuv, agmt_name, NULL);
+		slapi_log_error(SLAPI_LOG_REPL, NULL, _cl5PositionCursorForReplay (%s): Supplier RUV:\n, agmt_name);
+		ruv_dump (supplierRuv, agmt_name, NULL);
+	}

 	/*
 	 * get the sorted list of SupplierMinCSN (if no ConsumerMaxCSN)
@@ -5054,7 +5059,6 @@ static int _cl5PositionCursorForReplay (ReplicaId consumerRID, const RUV *consum
 			continue;
 
 startCSN = csns[i];
-csn_as_string(startCSN, PR_FALSE, csnStr); 
 
 		rc = clcache_get_buffer ( clcache, file-db, consumerRID, consumerRuv, supplierRuv );
 		if ( rc != 0 ) goto done;
@@ -5085,23 +5089,28 @@ static int _cl5PositionCursorForReplay (ReplicaId consumerRID, const RUV *consum
 if ((RUV_SUCCESS == ruv_get_min_csn(supplierRuv, startCSN)) 
 startCSN)
 { /* must now free startCSN */
-csn_as_string(startCSN, PR_FALSE, csnStr); 
-slapi_log_error(SLAPI_LOG_REPL, repl_plugin_name_cl, 
-%s: CSN %s not found and no purging, probably a reinit\n,
-agmt_name, csnStr);
-slapi_log_error(SLAPI_LOG_REPL, repl_plugin_name_cl, 
-%s: Will try to use supplier min CSN %s to load changelog\n,
-agmt_name, csnStr);
+if (slapi_is_loglevel_set(SLAPI_LOG_REPL)) {
+csn_as_string(startCSN, PR_FALSE, csnStr); 
+slapi_log_error(SLAPI_LOG_REPL, repl_plugin_name_cl, 
+%s: CSN %s not found and no purging, probably a reinit\n,
+agmt_name, csnStr);
+slapi_log_error(SLAPI_LOG_REPL, repl_plugin_name_cl, 
+%s: Will try to use supplier min CSN %s to load changelog\n,
+agmt_name, csnStr);
+}
 rc = clcache_load_buffer (clcache, startCSN, DB_SET);
 }
 else
 {
-slapi_log_error(SLAPI_LOG_FATAL, repl_plugin_name_cl,
-%s: CSN %s not found and no purging, probably a reinit\n,
-agmt_name, csnStr);
-slapi_log_error(SLAPI_LOG_FATAL, repl_plugin_name_cl, 
-%s: Could not get the min csn from the supplier RUV\n,
-   

Re: [389-devel] Please review: reduce calls to csn_as_string and slapi_log_error

2011-11-16 Thread Nathan Kinder

On 11/16/2011 08:48 AM, Rich Megginson wrote:




--
389-devel mailing list
389-devel@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/389-devel

ack.
--
389-devel mailing list
389-devel@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/389-devel