Repository: girder/girder
Issue: Search: don't try to sort by text score

It is not a real index and our unlimited search query attempts to build an index on the entire result set, which obviously does not scale.


Changed files:
girder/models/item.py, girder/constants.py, girder/models/model_base.py, tests/cases/search_test.py

diff --git a/girder/constants.py b/girder/constants.py
index b00ac2fd4..e869e8886 100644
--- a/girder/constants.py
+++ b/girder/constants.py
@@ -27,6 +27,11 @@ ROOT_PLUGINS_PACKAGE = 'girder.plugins'
 MAX_LOG_SIZE = 1024 * 1024 * 10  # Size in bytes before logs are rotated.
 LOG_BACKUP_COUNT = 5
 
+# Threshold below which text search results will be sorted by their text score.
+# Setting this too high causes mongodb to use too many resources for searches
+# that yield lots of results.
+TEXT_SCORE_SORT_MAX = 200
+
 
 class TerminalColor(object):
     """
diff --git a/girder/models/item.py b/girder/models/item.py
index bfe18cdee..c5fc33fc4 100644
--- a/girder/models/item.py
+++ b/girder/models/item.py
@@ -201,8 +201,8 @@ class Item(Model):
             filters = {}
 
         # get the non-filtered search result from Model.textSearch
-        results = Model.textSearch(self, query=query, limit=0, sort=sort,
-                                   filters=filters)
+        cursor = Model.textSearch(self, query=query, limit=0, sort=sort,
+                                  filters=filters)
 
         # list where we will store the filtered results
         filtered = []
@@ -212,7 +212,7 @@ class Item(Model):
 
         # loop through all results in the non-filtered list
         i = 0
-        for result in results:
+        for result in cursor:
             # check if the folderId is cached
             folderId = result['folderId']
 
@@ -234,6 +234,7 @@ class Item(Model):
                 if len(filtered) >= limit:
                     break
 
+        cursor.close()
         return filtered
 
     def hasAccess(self, item, user=None, level=AccessType.READ):
diff --git a/girder/models/model_base.py b/girder/models/model_base.py
index 951be23c0..56f6b1319 100644
--- a/girder/models/model_base.py
+++ b/girder/models/model_base.py
@@ -21,7 +21,7 @@ import pymongo
 
 from bson.objectid import ObjectId
 from girder import events
-from girder.constants import AccessType, TerminalColor
+from girder.constants import AccessType, TerminalColor, TEXT_SCORE_SORT_MAX
 from girder.utility.model_importer import ModelImporter
 from girder.models import getDbConfig, getDbConnection
 
@@ -146,23 +146,27 @@ class Model(ModelImporter):
         :type query: str
         :param filters: Any additional query operators to apply.
         :type filters: dict
+        :returns: A pymongo cursor. It is left to the caller to build the
+        results from the cursor.
         """
         if not filters:
             filters = {}
+        if not fields:
+            fields = {}
 
-        filters['$text'] = {
-            '$search': query
-        }
+        fields['_textScore'] = {'$meta': 'textScore'}
+        filters['$text'] = {'$search': query}
 
-        if sort is None:
-            if fields is None:
-                fields = {'_textScore': {'$meta': 'textScore'}}
-            else:
-                fields['_textScore'] = {'$meta': 'textScore'}
-            sort = [('_textScore', {'$meta': 'textScore'})]
+        cursor = self.find(filters, offset=offset, limit=limit,
+                           sort=sort, fields=fields)
 
-        return [r for r in self.find(filters, offset=offset, limit=limit,
-                                     sort=sort, fields=fields)]
+        # Sort by meta text score, but only if result count is below a certain
+        # threshold. The text score is not a real index, so we cannot always
+        # sort by it if there is a high number of matching documents.
+        if cursor.count() < TEXT_SCORE_SORT_MAX and sort is None:
+            cursor.sort([('_textScore', {'$meta': 'textScore'})])
+
+        return cursor
 
     def save(self, document, validate=True, triggerEvents=True):
         """
@@ -300,6 +304,9 @@ class Model(ModelImporter):
             if field in doc:
                 out[field] = doc[field]
 
+        if '_textScore' in doc:
+            out['_textScore'] = doc['_textScore']
+
         return out
 
 
@@ -669,11 +676,11 @@ class AccessControlledModel(Model):
         if not filters:
             filters = {}
 
-        results = Model.textSearch(
+        cursor = Model.textSearch(
             self, query=query, filters=filters, limit=0, sort=sort,
             fields=fields)
         return [r for r in self.filterResultsByPermission(
-            results, user=user, level=AccessType.READ, limit=limit,
+            cursor, user=user, level=AccessType.READ, limit=limit,
             offset=offset)]
 
 
diff --git a/tests/cases/search_test.py b/tests/cases/search_test.py
index f64807246..ffce5746f 100644
--- a/tests/cases/search_test.py
+++ b/tests/cases/search_test.py
@@ -184,6 +184,8 @@ class SearchTestCase(base.TestCase):
             '_id': str(coll1['_id']),
             'name': coll1['name']
         }, resp.json['collection'][1])
+        self.assertTrue(resp.json['collection'][0]['_textScore'] >
+                        resp.json['collection'][1]['_textScore'])
 
         # Exercise user search by login
         resp = self.request(path='/resource/search', params={