[SERVER-37241] Add testing to verify proper expiration of sessions in the sessions collection Created: 21/Sep/18  Updated: 29/Oct/23  Resolved: 31/Oct/18

Status: Closed
Project: Core Server
Component/s: None
Affects Version/s: None
Fix Version/s: 3.6.10, 4.0.6, 4.1.5

Type: Task Priority: Major - P3
Reporter: Blake Oler Assignee: Blake Oler
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Backports
Backwards Compatibility: Fully Compatible
Backport Requested:
v4.0, v3.6
Sprint: Sharding 2018-10-22, Sharding 2018-11-05
Participants:

 Description   

A lack of testing exists around verifying that sessions do indeed expire from the sessions collection when their lifetime is up.

The session expiry time is a server parameter, measured in minutes. We may want to consider increasing the granularity to seconds so we don't make tests wait an abnormal amount of time.

Proposed Fix

  1. Change the granularity of the logical session timeout to seconds instead of minutes. Since the drivers rely on the parameter "localLogicalSessionTimeoutMinutes" to exist in calls to isMaster(), I can take the following approaches:
    1. Create a duplicate parameter "logicalSessionTimeoutSeconds" which will be in the granularity of seconds... I'm not sure how this would work, and what precedent exists for duplicating parameters.
    2. Set the timeout to X seconds internally if test commands are enabled. I don't think this would play nicely with tests, but I haven't verified.
    3. Talk to drivers for a possible downstream change to "logicalSessionTimeoutSeconds" completely.
  2. Add a test case that will run "find, wait for TTL*0.5, getMore, wait for TTL*0.5, getMore, wait for TTL*0.5, getMore, close cursor, verify the session exists" i.e. verify that the cursors remain open and the session is not expired.
  3. Once the workload is over wait for the TTL *1.5 and check that there are no sessions in the collection
  4. No need to manually invoke any refresh commands, the getMore execution guarantees that sessions are there.


 Comments   
Comment by Githook User [ 28/Dec/18 ]

Author:

{'username': 'BlakeIsBlake', 'email': 'blake.oler@mongodb.com', 'name': 'Blake Oler'}

Message: SERVER-37241 Add testing to verify proper expiration of sessions in the sessions collection

(cherry picked from commit 3caa3c4a4be7b84823f22f481365f58b124d6d00)
Branch: v4.0
https://github.com/mongodb/mongo/commit/49210e8f6bec46b416113f7b93784d7fff655f19

Comment by Githook User [ 26/Dec/18 ]

Author:

{'username': 'BlakeIsBlake', 'email': 'blake.oler@mongodb.com', 'name': 'Blake Oler'}

Message: SERVER-37241 Add testing to verify proper expiration of sessions in the sessions collection

(cherry picked from commit 3caa3c4a4be7b84823f22f481365f58b124d6d00)
Branch: v3.6
https://github.com/mongodb/mongo/commit/2760fd38a929ae790c6fbb7ffbb7ed94b387fc61

Comment by Githook User [ 31/Oct/18 ]

Author:

{'name': 'Blake Oler', 'email': 'blake.oler@mongodb.com', 'username': 'BlakeIsBlake'}

Message: SERVER-37241 Add testing to verify proper expiration of sessions in the sessions collection
Branch: master
https://github.com/mongodb/mongo/commit/3caa3c4a4be7b84823f22f481365f58b124d6d00

Comment by Misha Tyulenev [ 29/Oct/18 ]

blake.oler i see 2 distinct phases in the refresh:
1. update lastUsed field - to enable session expiring. The test must validate that the update computes the documents that need to be updated.
2. find the set difference between sessions collection and the open cursors and close "exipred" cursors
Im ok with the changes as long as the test should verify those work.

Comment by Blake Oler [ 29/Oct/18 ]

misha.tyulenev I think the new approach might actually be simpler... when working with the sessions collection in isolation, we can dictate exactly what we want the TTL behavior to be. Since expiration from a TTL index can be modeled as a deletion, we can just blanket delete necessary documents to simulate expiration.

At any given time, the logical session cache's refresh is relying upon the existence (or non-existence) of documents in the sessions collection. The logical session cache by design can't distinguish between a manual deletion and expiration. It assumes that if a document no longer exists in the collection, it has expired. It wouldn't need to know the difference between expiration and deletion, anyway – manual deletion of sessions records on-disk is undefined behavior to an end user.

We're taking advantage of the lack of distinction between deletion and expiration much to our advantage. I foresee this approach being many fewer lines of code than the previous approach, with the added benefit of forgoing the need for server code or parameter changes.

Comment by Misha Tyulenev [ 29/Oct/18 ]

blake.oler I agree with the new plan. It's a bit more elaborate than the original because the test must now reimplement part of the TTL index functionality - namely check and remove documents that are "expired" in order to get the logical sessions refresh work as designed.
I think its worth the effort as it gives us more stable tests that will not depend on a TTL thread scheduling

Comment by Misha Tyulenev [ 29/Oct/18 ]

Thank you max.hirschhorn yes looks like this is what's needed.

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