32

The planned work on Review Queues includes Backend Architecture changes (listed on our Public Platform roadmap for February). As this project is now concluded, I would like to report to the Community about what was accomplished.

This is a pretty technical post. Here is a summary for those who are less familiar with the topics discussed below:

  • Discussed the shortcomings of the current backend architecture for populating Review Queue tasks and showing them to users. The biggest one is related to technical debt and poor documentation (rather than performance, as we initially thought).

  • Explored the possibility of a major rewrite of the architecture, but ultimately rejected this as it would not address our main issues.

  • Work in the end consisted of full audits of all code and queries, addressing shortcomings in documentation and comments as well as inefficiencies that had crept in over the years. Benchmarked performance before and after.

  • The work done here is completely on the backend. There are no changes to the UI nor to the user experience (aside from review tasks loading faster).

How things work

First, some background on how Review Queues are populated:

  • ReviewTask is the object in the database that records all interactions with an item to be reviewed in a specific queue.

    • A ReviewTask is related to a PostId or a SuggestedEditId as well as a specific review queue.

    • One ReviewTask can be active at any time for a specific post on a specific review queue (or more than one queue, depending on the specific queue).

  • Review Tasks are created during a Sync function that is run on a background thread every five minutes (per review queue, per site).

    • An SQL query is generated that returns Id values for all posts that should be eligible for review, based on the rules of the queue.

    • This query is used to add new Review Tasks (for Posts with Ids that are returned by the query where there is not an active ReviewTask for the queue) and to invalidate active Review Tasks (for posts with active ReviewTask items that are no longer on the eligible list).

    • For the Triage and Suggested Edit queues, new Review Tasks are created directly based on specific actions, and not through the scheduled sync.

When a user hits the /review/next-task route for a specific queue (by clicking on a queue name from the header or from within the reviews section), we return the details of a Review Task that the user is eligible to review.

  1. The ChooseNextReviewTaskCandidateForUser query is run, which creates a very complicated SQL query. This query is composed of a number of subqueries, which are themselves generated across a number of functions. This query does the following:

    1. Select 1000 items from the active Review Tasks for the queue, excluding tasks that the current user is disqualified from reviewing due to actions that they have performed on the site. Criteria vary per queue (for example, you cannot review an item in the Close Votes queue if you flagged it to be closed).

    2. Exclude posts where the current user is the PostOwner or tasks that the current user has already reviewed.

    3. When the query is being filtered, only 1 item is returned (filtering can be expensive).

  2. Filter out items that are currently being actively reviewed by other users (from a list stored in cache)

  3. Randomly select an item to be reviewed:

    1. For the Close Votes and Reopen queues (where multiple reviews are needed to close out an item), give preference to items that have the most reviews performed already.

    2. If the user has preferred or ignored tags on /questions, give preference to posts that match the preferred tags (and don't have the ignored tags).

    3. Make sure that nothing has changed with the item since the last Sync that would disqualify or invalidate it.

Meme image of complicated cockpit full of buttons, with text: "Review Queues - It's That Simple"

Architecture evaluation

The current architecture has served us well over the years, but it has a number of areas that need improvement.

  1. The code is very complicated and brittle. The queries defined above (the details above are high level, and exclude many details and minutiae) are built across multiple functions. They break easily when making changes, and are hard to test.

  2. There was poor documentation around the way that the pieces fit together, making it a challenge for a developer to know the correct and most efficient place to make changes to code to effect the desired result in the queue.

  3. The queries can have long execution times (for both Sync and ChooseNextCandidate), and require attention to keep performance in check, especially when making changes.

  4. Lack of benchmarks around performance and review activity led to challenges in decision-making and evaluation of queue health.

There are more plans for Review Queues in the future that we need our architecture to be able to support, potentially including:

  • Changes in workflow to the current queues and the additions of new queues

  • More advanced filtering

  • Showing users lists of items to select from (instead of only giving one item at a time)

  • Better mechanisms for recommending items more relevant to the interests and expertise of the current user

We took a long time thinking and discussing ways forward here and strongly considered (including a lot of discovery and proof of concept work) rewriting the backend system. The goal would have been to move away from the scheduled Sync function controlling Review Task population, and instead to create a new system based on evaluating post events in real time to determine which items should be in a review queue. This could also have included making things more cache-based, enabling potential simplification of the next-task selection process.

In the end however, we decided to stick with the current architecture. It turns out that when you have a system with many inputs and complicated rules that need to be enforced in a performant way, rewriting the system while keeping the same rules will not make things less complicated - it will just move the complicated bits to other areas. We could potentially greatly simplify the generation and performance of the SQL queries that are currently used. However, this would come at the cost of creating a new framework that would spread out the current SQL complexity to different areas governing the population and maintenance of up-to-date metadata in cache for all items in a given queue (and the ability to reload the cache at any time). After benchmarking the actual performance of the queries today, we found out that the execution time and SQL loads from the queries are not as bad as we had thought (and feared).

So what did we do?

The plan shifted away from a full refresh of the architecture, and on towards actions aimed at addressing technical debt around the main issue we faced: the system was so complicated and poorly documented that we were scared to touch it.

Here is what we have done:

  1. Fully benchmark execution times for next-task queries for all review queues

  2. Add event tracking to record details around review task loading (including how often users run filters, something that we had not been tracking in an easily reportable way)

  3. Perform a full audit on the code governing the review processes:

    1. Add missing documentation in the code for every relevant function.

    2. Reduce code complexity when possible, remove or consolidate unused functions and parameters, etc.

  4. Release new internal documentation (on our internal Stack Overflow for Teams instance) that gives an introduction for new developers to the overall code structure, as well as guidance on where to make different types of changes, and pitfalls to be aware of when making changes.

  5. Audit all the SQL that is run by the system

    1. Add comments in long queries where relevant

    2. Remove redundancies or reduce the complexity in the SQL that was generated (was able to make changes in nearly every queue)

    3. In one case (affecting Close Votes and Reopen), add a new column to ReviewTask to denormalize data, improving next-task performance significantly by avoiding an expensive subquery that had been part of next-task.

  6. Add a new dev route to allow for easier debugging of the Sync process and evaluation of the SQL that is generated and run.

All code changes are now live, and we have realized performance improvements for new-task on every queue. Below are some stats, including general stats for queues, the percentage of task loads that are filtered, baseline SQL run times for next-task on each queue (default and filtered), and final SQL run times for each queue (default and filtered, including percentage improvement).

Queue % Overall % FLT BL Default BL FLT Final: Default Final: FLT
First Post 25.70% 0.93% 15.7 4.0 14.2 (9.55%) 3.8 (5.0%)
Suggested Edit 22.52% 0.57% 20.7 341.5 18.2 (12.25%) 4.0 (98.83%)
Triage 16.94% N/A 3.7 N/A 3.2 (13.51%) N/A
Close Vote 10.45% 11.63% 39.2 48.4 13.3 (66.07%) 22.5 (53.51%)
Low Quality 8.86% 0.26% 11.6 8.8 6.4 (44.83%) 3.7 (57.95%)
Late Answer 8.75% 0.02% 4.6 3.3 4.0 (13.04%) 2.8 (15.15%)
Reopen Votes 6.63% 0.53% 36.7 21.2 24.6 (32.97%) 17.9 (15.57%)
Helper 0.15% N/A 3.1 N/A 2.5 (19.35%) N/A

All times are in milliseconds, stats run on Stack Overflow. “BL” = “Baseline” and “FLT” = “Filtered”. Percentages in the Final columns are the percent improvement

Some observations/errata:

  • SQL execution time was measured on Stack Overflow using MiniProfiler. I averaged at least five executions per queue to get these times. As system load can affect execution time, I tried to run my before/after benchmarks around the same time of the day.

  • We have improvements in execution time for every queue. However, we are talking about milliseconds here - the queries were for the most part fast enough that this will not be noticeable to the end-user.

  • The filtered query is cheaper than the default/non-filtered stat for some of the queues because the default returns 1000 items (from which the item to be reviewed is them selected) and filtered returns only one item (we don't return more, because filtering is expensive, though with these numbers to work with, we can explore more options in this area in the future).

  • It is noteworthy that Close Votes is the only queue with significant filtering: 11.63% of tasks loaded in Close Votes are filtered. Every other queue with filtering is below 1%. It will be interesting to see if and how these numbers change when our upcoming UI refresh makes the filtering options more accessible.

Thanks for reading this far. We are happy to answer relevant questions below.

3
  • 1
    "Add event tracking to record details around review task loading (including how often users run filters, something that we had not been tracking in an easily reportable way)" Someone somewhere hea... reads me.
    – Braiam
    Commented Feb 8, 2021 at 19:59
  • 2
    "Filter out items that are currently being actively reviewed by other users (from a list stored in cache)" What's the lifetime of this cache, out of curiosity?
    – TylerH
    Commented Feb 8, 2021 at 20:06
  • 4
    @TylerH cache timeout per item is 3 minutes (if you finish the review item before then, it gets removed from the cache right away)
    – Yaakov Ellis StaffMod
    Commented Feb 8, 2021 at 20:12

0

You must log in to answer this question.

Browse other questions tagged .