[SERVER-84117] Make Sessions uniquely owned by SessionManagers, which are uniquely owned by TransportLayers Created: 12/Dec/23  Updated: 13/Dec/23

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

Type: Improvement Priority: Major - P3
Reporter: Sara Golemon Assignee: Backlog - Service Architecture
Resolution: Unresolved Votes: 0
Labels: techdebt
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Related
Assigned Teams:
Service Arch
Sprint: Service Arch Prioritized List
Participants:

 Description   

SERVER-80769 moved several metrics (notably HelloMetrics) from ServiceContext to SessionManager, but due to loose lifecycle management of `shared_ptr<Session>` and the lack of requirement for a TransportLayer in unit tests which using replication logic, it's currently impossible to guarantee a SessionManager is available in all contexts which wish to mutate HelloMetrics.

Resolve this by tightening up transport object ownership, particularly around the Session objects. This will also allow removing the dummy "transportlessHelloMetrics" container for when (during unit tests) Sessions are unassociated to SessionManagers.



 Comments   
Comment by Patrick Freed [ 12/Dec/23 ]

The motivation for this would be that a simpler model would make it easier to know the lifetimes different structures in the ingress path, and that should reduce the frequency of memory usage bugs/BFs and in general make it easier to write safe ingress path code. While we expect the code changes involved defining an ownership model would not necessarily take a long time, it will require design and consensus as to what the right ownership model would be and how much from the current one we should depart from, since there are lots of directions this could go in. This is related to and likely synergizes with PM-2896, though this can potentially be done independently of it.

I think the HelloMetrics issue is related but possibly separate, since that could be fixed in the unit tests without us changing the ownership model. Once we decide upon a model, we should revisit whether it will address the metrics issue or if a separate ticket needs to be filed about updating the unit tests (specifically AlwaysDecrementNumAwaitingTopologyChangesOnErrorMongoD and AlwaysDecrementNumAwaitingTopologyChangesOnErrorMongoS).

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