Tag search optimization

askbot/doc/source/changelog.rst

@@ -3,6 +3,7 @@ Changes in Askbot
 
 Development (not yet released)
 ------------------------------
+* Optimized multi-tag question search: replaced per-tag JOINs with a single subquery.
 * Added per subnet rate-limiting feature that allows
   limiting three types of requests: get requests,
   post requests from watched users

askbot/models/question.py

@@ -6,7 +6,7 @@
 from copy import copy
 from django.conf import settings as django_settings
 from django.db import models
-from django.db.models import F, Q
+from django.db.models import Count, F, Q
 from django.contrib.auth.models import User
 from django.contrib.contenttypes.models import ContentType
 from django.core import cache  # import cache, not from cache import cache, to be able to monkey-patch cache.cache in test cases
@@ -358,10 +358,29 @@ def run_advanced_search(self, request_user, search_state):
             else:
                 meta_data['non_existing_tags'] = list()
 
-            # construct filter for the tag search
-            for tag in tags:
-                # Tags or AND-ed here, not OR-ed (i.e. we fetch only threads with all tags)
-                qs = qs.filter(tags__name=tag)
+            # AND across tags: fetch only threads tagged with every requested tag.
+            # The natural ORM expression — one .filter(tags__name=tag) per tag —
+            # produces a JOIN through the M2M for every tag, which scales
+            # superlinearly and degrades to multi-second response times for
+            # 6-8 tag queries even on small datasets. The subquery + HAVING
+            # COUNT form is the relational idiom for set intersection.
+            # Deduplicate: unified_tags() concatenates query_tags + tags,
+            # so the same tag can appear twice; an inflated len() would
+            # never match Count(distinct=True). Empty list means all
+            # requested tags were unknown, in which case the search is
+            # unfiltered by tags (the names are reported via meta_data).
+            tags = list(set(tags))
+            if tags:
+                ThreadTagModel = self.model.tags.through
+                matching_thread_ids = (
+                    ThreadTagModel.objects
+                    .filter(tag__name__in=tags)
+                    .values('thread_id')
+                    .annotate(matched=Count('tag_id', distinct=True))
+                    .filter(matched=len(tags))
+                    .values_list('thread_id', flat=True)
+                )
+                qs = qs.filter(id__in=matching_thread_ids)
         else:
             meta_data['non_existing_tags'] = list()
 

askbot/tests/test_post_model.py

@@ -239,10 +239,49 @@ def test_run_adv_search_ANDing_tags(self):
         qs, meta_data = Thread.objects.run_advanced_search(request_user=self.user, search_state=ss.add_tag('tag1').add_tag('tag3').add_tag('tag6'))
         self.assertEqual(1, qs.count())
 
+        # 4+ tag AND: exercises the subquery+HAVING COUNT path. q4 is the
+        # only thread tagged with all six tags so any 4-6 tag subset matches
+        # only q4.
+        qs, meta_data = Thread.objects.run_advanced_search(request_user=self.user, search_state=ss.add_tag('tag1').add_tag('tag2').add_tag('tag3').add_tag('tag6'))
+        self.assertEqual(1, qs.count())
+        self.assertEqual(self.q4.thread_id, qs[0].id)
+
+        qs, meta_data = Thread.objects.run_advanced_search(request_user=self.user, search_state=ss.add_tag('tag1').add_tag('tag2').add_tag('tag3').add_tag('tag4').add_tag('tag5').add_tag('tag6'))
+        self.assertEqual(1, qs.count())
+        self.assertEqual(self.q4.thread_id, qs[0].id)
+
         ss = SearchState(scope=None, sort=None, query="#tag3", tags='tag1, tag6', author=None, page=None, user_logged_in=None)
         qs, meta_data = Thread.objects.run_advanced_search(request_user=self.user, search_state=ss)
         self.assertEqual(1, qs.count())
 
+    @with_settings(TAG_SEARCH_INPUT_ENABLED=False)
+    def test_run_adv_search_ANDing_tags_duplicate_across_query_and_selector(self):
+        # unified_tags() concatenates query_tags + tags, so the same tag in
+        # both the #tag query syntax and the selector produces a duplicate.
+        # The result set must still be the AND over the *distinct* tags.
+        ss = SearchState(scope=None, sort=None, query="#tag3",
+                         tags='tag1, tag3', author=None, page=None,
+                         user_logged_in=None)
+        qs, meta_data = Thread.objects.run_advanced_search(
+            request_user=self.user, search_state=ss)
+        # q1 (tag1,tag2,tag3) and q4 (tag1..tag6) both carry tag1 AND tag3
+        self.assertEqual(2, qs.count())
+        self.assertEqual({self.q1.thread_id, self.q4.thread_id},
+                         {t.id for t in qs})
+
+    @with_settings(TAG_SEARCH_INPUT_ENABLED=True)
+    def test_run_adv_search_ANDing_tags_all_nonexistent(self):
+        # When every requested tag is unknown, the search should not narrow
+        # the result set; it should return all visible threads and report
+        # the missing tags via meta_data.
+        ss = SearchState.get_empty()
+        ss = ss.add_tag('nope-one').add_tag('nope-two')
+        qs, meta_data = Thread.objects.run_advanced_search(
+            request_user=self.user, search_state=ss)
+        self.assertEqual(4, qs.count())
+        self.assertEqual({'nope-one', 'nope-two'},
+                         set(meta_data['non_existing_tags']))
+
     def test_run_adv_search_query_author(self):
         ss = SearchState(scope=None, sort=None, query="@user", tags=None, author=None, page=None, user_logged_in=None)
         qs, meta_data = Thread.objects.run_advanced_search(request_user=self.user, search_state=ss)

scripts/perf_tag_search.py

@@ -0,0 +1,134 @@
+"""One-off perf script for the tag-intersection query.
+
+Picks a real thread that carries the largest tag set in the database, then
+issues GETs to /questions/tags:t1,t2,.../ for increasing tag counts so the
+intersection always returns at least one question. Times each request with
+warm-up + repeats so the numbers are stable enough to compare branches.
+
+Usage:
+    cd askbot_site
+    ../env-md/bin/python ../perf_tag_search.py --base-url http://127.0.0.1:8000
+
+The dev server must already be running (manage.py runserver, gunicorn, etc.)
+on the URL passed in. Run the script once before applying the optimization
+and once after; compare the columns.
+"""
+import argparse
+import os
+import statistics
+import sys
+import time
+import urllib.error
+import urllib.parse
+import urllib.request
+
+import django
+
+
+def setup_django():
+    os.environ.setdefault('DJANGO_SETTINGS_MODULE', 'askbot_site.settings')
+    if os.getcwd() not in sys.path:
+        sys.path.insert(0, os.getcwd())
+    django.setup()
+
+
+def pick_tag_set(min_count, max_count):
+    """Return a list of tag names that all co-occur on the same thread.
+
+    Uses the thread carrying the most tags so the intersection is guaranteed
+    non-empty for any prefix of the returned list.
+    """
+    from askbot.models import Thread
+    from django.db.models import Count
+
+    threads = (
+        Thread.objects
+        .annotate(n_tags=Count('tags'))
+        .filter(n_tags__gte=min_count)
+        .order_by('-n_tags')
+    )
+    thread = threads.first()
+    if thread is None:
+        raise SystemExit(
+            f'No thread has at least {min_count} tags - lower --min-tags or '
+            'load a larger dataset.'
+        )
+    names = list(thread.tags.values_list('name', flat=True))
+    if max_count:
+        names = names[:max_count]
+    print(f'Using thread id={thread.id} with {len(names)} tags: {names}')
+    return names
+
+
+def build_url(base_url, tags):
+    encoded = urllib.parse.quote(','.join(tags), safe=',+.-_')
+    return f'{base_url.rstrip("/")}/questions/tags:{encoded}/'
+
+
+def time_request(url, timeout):
+    start = time.perf_counter()
+    with urllib.request.urlopen(url, timeout=timeout) as resp:
+        body = resp.read()
+        status = resp.status
+    elapsed = time.perf_counter() - start
+    return elapsed, status, len(body)
+
+
+def run(base_url, tags, repeats, warmups, timeout, sleep):
+    header = f'{"N tags":>6} | {"min ms":>8} | {"med ms":>8} | {"max ms":>8} | {"bytes":>8} | status'
+    print(header)
+    print('-' * len(header))
+    for n in range(1, len(tags) + 1):
+        url = build_url(base_url, tags[:n])
+        last_status = None
+        last_size = None
+        for _ in range(warmups):
+            _, last_status, last_size = time_request(url, timeout)
+            if sleep:
+                time.sleep(sleep)
+        samples = []
+        for _ in range(repeats):
+            elapsed, last_status, last_size = time_request(url, timeout)
+            samples.append(elapsed * 1000)
+            if sleep:
+                time.sleep(sleep)
+        print(
+            f'{n:>6} | {min(samples):>8.1f} | {statistics.median(samples):>8.1f} | '
+            f'{max(samples):>8.1f} | {last_size:>8} | {last_status}'
+        )
+        print(f'         url: {url}')
+
+
+def main():
+    parser = argparse.ArgumentParser(description=__doc__)
+    parser.add_argument('--base-url', default='http://127.0.0.1:8000',
+                        help='Base URL of the running askbot site.')
+    parser.add_argument('--min-tags', type=int, default=2,
+                        help='Skip threads that carry fewer tags than this.')
+    parser.add_argument('--max-tags', type=int, default=8,
+                        help='Cap the tag list length (0 = no cap).')
+    parser.add_argument('--repeats', type=int, default=5,
+                        help='Timed requests per tag count.')
+    parser.add_argument('--warmups', type=int, default=1,
+                        help='Untimed requests per tag count before timing.')
+    parser.add_argument('--timeout', type=float, default=60.0,
+                        help='Per-request timeout in seconds.')
+    parser.add_argument('--sleep', type=float, default=1.5,
+                        help='Pause between requests to avoid the rate limiter.')
+    args = parser.parse_args()
+
+    setup_django()
+    tags = pick_tag_set(args.min_tags, args.max_tags)
+    print(f'Base URL: {args.base_url}')
+    print(f'Repeats: {args.repeats}, warmups: {args.warmups}')
+    print()
+    try:
+        run(args.base_url, tags, args.repeats, args.warmups, args.timeout,
+            args.sleep)
+    except urllib.error.URLError as exc:
+        print(f'Request failed - is the dev server running? {exc}', file=sys.stderr)
+        sys.exit(1)
+
+
+if __name__ == '__main__':
+    main()