[SERVER-46634] LockInfo results missing a layer of hierarchy Created: 05/Mar/20  Updated: 29/Oct/23  Resolved: 10/Mar/20

Status: Closed
Project: Core Server
Component/s: Internal Code
Affects Version/s: None
Fix Version/s: 4.7.0

Type: Bug Priority: Major - P3
Reporter: Billy Donahue Assignee: Billy Donahue
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Depends
depends on SERVER-46360 Hang analyzer json logging improvement Closed
is depended on by DOCS-11516 document lockInfo command Closed
Backwards Compatibility: Fully Compatible
Operating System: ALL
Sprint: Dev Tools 2020-03-09, Dev Tools 2020-03-23
Participants:

 Description   

It's obvious we're missing a layer of hierarchy in the structure.

The group of repeating keys (resourceId,granted,pending) should instead be the keys of a subobject created for each lock in the bucket.

existing (bad) format:

     // add an element to result object:
      //     "lockInfo": [
      //         // for each nonempty bucket in LockManager, add a suboject.
      //         {
      //             "resourceId": <string>,
      //             "granted": [ {}, ... ],  // array of lock requests
      //             "pending": [ {}, ... ],  // array of lock requests
      //
      //             // this triple of repeated keys are used to represent each lock in a bucket
      //             "resourceId": <string>,
      //             "granted": [ {}, ... ],  // array of lock requests
      //             "pending": [ {}, ... ],  // array of lock requests
      //
      //             ...
      //         },
      //         ...
      //     ]



 Comments   
Comment by Billy Donahue [ 10/Mar/20 ]

Fixed by SERVER-46360

Comment by Billy Donahue [ 05/Mar/20 ]

relevant CR https://mongodbcr.appspot.com/578250001

Comment by Billy Donahue [ 05/Mar/20 ]

Upon further thought, the LockManager's buckets are an implementation detail of its hash table design, and don't need to be exported in dumps or lockInfo results.
We'll just go through the whole list of buckets looking for locks and print those as individual objects.
So the LockManager dump result will be an array of objects, each representing a lock. We will ignore bucket structure.

sample:

[
    {
	"lockAddr":"0x7fffdf0f7be0",
	"resourceId":"{6917529027641081857: Global, 1}",
	"granted":[
	    {
		"lockRequest":"0x136",
		"lockRequestAddr":"0x7fffdfdbf3a0",
		"thread":"140736797202176",
		"mode":"IS",
		"convertMode":"NONE",
		"enqueueAtFront":false,
		"compatibleFirst":false,
		"debugInfo":"{ serverStatus: 1, tcMalloc: true, sharding: false, timing: false, defaultRWConcern: false, $db: \"\" }",
		"clientInfo":{"desc":"ftdc","opid":310}}
	],
	"pending":[]
    },
    {
	"lockAddr":"0x7fffdf0f7220",
	"resourceId":"{4611686018427387905: ReplicationStateTransition, 1}",
	"granted":[
	    {
		"lockRequest":"0x136",
		"lockRequestAddr":"0x7fffdfdbf3a0",
		"thread":"140736797202176",
		"mode":"IX",
		"convertMode":"NONE",
		"enqueueAtFront":false,
		"compatibleFirst":false,
		"debugInfo":"{ serverStatus: 1, tcMalloc: true, sharding: false, timing: false, defaultRWConcern: false, $db: \"\" }",
		"clientInfo":{"desc":"ftdc","opid":310}}
	],
	"pending":[]
    }
]

Comment by Billy Donahue [ 05/Mar/20 ]

hang analyzer work overlaps with this, as they produce the same stuff with copy-pasted code. Unifying them.

Comment by Billy Donahue [ 05/Mar/20 ]

Oooh. There's another layer missing internal to the lock request objects in the "granted" and "pending" arrays.
These requests have fields like "enqueueAtFront", "mode", and "debugInfo". That's all fine. They ALSO report on Client information.
When we do a LockManager::dump(), we put all the client info into a clientInfo subobject of the lock request object, so it has its own keyspace and all the attributes there are understood to be about the client. But LockInfo handles the Client info differently and IMO incorrectly. It merges the Client info fields directly into the LockRequest object, so if you look at a LockRequest you can't tell what's client info and what's lock request info. We should fix that too and just do what dump() does, giving the client info its own subobject.

dump() (cliient info gets its own subobject)
https://github.com/mongodb/mongo/blob/3958486551573a36f7f721097721b5b9c923dee7/src/mongo/db/concurrency/lock_manager.cpp#L939

LockInfo (merge client info)
https://github.com/mongodb/mongo/blob/3958486551573a36f7f721097721b5b9c923dee7/src/mongo/db/concurrency/lock_manager.cpp#L887

Comment by Billy Donahue [ 05/Mar/20 ]

There's an object per bucket. Buckets contain locks. Locks have 3 properties each.
When we iterate over a bucket''s locks, we don't open a new object for each one. We just append 3 more properties to the bucket.
So the Bucket looks like one big object with a set of 3 keys repeated over and over again, and there's no structural delineation between locks.
The Bucket needs to be an array of locks, with each lock being an object with 3 elements.

If a bucket might have its own properties, it might also be appropriate to have a bucket be an object containing a "locks" element mapped to that array of locks. Then there's a place to attach the bucket's metadata as other elements of the bucket object.

Comment by Eric Milkie [ 05/Mar/20 ]

What layer is missing?

Generated at Thu Feb 08 05:12:01 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.