瀏覽代碼

513916 Server doesn't ignore paged control, if page size and server's estimate of total no of entries are same
The code processing search results were returning the PAGE END without
knowing there are more entries to return or not. To learn it, introduced
"read ahead" one entry when it comes to the PAGE END. If there are more
entries, the code undo the read ahead, which prompts for the next page
on the client side. If there is no more entries, it returns the status
SEARCH END instead of PAGE END.

In addition to the read ahead implementation to fix the bug 513916,
* supporting Simple Paged Results for chaining backend is added.
* fixed a bug in idl_new_fetch (idl_new.c) -- idlistscanlimit was not
checked when the cursor comes to the end of an index file.

Noriko Hosoi 16 年之前
父節點
當前提交
0565e8c398

+ 3 - 0
ldap/servers/plugins/chainingdb/cb.h

@@ -400,6 +400,7 @@ typedef struct _cb_searchContext {
 	Slapi_Entry	*tobefreed;
 	LDAPMessage	*pending_result;
 	int 		pending_result_type;
+	Slapi_Entry	*readahead;
 } cb_searchContext;
 
 #define CB_REOPEN_CONN		-1968	/* Different from any LDAP_XXX errors */
@@ -481,6 +482,8 @@ int chainingdb_bind (Slapi_PBlock *pb );
 int cb_db_size (Slapi_PBlock *pb );
 int cb_back_close (Slapi_PBlock *pb );
 int cb_back_cleanup (Slapi_PBlock *pb );
+void chaining_back_search_results_release( void **sr );
+void chainingdb_prev_search_results ( Slapi_PBlock *pb );
 
 long cb_atol(char *str);
 

+ 4 - 0
ldap/servers/plugins/chainingdb/cb_init.c

@@ -94,6 +94,10 @@ chaining_back_init( Slapi_PBlock *pb )
 		(void *) chainingdb_build_candidate_list );
 	rc |= slapi_pblock_set( pb, SLAPI_PLUGIN_DB_NEXT_SEARCH_ENTRY_FN, 
 		(void *) chainingdb_next_search_entry );
+	rc |= slapi_pblock_set( pb, SLAPI_PLUGIN_DB_PREV_SEARCH_RESULTS_FN, 
+		(void *) chainingdb_prev_search_results );
+	rc |= slapi_pblock_set( pb, SLAPI_PLUGIN_DB_SEARCH_RESULTS_RELEASE_FN, 
+		(void *) chaining_back_search_results_release );
 	rc |= slapi_pblock_set( pb, SLAPI_PLUGIN_START_FN, 
 		(void *) chainingdb_start ) ;
 	rc |= slapi_pblock_set( pb, SLAPI_PLUGIN_DB_BIND_FN, 

+ 40 - 0
ldap/servers/plugins/chainingdb/cb_search.c

@@ -454,6 +454,18 @@ chainingdb_next_search_entry ( Slapi_PBlock *pb )
 		return 0;
 	}
 
+	if ( NULL != ctx->readahead) {
+		slapi_pblock_set( pb, SLAPI_SEARCH_RESULT_SET, ctx);
+		slapi_pblock_set( pb, SLAPI_SEARCH_RESULT_ENTRY, ctx->readahead);
+		if (ctx->tobefreed != ctx->readahead) {
+			slapi_entry_free(ctx->tobefreed);
+		}
+		ctx->tobefreed = ctx->readahead;
+		ctx->readahead = NULL;
+		cb_set_acl_policy(pb);
+		return 0;
+	}
+
 	if ( NULL != ctx->tobefreed ) {
 		slapi_entry_free(ctx->tobefreed);
 		ctx->tobefreed=NULL;
@@ -737,3 +749,31 @@ chaining_back_entry_release ( Slapi_PBlock *pb ) {
 	return 0;
 }
 
+void
+chaining_back_search_results_release ( void **sr )
+{
+    cb_searchContext *ctx = (cb_searchContext *)(*sr);
+
+    slapi_log_error( SLAPI_LOG_PLUGIN, CB_PLUGIN_SUBSYSTEM,
+                     "chaining_back_search_results_release\n");
+    if (ctx->readahead != ctx->tobefreed) {
+        slapi_entry_free(ctx->readahead);
+    }
+    slapi_entry_free(ctx->tobefreed);
+    ctx->tobefreed = NULL;
+    slapi_ch_free((void **)&ctx->data);
+    slapi_ch_free((void **)&ctx);
+    return;
+}
+
+void
+chainingdb_prev_search_results ( Slapi_PBlock *pb )
+{
+    cb_searchContext *ctx = NULL;
+    Slapi_Entry      *entry = NULL;
+
+    slapi_pblock_get( pb, SLAPI_SEARCH_RESULT_SET, &ctx );
+    slapi_pblock_get( pb, SLAPI_SEARCH_RESULT_ENTRY, &entry );
+    ctx->readahead = entry;
+    return;
+}

+ 3 - 1
ldap/servers/slapd/back-ldbm/idl_common.c

@@ -413,7 +413,9 @@ idl_iterator idl_iterator_increment(idl_iterator *i)
 idl_iterator idl_iterator_decrement(idl_iterator *i)
 {
 	size_t t = (size_t) *i;
-	t -= 1;
+	if (t > 0) {
+		t -= 1;
+	}
 	*i = (idl_iterator) t;
 	return *i;
 }

+ 4 - 5
ldap/servers/slapd/back-ldbm/idl_new.c

@@ -292,11 +292,6 @@ IDList * idl_new_fetch(
         }
 
         LDAPDebug(LDAP_DEBUG_TRACE, "bulk fetch buffer nids=%d\n", count, 0, 0); 
-
-        ret = cursor->c_get(cursor,&key,&data,DB_NEXT_DUP|DB_MULTIPLE);
-        if (0 != ret) {
-            break;
-        }
 #if defined(DB_ALLIDS_ON_READ)	
 		/* enforce the allids read limit */
 		if (NEW_IDL_NO_ALLID != *flag_err &&
@@ -307,6 +302,10 @@ IDList * idl_new_fetch(
 			break;
 		}
 #endif
+        ret = cursor->c_get(cursor,&key,&data,DB_NEXT_DUP|DB_MULTIPLE);
+        if (0 != ret) {
+            break;
+        }
    }
 #else
     for (;;) {

+ 2 - 0
ldap/servers/slapd/back-ldbm/init.c

@@ -184,6 +184,8 @@ ldbm_back_init( Slapi_PBlock *pb )
 	    (void *) ldbm_back_next_search_entry );
 	rc |= slapi_pblock_set( pb, SLAPI_PLUGIN_DB_NEXT_SEARCH_ENTRY_EXT_FN,
 	    (void *) ldbm_back_next_search_entry_ext );
+	rc |= slapi_pblock_set( pb, SLAPI_PLUGIN_DB_PREV_SEARCH_RESULTS_FN,
+	    (void *) ldbm_back_prev_search_results );
 	rc |= slapi_pblock_set( pb, SLAPI_PLUGIN_DB_ENTRY_RELEASE_FN,
 	    (void *) ldbm_back_entry_release );
 	rc |= slapi_pblock_set( pb, SLAPI_PLUGIN_DB_SEARCH_RESULTS_RELEASE_FN,

+ 16 - 0
ldap/servers/slapd/back-ldbm/ldbm_search.c

@@ -1418,6 +1418,22 @@ bail:
     return rc;
 }
 
+/*
+ * Move back the current position in the search result set by one.
+ * Paged Results needs to read ahead one entry to catch the end of the search
+ * result set at the last entry not to show the prompt when there is no more
+ * entries.
+ */
+void
+ldbm_back_prev_search_results( Slapi_PBlock *pb )
+{
+    back_search_result_set *sr;
+    slapi_pblock_get( pb, SLAPI_SEARCH_RESULT_SET, &sr );
+    if (sr) {
+        idl_iterator_decrement(&(sr->sr_current));
+    }
+    return;
+}
 
 static back_search_result_set*
 new_search_result_set(IDList *idl, int vlv, int lookthroughlimit)

+ 1 - 0
ldap/servers/slapd/back-ldbm/proto-back-ldbm.h

@@ -461,6 +461,7 @@ int ldbm_back_db_test( Slapi_PBlock *pb );
 int ldbm_back_entry_release( Slapi_PBlock *pb, void *backend_info_ptr );
 void ldbm_back_search_results_release( void **search_results );
 int ldbm_back_init( Slapi_PBlock *pb ); 
+void ldbm_back_prev_search_results( Slapi_PBlock *pb );
 
 /*
  * monitor.c

+ 6 - 0
ldap/servers/slapd/backend.c

@@ -396,6 +396,9 @@ slapi_be_getentrypoint(Slapi_Backend *be, int entrypoint, void **ret_fnptr, Slap
 	case SLAPI_PLUGIN_DB_SEARCH_RESULTS_RELEASE_FN:
 		*ret_fnptr = (void*)be->be_search_results_release;
 		break;
+	case SLAPI_PLUGIN_DB_PREV_SEARCH_RESULTS_FN:
+		*ret_fnptr = be->be_prev_search_results;
+		break;
 	case SLAPI_PLUGIN_DB_SIZE_FN:
 		*ret_fnptr = (void*)be->be_dbsize;
 		break;
@@ -504,6 +507,9 @@ slapi_be_setentrypoint(Slapi_Backend *be, int entrypoint, void *ret_fnptr, Slapi
 	case SLAPI_PLUGIN_DB_SEARCH_RESULTS_RELEASE_FN:
         be->be_search_results_release=(VFPP) ret_fnptr;
         break;
+	case SLAPI_PLUGIN_DB_PREV_SEARCH_RESULTS_FN:
+		be->be_prev_search_results = (VFP) ret_fnptr;
+		break;
 	case SLAPI_PLUGIN_DB_SIZE_FN:
         be->be_dbsize=(IFP) ret_fnptr;
         break;

+ 1 - 0
ldap/servers/slapd/backend_manager.c

@@ -139,6 +139,7 @@ be_new_internal(struct dse *pdse, const char *type, const char *name)
     be->be_database->plg_search= &dse_search;
     be->be_database->plg_next_search_entry= &dse_next_search_entry;
     be->be_database->plg_search_results_release= &dse_search_set_release;
+    be->be_database->plg_prev_search_results= &dse_prev_search_results;
     be->be_database->plg_compare= &be_plgfn_unwillingtoperform;
     be->be_database->plg_modify= &dse_modify;
     be->be_database->plg_modrdn= &be_plgfn_unwillingtoperform;

+ 13 - 0
ldap/servers/slapd/dl.c

@@ -160,6 +160,19 @@ void *dl_get_next (const DataList *dl, int *cookie)
 	return dl->elements[(*cookie)++];
 }
 
+void *dl_get_prev (const DataList *dl, int *cookie)
+{
+  PR_ASSERT (dl);
+  PR_ASSERT (cookie && *cookie > 0);
+
+  if (*cookie < 0)
+      return NULL;
+  if (*cookie > 0)
+      (*cookie)--;
+
+  return dl->elements[*cookie];
+}
+
 void *dl_replace(const DataList *dl, const void *elementOld, void *elementNew, CMPFN cmpfn, FREEFN freefn)
 {
 	int i;

+ 12 - 0
ldap/servers/slapd/dse.c

@@ -2322,6 +2322,18 @@ dse_search_set_release (void **ss)
 	dse_search_set_delete(*(dse_search_set **)ss);
 }
 
+void 
+dse_prev_search_results (Slapi_PBlock *pb)
+{
+	dse_search_set *ss;
+	slapi_pblock_get( pb, SLAPI_SEARCH_RESULT_SET, &ss );
+	if (ss) {
+		dl_get_prev (&ss->dl, &ss->current_entry);
+	}
+	return;
+
+}
+
 static void 
 dse_search_set_delete (dse_search_set *ss)
 {

+ 40 - 31
ldap/servers/slapd/opshared.c

@@ -1065,21 +1065,20 @@ iterate_with_lookahead(Slapi_PBlock *pb, Slapi_Backend *be, int send_result, int
  */
 static int
 iterate(Slapi_PBlock *pb, Slapi_Backend *be, int send_result,
-        int *pnentries, int pagesize, unsigned int *pr_stat) 
+        int *pnentries, int pagesize, unsigned int *pr_statp) 
 {
     int rc;
+    int rval = 1; /* no error, by default */
     int attrsonly;
     int done = 0;
     Slapi_Entry *e = NULL;
     char **attrs = NULL;
+    unsigned int pr_stat = 0;
 
     slapi_pblock_get(pb, SLAPI_SEARCH_ATTRS, &attrs);
     slapi_pblock_get(pb, SLAPI_SEARCH_ATTRSONLY, &attrsonly);
 
     *pnentries = 0;
-    if (pr_stat) {
-        *pr_stat = 0;
-    }
     
     while (!done) 
     {
@@ -1097,9 +1096,11 @@ iterate(Slapi_PBlock *pb, Slapi_Backend *be, int send_result,
             {
                 operation_out_of_disk_space();
             }
-            *pr_stat = PAGEDRESULTS_SEARCH_END;
+            pr_stat = PAGEDRESULTS_SEARCH_END;
             pagedresults_set_timelimit(pb->pb_conn, 0);
-            return -1;
+            rval = -1;
+            done = 1;
+            continue;
         } 
 
         slapi_pblock_get(pb, SLAPI_SEARCH_RESULT_ENTRY, &e);
@@ -1148,14 +1149,15 @@ iterate(Slapi_PBlock *pb, Slapi_Backend *be, int send_result,
                     slapi_pblock_get(pb, SLAPI_SEARCH_RESULT_ENTRY, &e);
                     if (NULL == e) {
                         /* everything is ok - don't send the result */
-                        *pr_stat = PAGEDRESULTS_SEARCH_END;
-                        return 1;
+                        pr_stat = PAGEDRESULTS_SEARCH_END;
+                        done = 1;
+                        continue;
                     }
                     gerentry = e;
                 }
                 if ( rc != LDAP_SUCCESS ) {
                     /* Send error result and 
-                           abort op if the control is critical */
+                       abort op if the control is critical */
                      LDAPDebug( LDAP_DEBUG_ANY,
                     "Failed to get effective rights for entry (%s), rc=%d\n",
                     slapi_entry_get_dn_const(e), rc, 0 );
@@ -1163,13 +1165,14 @@ iterate(Slapi_PBlock *pb, Slapi_Backend *be, int send_result,
                     slapi_ch_free ( (void**)&errbuf );
                     if (gerentry)
                     {
-                        slapi_pblock_set(pb,
-                                        SLAPI_SEARCH_RESULT_ENTRY, NULL);
+                        slapi_pblock_set(pb, SLAPI_SEARCH_RESULT_ENTRY, NULL);
                         slapi_entry_free(gerentry);
                         gerentry = e = NULL;
                     }
-                    *pr_stat = PAGEDRESULTS_SEARCH_END;
-                    return( -1 );
+                    pr_stat = PAGEDRESULTS_SEARCH_END;
+                    rval = -1;
+                    done = 1;
+                    continue;
                 }
                 slapi_ch_free ( (void**)&errbuf );
                 if (process_entry(pb, e, send_result)) 
@@ -1177,8 +1180,7 @@ iterate(Slapi_PBlock *pb, Slapi_Backend *be, int send_result,
                     /* shouldn't send this entry */
                     if (gerentry)
                     {
-                        slapi_pblock_set(pb,
-                                        SLAPI_SEARCH_RESULT_ENTRY, NULL);
+                        slapi_pblock_set(pb, SLAPI_SEARCH_RESULT_ENTRY, NULL);
                         slapi_entry_free(gerentry);
                         gerentry = e = NULL;
                     }
@@ -1190,8 +1192,7 @@ iterate(Slapi_PBlock *pb, Slapi_Backend *be, int send_result,
                  * managedsait control is on.  In either case, send
                  * the entry.
                  */
-                switch (send_ldap_search_entry(pb, e,
-                                        NULL, attrs, attrsonly)) 
+                switch (send_ldap_search_entry(pb, e, NULL, attrs, attrsonly)) 
                 {
                     case 0:        /* entry sent ok */
                         (*pnentries)++;
@@ -1199,7 +1200,7 @@ iterate(Slapi_PBlock *pb, Slapi_Backend *be, int send_result,
                         break;
                     case 1:        /* entry not sent */
                         break;
-                    case -1:    /* connection closed */
+                    case -1:       /* connection closed */
                         /*
                          * mark the operation as abandoned so the backend
                          * next entry function gets called again and has
@@ -1222,13 +1223,21 @@ iterate(Slapi_PBlock *pb, Slapi_Backend *be, int send_result,
             {
                 /* no more entries */
                 done = 1;
-                if (pr_stat) {
-                    *pr_stat = PAGEDRESULTS_SEARCH_END;
-                }
+                pr_stat = PAGEDRESULTS_SEARCH_END;
             }
         }
         else if (e)
         {
+            if (PAGEDRESULTS_PAGE_END == pr_stat)
+            {
+                /* 
+                 * read ahead -- there is at least more entry.
+                 * undo it and return the PAGE_END
+                 */
+                be->be_prev_search_results(pb);
+                done = 1;
+                continue;
+            }
             if (process_entry(pb, e, send_result)) 
             {
                 /* shouldn't  send this entry */
@@ -1248,7 +1257,7 @@ iterate(Slapi_PBlock *pb, Slapi_Backend *be, int send_result,
                     break;
                 case 1:        /* entry not sent */
                     break;
-                case -1:    /* connection closed */
+                case -1:       /* connection closed */
                     /*
                      * mark the operation as abandoned so the backend
                      * next entry function gets called again and has
@@ -1260,26 +1269,26 @@ iterate(Slapi_PBlock *pb, Slapi_Backend *be, int send_result,
             if (pagesize == *pnentries)
             { 
                 /* PAGED RESULTS: reached the pagesize */
-                done = 1;
-                if (pr_stat) {
-                    *pr_stat = PAGEDRESULTS_PAGE_END;
-                }
+                /* We don't set "done = 1" here.
+                 * We read ahead next entry to check whether there is
+                 * more entries to return or not. */
+                pr_stat = PAGEDRESULTS_PAGE_END;
             }
         }
         else
         {
             /* no more entries */
             done = 1;
-            if (pr_stat) {
-                *pr_stat = PAGEDRESULTS_SEARCH_END;
-            }
+            pr_stat = PAGEDRESULTS_SEARCH_END;
         }
     }
 
-    return 1;
+    if (pr_statp) {
+        *pr_statp = pr_stat;
+    }
+    return rval;
 }
 
-
 static int        timelimit_reslimit_handle = -1;
 static int        sizelimit_reslimit_handle = -1;
 

+ 12 - 0
ldap/servers/slapd/pblock.c

@@ -518,6 +518,12 @@ slapi_pblock_get( Slapi_PBlock *pblock, int arg, void *value )
 		}
 		(*(VFPP *)value) = pblock->pb_plugin->plg_search_results_release;
 		break;
+	case SLAPI_PLUGIN_DB_PREV_SEARCH_RESULTS_FN:
+		if ( pblock->pb_plugin->plg_type != SLAPI_PLUGIN_DATABASE ) {
+			return( -1 );
+		}
+		(*(VFP *)value) = pblock->pb_plugin->plg_prev_search_results;
+		break;
 	case SLAPI_PLUGIN_DB_COMPARE_FN:
 		if ( pblock->pb_plugin->plg_type != SLAPI_PLUGIN_DATABASE ) {
 			return( -1 );
@@ -1824,6 +1830,12 @@ slapi_pblock_set( Slapi_PBlock *pblock, int arg, void *value )
 		}
 		pblock->pb_plugin->plg_search_results_release = (VFPP) value;
 		break;
+	case SLAPI_PLUGIN_DB_PREV_SEARCH_RESULTS_FN:
+		if ( pblock->pb_plugin->plg_type != SLAPI_PLUGIN_DATABASE ) {
+			return( -1 );
+		}
+		pblock->pb_plugin->plg_prev_search_results = (VFP) value;
+		break;
 	case SLAPI_PLUGIN_DB_COMPARE_FN:
 		if ( pblock->pb_plugin->plg_type != SLAPI_PLUGIN_DATABASE ) {
 			return( -1 );

+ 1 - 1
ldap/servers/slapd/proto-slap.h

@@ -604,7 +604,7 @@ void dse_unset_dont_ever_write_dse_files(void);
 int dse_next_search_entry (Slapi_PBlock *pb);
 char *dse_read_next_entry( char *buf, char **lastp );
 void dse_search_set_release (void **ss);
-
+void dse_prev_search_results (Slapi_PBlock *pb);
 
 
 /*

+ 3 - 0
ldap/servers/slapd/slap.h

@@ -777,6 +777,7 @@ struct slapdplugin {
 			IFP	plg_un_db_next_search_entry;	/* iterate */
 	        IFP plg_un_db_next_search_entry_ext;
 			VFPP plg_un_db_search_results_release; /* PAGED RESULTS */
+			VFP plg_un_db_prev_search_results;     /* PAGED RESULTS */
 	        IFP plg_un_db_entry_release;
 			IFP	plg_un_db_compare;	  /* compare */
 			IFP	plg_un_db_modify;	  /* modify */
@@ -815,6 +816,7 @@ struct slapdplugin {
 #define plg_next_search_entry	plg_un.plg_un_db.plg_un_db_next_search_entry
 #define plg_next_search_entry_ext plg_un.plg_un_db.plg_un_db_next_search_entry_ext
 #define plg_search_results_release plg_un.plg_un_db.plg_un_db_search_results_release
+#define plg_prev_search_results plg_un.plg_un_db.plg_un_db_prev_search_results
 #define plg_entry_release       plg_un.plg_un_db.plg_un_db_entry_release
 #define plg_compare		plg_un.plg_un_db.plg_un_db_compare
 #define plg_modify		plg_un.plg_un_db.plg_un_db_modify
@@ -1069,6 +1071,7 @@ typedef struct backend {
 #define be_next_search_entry_ext be_database->plg_next_search_entry_ext
 #define be_entry_release be_database->plg_entry_release
 #define be_search_results_release be_database->plg_search_results_release
+#define be_prev_search_results be_database->plg_prev_search_results
 #define be_compare		be_database->plg_compare
 #define be_modify		be_database->plg_modify
 #define be_modrdn		be_database->plg_modrdn

+ 2 - 0
ldap/servers/slapd/slapi-private.h

@@ -451,6 +451,7 @@ void dl_add_index(DataList *dl, void *element, int index);
 void *dl_replace(const DataList *dl, const void *elementOld, void *elementNew, CMPFN cmpfn, FREEFN freefn);
 void *dl_get_first (const DataList *dl, int *cookie);
 void *dl_get_next (const DataList *dl, int *cookie);
+void *dl_get_prev (const DataList *dl, int *cookie);
 void *dl_get (const DataList *dl, const void *element, CMPFN cmpfn);
 void *dl_delete (DataList *dl, const void *element, CMPFN cmpfn, FREEFN freefn);
 int  dl_get_count (const DataList *dl);
@@ -868,6 +869,7 @@ int valuearray_find(const Slapi_Attr *a, Slapi_Value **va, const Slapi_Value *v)
 #define SLAPI_PLUGIN_DB_DBVERIFY_FN			236
 #define SLAPI_PLUGIN_DB_ADD_SCHEMA_FN		237
 #define SLAPI_PLUGIN_DB_SEARCH_RESULTS_RELEASE_FN	238
+#define SLAPI_PLUGIN_DB_PREV_SEARCH_RESULTS_FN	239
 /* database plugin-specific parameters */
 #define SLAPI_PLUGIN_DB_NO_ACL        		250
 #define SLAPI_PLUGIN_DB_RMDB_FN         	280