[tap1r]

Description

Summary of Changes

ServerSession.hasTimedOut now compares elapsed idle minutes with logicalSessionTimeoutMinutes directly. It no longer reduces that duration modulo one day and then modulo one hour.

Notes for Reviewers

The - 1 minute comparison is the existing "within one minute of expiry, do not reuse" rule. Math.round is gone so a fractional minute cannot change which side of that boundary a session falls on. 9.5 minutes against a 10 minute timeout is still not reused (9.5 > 9).

Jira: NODE-7984

What is the motivation for this change?

A pooled server session idle longer than logicalSessionTimeoutMinutes was returned to the application during part of every hour. Commands then sent an lsid the server was allowed to have expired.

For bug fixes

Current (incorrect) behavior:

Idle time is folded into 0–59 minutes. With a 30 minute timeout, a session idle for 60, 120, 180, ... minutes is reused. With the driver's mock/default 10 minute timeout, a session idle for 60 minutes is reused (0 > 9 is false).

Expected behavior:

Idle time is the elapsed minutes since lastUse. The session is not reused when that value is greater than logicalSessionTimeoutMinutes - 1.

How to reproduce:

const session = new ServerSession();
session.lastUse = processTimeMS() - 60 * 60 * 1000;
session.hasTimedOut(30); // buggy result: false. elapsed minutes are 60, and 60 > 29.

Unit coverage: test/unit/sessions.test.ts ("does not reuse a session idle for one hour when the timeout is 30 minutes").

Affected versions:

The modulo is present on main (7.x / 6.x). It does not depend on a server version or topology. The server default for logicalSessionTimeoutMinutes is 30. When the topology has not reported a timeout, the pool uses 10.

Double check the following

• Lint is passing (npm run check:lint)
• Self-review completed using the steps outlined here
• PR title follows the correct format: type(NODE-xxxx)[!]: description
  • Example: feat(NODE-1234)!: rewriting everything in coffeescript
• Changes are covered by tests
• New TODOs have a related JIRA ticket

[PavelSafronov]

@tap1r, thank you for taking the time to create this PR! This is tracked in NODE-7984. The team is currently at capacity, so we will be reviewing it over the next few quarters as capacity allows.