[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: |
|
||||||||
| 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 |
| 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. |
| 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 ] |
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 - |
| 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? |