[SERVER-38034] OperationContextSessionMongod constructor should just take required parameters, not entire OperationSessionInfoFromClient Created: 08/Nov/18  Updated: 27/Oct/23  Resolved: 12/Nov/18

Status: Closed
Project: Core Server
Component/s: Sharding
Affects Version/s: None
Fix Version/s: None

Type: Task Priority: Major - P3
Reporter: Esha Maharishi (Inactive) Assignee: Esha Maharishi (Inactive)
Resolution: Gone away Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Related
related to SERVER-38095 Remove unused OperationSessionInfoFro... Closed
Participants:

 Description   

because the coordinateCommit command needs to check out the session from inside the command body in order to recover a decision from the local participant, and an OperationSessionInfoFromClient cannot be constructed inside the coordinateCommit command.



 Comments   
Comment by Esha Maharishi (Inactive) [ 12/Nov/18 ]

Closing in lieu of SERVER-38095, since after SERVER-37179, OperationContextSessionMongod no longer uses the OperationSessionInfoFromClient parameter at all.

Comment by Esha Maharishi (Inactive) [ 12/Nov/18 ]

siyuan.zhou, I dug a little deeper and found that:

1) Checking out a session uses the LogicalSessionId on the OperationContext

2) the LogicalSessionId on the OperationContext is set in initializeOperationSessionInfo through the makeLogicalSessionId helper, which sets both the lsid and uid.

So, changing OperationContextSessionMongod's constructor not to take the whole OperationSessionInfoFromClient should make no difference on the LogicalSessionId used to check out the session.

CC kaloian.manassiev

Comment by Kaloian Manassiev [ 12/Nov/18 ]

This doesn't make sense. The UID is part of the LogicalSessionId, which is returned by ScopedCheckedOutSession::get(opCtx)->getSessionId().

Are you saying that the returned LSID doesn't contain UID (because that would be a bug)?

Comment by Siyuan Zhou [ 09/Nov/18 ]

kaloian.manassiev, the reason why Esha wants a new structure is that we only have the session id on operation context without the uid when calling OperationContextSessionMongod in sharding.

Comment by Kaloian Manassiev [ 09/Nov/18 ]

I'm not sure whether the uid field of logical session id is required or not though.

The uid of a logical session is an integral part of the LSID and we have had bugs previously because of code, which ignores it - SERVER-37657. siyuan.zhou, in what context are you asking about this?

Comment by Esha Maharishi (Inactive) [ 08/Nov/18 ]

judah.schvimer, OperationContextSessionMongod only accesses the 'autocommit', 'startTransaction' and 'coordinator' fields of OperationSessionInfoFromClient. I think passing OperationSessionInfoFromClient was not only sufficient but too much, and it makes checking out the session in places other than the service entry point more difficult because the entire OperationSessionInfoFromClient may not be available.

Comment by Siyuan Zhou [ 08/Nov/18 ]

Talked with Esha yesterday, this ticket is the conclusion from that discussion. I'm not sure whether the uid field of logical session id is required or not though. I guess sharding team has more context on logical session.

Comment by Judah Schvimer [ 08/Nov/18 ]

siyuan.zhou, is this essentially undoing https://github.com/mongodb/mongo/commit/248601a6473fc7364e5d790a357acbace2a42f7a, is that ok?

Generated at Thu Feb 08 04:47:46 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.