[SERVER-73307] Make `CurOpStack` always store a pointer to its `OpCtx` Created: 25/Jan/23  Updated: 29/Oct/23  Resolved: 17/Feb/23

Status: Closed
Project: Core Server
Component/s: Internal Code
Affects Version/s: None
Fix Version/s: 7.0.0-rc0

Type: Improvement Priority: Major - P3
Reporter: Amirsaman Memaripour Assignee: Patrick Freed
Resolution: Fixed Votes: 0
Labels: neweng
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Problem/Incident
Related
is related to SERVER-70032 Collect and report operation CPU time... Closed
Assigned Teams:
Service Arch
Backwards Compatibility: Fully Compatible
Sprint: Service Arch 2023-02-06, Service Arch 2023-02-20
Participants:
Linked BF Score: 155

 Description   

At the time of writing this ticket, CurOp is defined as a decoration on OperationContext, through CurOpStack which may contain a stack of CurOp objects:

/**
 * This type decorates a Client object with a stack of active CurOp objects.
 *
 * It encapsulates the nesting logic for curops attached to a Client, along with
 * the notion that there is always a root CurOp attached to a Client.
 *
 * The stack itself is represented in the _parent pointers of the CurOp class.
 */
class CurOp::CurOpStack {
...
private:
    OperationContext* _opCtx = nullptr;
 
    // Top of the stack of CurOps for a Client.
    CurOp* _top = nullptr;
 
    // The bottom-most CurOp for a client.
    const CurOp _base;
};

CurOpStack holds a pointer to OperationContext, however, this pointer is not initialized until after the first CurOp is pushed to the stack:

void CurOpStack::push(OperationContext* opCtx, CurOp* curOp) {
    invariant(opCtx);
    if (_opCtx) {
        invariant(_opCtx == opCtx);
    } else {
        _opCtx = opCtx;
    }
    stdx::lock_guard<Client> lk(*_opCtx->getClient());
    push_nolock(curOp);
}

We can use constructor actions (similar to the following example) to initialize the _opCtx member on CurOpStack during the construction of OperationContext, making sure _opCtx is always initialized and obviating the need to provide opCtx as an argument to CurOpStack and CurOp member functions. This also helps with simplifying the changes introduced by SERVER-70032.

class CurOpClientObserver final : public ServiceContext::ClientObserver {
    void onCreateClient(Client*) {}
    void onDestroyClient(Client*) {}
    void onCreateOperationContext(OperationContext* opCtx) {
        _curopStack(opCtx).setOpCtx(opCtx);
    }
    void onDestroyOperationContext(OperationContext*) {}
};
 
ServiceContext::ConstructorActionRegisterer registerCurOpClientObserver{"CurOpClientObserver", [](ServiceContext* service) {
    service->registerClientObserver(std::make_unique<CurOpClientObserver>());
}};



 Comments   
Comment by Githook User [ 17/Feb/23 ]

Author:

{'name': 'Patrick Freed', 'email': 'patrick.freed@mongodb.com', 'username': 'patrickfreed'}

Message: SERVER-73307 Ensure CurOpStack always has access to its OperationContext (#10536)

This also ensures base lock stats are properly subtracted out when getting the lock stats for a given sub operation (fixes SERVER-73571).
Branch: master
https://github.com/mongodb/mongo/commit/b221af175a73b66a640a92fb364b8cc1a759e5e3

Comment by Githook User [ 15/Feb/23 ]

Author:

{'name': 'liubov.molchanova', 'email': 'liubov.molchanova@mongodb.com', 'username': 'liubov-molchanova'}

Message: Revert "SERVER-73307 Ensure CurOpStack always has access to its OperationContext (#10536)"

This reverts commit ff7fcd42e84317e2a34e0b413ee4cc1d9be66884.
Branch: master
https://github.com/mongodb/mongo/commit/8f36c90d997708507e1d73c4200d79a92619f422

Comment by Githook User [ 15/Feb/23 ]

Author:

{'name': 'Patrick Freed', 'email': 'patrick.freed@mongodb.com', 'username': 'patrickfreed'}

Message: SERVER-73307 Ensure CurOpStack always has access to its OperationContext (#10536)

This also ensures base lock stats are properly subtracted out when getting the lock stats for a given sub operation (fixes SERVER-73571).
Branch: master
https://github.com/mongodb/mongo/commit/ff7fcd42e84317e2a34e0b413ee4cc1d9be66884

Comment by Amirsaman Memaripour [ 26/Jan/23 ]

Another approach to get the opCtx pointer is through the owner interface:

OperationContext* CurOp::CurOpStack::opCtx() {
    return CurOp::_curopStack.owner(*this);
}

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