[Fedora-directory-commits] ldapserver/ldap/servers/plugins/replication windows_private.c, 1.15, 1.16 windows_prot_private.h, 1.7, 1.8 windows_protocol_util.c, 1.32, 1.33 windowsrepl.h, 1.12, 1.13
Nathan Kinder (nkinder)
fedora-directory-commits at redhat.com
Mon Sep 17 19:18:33 UTC 2007
Author: nkinder
Update of /cvs/dirsec/ldapserver/ldap/servers/plugins/replication
In directory cvs-int.fedora.redhat.com:/tmp/cvs-serv19325/plugins/replication
Modified Files:
windows_private.c windows_prot_private.h
windows_protocol_util.c windowsrepl.h
Log Message:
Resolves: 242551
Summary: Performance cleanup of sync code. Improve tombstone search performance.
Index: windows_private.c
===================================================================
RCS file: /cvs/dirsec/ldapserver/ldap/servers/plugins/replication/windows_private.c,v
retrieving revision 1.15
retrieving revision 1.16
diff -u -r1.15 -r1.16
--- windows_private.c 12 Sep 2007 23:05:24 -0000 1.15
+++ windows_private.c 17 Sep 2007 19:18:30 -0000 1.16
@@ -66,6 +66,10 @@
char *windows_domain;
int isnt4;
int iswin2k3;
+ /* This filter is used to determine if an entry belongs to this agreement. We put it here
+ * so we only have to allocate each filter once instead of doing it every time we receive a change. */
+ Slapi_Filter *directory_filter; /* Used for checking if local entries need to be sync'd to AD */
+ Slapi_Filter *deleted_filter; /* Used for checking if an entry is an AD tombstone */
};
static int
@@ -192,6 +196,8 @@
dp = (Dirsync_Private *)slapi_ch_calloc(sizeof(Dirsync_Private),1);
dp->dirsync_maxattributecount = -1;
+ dp->directory_filter = NULL;
+ dp->deleted_filter = NULL;
LDAPDebug( LDAP_DEBUG_TRACE, "<= windows_private_new\n", 0, 0, 0 );
return dp;
@@ -206,8 +212,8 @@
PR_ASSERT(dp != NULL);
- /* DBDB: need to free payoad here */
-
+ slapi_filter_free(dp->directory_filter, 1);
+ slapi_filter_free(dp->deleted_filter, 1);
slapi_ch_free((void **)dp);
LDAPDebug( LDAP_DEBUG_TRACE, "<= windows_private_delete\n", 0, 0, 0 );
@@ -278,6 +284,53 @@
LDAPDebug( LDAP_DEBUG_TRACE, "<= windows_private_set_iswin2k3\n", 0, 0, 0 );
}
+/* Returns a copy of the Slapi_Filter pointer. The caller should not free it */
+Slapi_Filter* windows_private_get_directory_filter(const Repl_Agmt *ra)
+{
+ Dirsync_Private *dp;
+
+ LDAPDebug( LDAP_DEBUG_TRACE, "=> windows_private_get_directory_filter\n", 0, 0, 0 );
+
+ PR_ASSERT(ra);
+
+ dp = (Dirsync_Private *) agmt_get_priv(ra);
+ PR_ASSERT (dp);
+
+ if (dp->directory_filter == NULL) {
+ char *string_filter = slapi_ch_strdup("(&(|(objectclass=ntuser)(objectclass=ntgroup))(ntUserDomainId=*))");
+ /* The filter gets freed in windows_agreement_delete() */
+ dp->directory_filter = slapi_str2filter( string_filter );
+ slapi_ch_free_string(&string_filter);
+ }
+
+ LDAPDebug( LDAP_DEBUG_TRACE, "<= windows_private_get_directory_filter\n", 0, 0, 0 );
+
+ return dp->directory_filter;
+}
+
+/* Returns a copy of the Slapi_Filter pointer. The caller should not free it */
+Slapi_Filter* windows_private_get_deleted_filter(const Repl_Agmt *ra)
+{
+ Dirsync_Private *dp;
+
+ LDAPDebug( LDAP_DEBUG_TRACE, "=> windows_private_get_deleted_filter\n", 0, 0, 0 );
+
+ PR_ASSERT(ra);
+
+ dp = (Dirsync_Private *) agmt_get_priv(ra);
+ PR_ASSERT (dp);
+
+ if (dp->deleted_filter == NULL) {
+ char *string_filter = slapi_ch_strdup("(isdeleted=*)");
+ /* The filter gets freed in windows_agreement_delete() */
+ dp->deleted_filter = slapi_str2filter( string_filter );
+ slapi_ch_free_string(&string_filter);
+ }
+
+ LDAPDebug( LDAP_DEBUG_TRACE, "<= windows_private_get_deleted_filter\n", 0, 0, 0 );
+
+ return dp->deleted_filter;
+}
/* Returns a copy of the Slapi_DN pointer, no need to free it */
const Slapi_DN* windows_private_get_windows_subtree (const Repl_Agmt *ra)
Index: windows_prot_private.h
===================================================================
RCS file: /cvs/dirsec/ldapserver/ldap/servers/plugins/replication/windows_prot_private.h,v
retrieving revision 1.7
retrieving revision 1.8
diff -u -r1.7 -r1.8
--- windows_prot_private.h 10 Nov 2006 23:45:17 -0000 1.7
+++ windows_prot_private.h 17 Sep 2007 19:18:30 -0000 1.8
@@ -52,35 +52,6 @@
#define ACQUIRE_CONSUMER_WAS_UPTODATE 104
#define ACQUIRE_TRANSIENT_ERROR 105
-typedef struct windows_private_repl_protocol
-{
- void (*delete)(struct windows_private_repl_protocol **);
- void (*run)(struct windows_private_repl_protocol *);
- int (*stop)(struct windows_private_repl_protocol *);
- int (*status)(struct windows_private_repl_protocol *);
- void (*notify_update)(struct windows_private_repl_protocol *);
- void (*notify_agmt_changed)(struct windows_private_repl_protocol *);
- void (*notify_window_opened)(struct windows_private_repl_protocol *);
- void (*notify_window_closed)(struct windows_private_repl_protocol *);
- void (*update_now)(struct windows_private_repl_protocol *);
- PRLock *lock;
- PRCondVar *cvar;
- int stopped;
- int terminate;
- PRUint32 eventbits;
- Repl_Connection *conn;
- int last_acquire_response_code;
- Repl_Agmt *agmt;
- Object *replica_object;
- void *private;
- PRBool replica_acquired;
-} Windows_Private_Repl_Protocol;
-
-/*
-extern Windows_Private_Repl_Protocol *Windows_Inc_Protocol_new();
-extern Windows_Private_Repl_Protocol *Windows_Tot_Protocol_new();
-*/
-
#define PROTOCOL_TERMINATION_NORMAL 301
#define PROTOCOL_TERMINATION_ABNORMAL 302
#define PROTOCOL_TERMINATION_NEEDS_TOTAL_UPDATE 303
Index: windows_protocol_util.c
===================================================================
RCS file: /cvs/dirsec/ldapserver/ldap/servers/plugins/replication/windows_protocol_util.c,v
retrieving revision 1.32
retrieving revision 1.33
diff -u -r1.32 -r1.33
--- windows_protocol_util.c 12 Sep 2007 23:05:25 -0000 1.32
+++ windows_protocol_util.c 17 Sep 2007 19:18:30 -0000 1.33
@@ -65,10 +65,12 @@
static Slapi_DN* map_dn_group(Slapi_DN *sdn, int map_to, const Slapi_DN *root);
static void make_mods_from_entries(Slapi_Entry *new_entry, Slapi_Entry *existing_entry, LDAPMod ***attrs);
static void windows_map_mods_for_replay(Private_Repl_Protocol *prp,LDAPMod **original_mods, LDAPMod ***returned_mods, int is_user, char** password);
-static int is_subject_of_agreemeent_local(const Slapi_Entry *local_entry,const Repl_Agmt *ra);
+static int is_subject_of_agreement_local(const Slapi_Entry *local_entry,const Repl_Agmt *ra);
static int windows_create_remote_entry(Private_Repl_Protocol *prp,Slapi_Entry *original_entry, Slapi_DN *remote_sdn, Slapi_Entry **remote_entry, char** password);
static int windows_get_local_entry(const Slapi_DN* local_dn,Slapi_Entry **local_entry);
static int windows_get_local_entry_by_uniqueid(Private_Repl_Protocol *prp,const char* uniqueid,Slapi_Entry **local_entry);
+static int windows_get_local_tombstone_by_uniqueid(Private_Repl_Protocol *prp,const char* uniqueid,Slapi_Entry **local_entry);
+static int windows_search_local_entry_by_uniqueid(Private_Repl_Protocol *prp, const char *uniqueid, char ** attrs, Slapi_Entry **ret_entry, int tombstone, void * component_identity);
static int map_entry_dn_outbound(Slapi_Entry *e, Slapi_DN **dn, Private_Repl_Protocol *prp, int *missing_entry, int want_guid);
static char* extract_ntuserdomainid_from_entry(Slapi_Entry *e);
static char* extract_container(const Slapi_DN *entry_dn, const Slapi_DN *suffix_dn);
@@ -76,7 +78,7 @@
static int windows_get_remote_tombstone(Private_Repl_Protocol *prp, const Slapi_DN* remote_dn,Slapi_Entry **remote_entry);
static int windows_reanimate_tombstone(Private_Repl_Protocol *prp, const Slapi_DN* tombstone_dn, const char* new_dn);
static const char* op2string (int op);
-static int is_subject_of_agreemeent_remote(Slapi_Entry *e, const Repl_Agmt *ra);
+static int is_subject_of_agreement_remote(Slapi_Entry *e, const Repl_Agmt *ra);
static int map_entry_dn_inbound(Slapi_Entry *e, Slapi_DN **dn, const Repl_Agmt *ra);
static int windows_update_remote_entry(Private_Repl_Protocol *prp,Slapi_Entry *remote_entry,Slapi_Entry *local_entry);
static int is_guid_dn(Slapi_DN *remote_dn);
@@ -405,7 +407,7 @@
int missing_entry = 0;
Slapi_DN *remote_dn = NULL;
/* Now map the DN */
- is_ours = is_subject_of_agreemeent_local(local_entry,prp->agmt);
+ is_ours = is_subject_of_agreement_local(local_entry,prp->agmt);
if (is_ours)
{
map_entry_dn_outbound(local_entry,&remote_dn,prp,&missing_entry, 0 /* don't want GUID form here */);
@@ -447,7 +449,7 @@
retval = windows_get_remote_entry(prp,original_dn,&remote_entry);
if (remote_entry && 0 == retval)
{
- is_ours = is_subject_of_agreemeent_remote(remote_entry,prp->agmt);
+ is_ours = is_subject_of_agreement_remote(remote_entry,prp->agmt);
if (is_ours)
{
retval = map_entry_dn_inbound(remote_entry,&local_dn,prp->agmt);
@@ -1121,10 +1123,16 @@
local_dn = slapi_sdn_new_dn_byref( op->target_address.dn );
/* Since we have the target uniqueid in the op structure, let's
- * fetch the local entry here using it.
+ * fetch the local entry here using it. We do not want to search
+ * across tombstone entries unless we are dealing with a delete
+ * operation here since searching across tombstones can be very
+ * inefficient as the tombstones build up.
*/
-
- rc = windows_get_local_entry_by_uniqueid(prp, op->target_address.uniqueid, &local_entry);
+ if (op->operation_type != SLAPI_OPERATION_DELETE) {
+ rc = windows_get_local_entry_by_uniqueid(prp, op->target_address.uniqueid, &local_entry);
+ } else {
+ rc = windows_get_local_tombstone_by_uniqueid(prp, op->target_address.uniqueid, &local_entry);
+ }
if (rc)
{
@@ -1135,7 +1143,7 @@
goto error;
}
- is_ours = is_subject_of_agreemeent_local(local_entry, prp->agmt);
+ is_ours = is_subject_of_agreement_local(local_entry, prp->agmt);
windows_is_local_entry_user_or_group(local_entry,&is_user,&is_group);
slapi_log_error(SLAPI_LOG_REPL, repl_plugin_name,
@@ -1839,25 +1847,15 @@
/* Is this entry a tombstone ? */
static int
-is_tombstone(Slapi_Entry *e)
+is_tombstone(Private_Repl_Protocol *prp, Slapi_Entry *e)
{
int retval = 0;
- char *string_deleted = slapi_ch_strdup("(isdeleted=*)");
-
- /* DBDB: we should allocate these filters once and keep them around for better performance */
- Slapi_Filter *filter_deleted = slapi_str2filter( string_deleted );
-
- slapi_ch_free_string(&string_deleted);
- /* DBDB: this should be one filter, the code originally tested separately and hasn't been fixed yet */
- if ( (slapi_filter_test_simple( e, filter_deleted ) == 0) )
+ if ( (slapi_filter_test_simple( e, (Slapi_Filter*)windows_private_get_deleted_filter(prp->agmt) ) == 0) )
{
retval = 1;
}
- slapi_filter_free(filter_deleted,1);
- filter_deleted = NULL;
-
return retval;
}
@@ -2724,7 +2722,7 @@
* and does it have the right attribute values for sync ?)
*/
static int
-is_subject_of_agreemeent_local(const Slapi_Entry *local_entry, const Repl_Agmt *ra)
+is_subject_of_agreement_local(const Slapi_Entry *local_entry, const Repl_Agmt *ra)
{
int retval = 0;
int is_in_subtree = 0;
@@ -2741,23 +2739,16 @@
{
/* Next test for the correct kind of entry */
if (local_entry) {
- /* DBDB: we should allocate these filters once and keep them around for better performance */
- char *string_filter = slapi_ch_strdup("(&(|(objectclass=ntuser)(objectclass=ntgroup))(ntUserDomainId=*))");
- Slapi_Filter *filter = slapi_str2filter( string_filter );
-
- slapi_ch_free_string(&string_filter);
- if (slapi_filter_test_simple( (Slapi_Entry*)local_entry, filter ) == 0)
+ if (slapi_filter_test_simple( (Slapi_Entry*)local_entry,
+ (Slapi_Filter*)windows_private_get_directory_filter(ra)) == 0)
{
retval = 1;
}
-
- slapi_filter_free(filter,1);
- filter = NULL;
} else
{
/* Error: couldn't find the entry */
slapi_log_error(SLAPI_LOG_FATAL, windows_repl_plugin_name,
- "failed to find entry in is_subject_of_agreemeent_local: %d\n", retval);
+ "failed to find entry in is_subject_of_agreement_local: %d\n", retval);
retval = 0;
}
}
@@ -2767,7 +2758,7 @@
/* Tests if the entry is subject to our agreement (i.e. is it in the sync'ed subtree in AD and either a user or a group ?) */
static int
-is_subject_of_agreemeent_remote(Slapi_Entry *e, const Repl_Agmt *ra)
+is_subject_of_agreement_remote(Slapi_Entry *e, const Repl_Agmt *ra)
{
int retval = 0;
int is_in_subtree = 0;
@@ -3342,7 +3333,7 @@
int missing_entry = 0;
const Slapi_DN *local_dn = slapi_entry_get_sdn_const(e);
/* First check if the entry is for us */
- is_ours = is_subject_of_agreemeent_local(e, prp->agmt);
+ is_ours = is_subject_of_agreement_local(e, prp->agmt);
slapi_log_error(SLAPI_LOG_REPL, repl_plugin_name,
"%s: windows_process_total_entry: Looking dn=\"%s\" (%s)\n",
agmt_get_long_name(prp->agmt), slapi_sdn_get_dn(slapi_entry_get_sdn_const(e)), is_ours ? "ours" : "not ours");
@@ -3366,8 +3357,8 @@
return retval;
}
-int
-windows_search_local_entry_by_uniqueid(Private_Repl_Protocol *prp, const char *uniqueid, char ** attrs, Slapi_Entry **ret_entry , void * component_identity)
+static int
+windows_search_local_entry_by_uniqueid(Private_Repl_Protocol *prp, const char *uniqueid, char ** attrs, Slapi_Entry **ret_entry, int tombstone, void * component_identity)
{
Slapi_Entry **entries = NULL;
Slapi_PBlock *int_search_pb = NULL;
@@ -3377,7 +3368,15 @@
*ret_entry = NULL;
local_subtree = windows_private_get_directory_subtree(prp->agmt);
- filter_string = PR_smprintf("(&(|(objectclass=*)(objectclass=ldapsubentry)(objectclass=nsTombstone))(nsUniqueid=%s))",uniqueid);
+
+ /* Searching for tombstones can be expensive, so the caller needs to specify if
+ * we should search for a tombstone entry, or a normal entry. */
+ if (tombstone) {
+ filter_string = PR_smprintf("(&(objectclass=nsTombstone)(nsUniqueid=%s))", uniqueid);
+ } else {
+ filter_string = PR_smprintf("(&(|(objectclass=*)(objectclass=ldapsubentry))(nsUniqueid=%s))",uniqueid);
+ }
+
int_search_pb = slapi_pblock_new ();
slapi_search_internal_set_pb ( int_search_pb, slapi_sdn_get_dn(local_subtree), LDAP_SCOPE_SUBTREE, filter_string,
attrs ,
@@ -3412,7 +3411,7 @@
{
int retval = ENTRY_NOTFOUND;
Slapi_Entry *new_entry = NULL;
- windows_search_local_entry_by_uniqueid( prp, uniqueid, NULL, &new_entry,
+ windows_search_local_entry_by_uniqueid( prp, uniqueid, NULL, &new_entry, 0, /* Don't search tombstones */
repl_get_plugin_identity (PLUGIN_MULTIMASTER_REPLICATION));
if (new_entry)
{
@@ -3423,6 +3422,21 @@
}
static int
+windows_get_local_tombstone_by_uniqueid(Private_Repl_Protocol *prp,const char* uniqueid,Slapi_Entry **local_entry)
+{
+ int retval = ENTRY_NOTFOUND;
+ Slapi_Entry *new_entry = NULL;
+ windows_search_local_entry_by_uniqueid( prp, uniqueid, NULL, &new_entry, 1, /* Search for tombstones */
+ repl_get_plugin_identity (PLUGIN_MULTIMASTER_REPLICATION));
+ if (new_entry)
+ {
+ *local_entry = new_entry;
+ retval = 0;
+ }
+ return retval;
+}
+
+static int
windows_get_local_entry(const Slapi_DN* local_dn,Slapi_Entry **local_entry)
{
int retval = ENTRY_NOTFOUND;
@@ -3446,7 +3460,7 @@
/* deleted users are outside the 'correct container'.
They live in cn=deleted objects, windows_private_get_directory_subtree( prp->agmt) */
- if (is_tombstone(e))
+ if (is_tombstone(prp, e))
{
rc = map_tombstone_dn_inbound(e, &local_sdn, prp->agmt);
if ((0 == rc) && local_sdn)
@@ -3461,7 +3475,7 @@
} else
{
/* Is this entry one we should be interested in ? */
- if (is_subject_of_agreemeent_remote(e,prp->agmt))
+ if (is_subject_of_agreement_remote(e,prp->agmt))
{
/* First make its local DN */
rc = map_entry_dn_inbound(e, &local_sdn, prp->agmt);
Index: windowsrepl.h
===================================================================
RCS file: /cvs/dirsec/ldapserver/ldap/servers/plugins/replication/windowsrepl.h,v
retrieving revision 1.12
retrieving revision 1.13
diff -u -r1.12 -r1.13
--- windowsrepl.h 12 Sep 2007 23:05:25 -0000 1.12
+++ windowsrepl.h 17 Sep 2007 19:18:30 -0000 1.13
@@ -68,6 +68,8 @@
void windows_private_set_isnt4(const Repl_Agmt *ra, int isit);
int windows_private_get_iswin2k3(const Repl_Agmt *ra);
void windows_private_set_iswin2k3(const Repl_Agmt *ra, int isit);
+Slapi_Filter* windows_private_get_directory_filter(const Repl_Agmt *ra);
+Slapi_Filter* windows_private_get_deleted_filter(const Repl_Agmt *ra);
const char* windows_private_get_purl(const Repl_Agmt *ra);
/* in windows_connection.c */
More information about the Fedora-directory-commits
mailing list