[SERVER-78749] Avoid applying hmac while holding partition lock Created: 06/Jul/23  Updated: 29/Oct/23  Resolved: 25/Jul/23

Status: Closed
Project: Core Server
Component/s: None
Affects Version/s: None
Fix Version/s: 7.1.0-rc0

Type: Improvement Priority: Major - P3
Reporter: Charlie Swanson Assignee: Naafiyan Ahmed (Inactive)
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Attachments: PNG File Screenshot 2023-07-06 at 10.48.46 AM.png    
Issue Links:
Depends
is depended on by SERVER-85064 Tracking: M0 performance improvement ... Closed
Backwards Compatibility: Fully Compatible
Sprint: QO 2023-07-24, QO 2023-08-07
Participants:

 Description   

I ran an experiment or two to see what the impact was on regular traffic when $queryStats is run and it seems we have an opportunity to reduce the impact (aside from already-filed SERVER-77567).

I noticed that we acquire the lock for a single partition at a time, but then we go through and compute the hmac-transformed key for each entry still while holding the lock. This should not be necessary, but will take some care to get right.

Here is where we do the hmac-intensive work: https://github.com/10gen/mongo/blob/505d34325477993864f4c8ead7878dda662ed9d5/src/mongo/db/pipeline/document_source_query_stats.cpp#L177

You can see it is within scope of the 'onePartition' locked object, preventing any progress for queries which would need to record metrics in that partition.

I think what should be done is:

  • Reduce the critical section to only acquire the things we need to do our computations. Namely, rather than producing a vector of Document result objects during the critical section, only collect shared_ptr<QueryStatsEntry>}}s and {{metrics->toBSON() while holding the lock.
  • Make sure we don't have race conditions by:
    • using shared_ptr to ensure the QueryStatsEntrys do not get evicted and freed out from under us
    • Make QueryStatsEntry::keyGenerator be const to prove/enforce that there will not be any data races between ongoing metric recording and us calling 'computeQueryStatsKey()' - which we will now do outside of the lock.


 Comments   
Comment by Githook User [ 25/Jul/23 ]

Author:

{'name': 'Naafiyan Ahmed', 'email': 'naafiyan.ahmed@mongodb.com', 'username': 'naafiyan'}

Message: SERVER-78749 removed key computation from CS
Branch: master
https://github.com/mongodb/mongo/commit/f9e93157670afb96a80295ef6653ed8f8a42de1c

Comment by Naafiyan Ahmed (Inactive) [ 13/Jul/23 ]

Ran some genny tests on master build vs my build and here are the results

Generated at Thu Feb 08 06:39:09 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.