Skip to content

feat(NODE-3697): reduce serverSession allocation #3171

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
Mar 25, 2022
Merged

feat(NODE-3697): reduce serverSession allocation #3171

Changes from 2 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
8f7c72a
feat(NODE-3697): reduce serverSession allocation
nbbeeken Mar 15, 2022
6bf7b53
wip
nbbeeken Mar 16, 2022
02464c8
fix: hasEnded logic and test
nbbeeken Mar 16, 2022
a417d00
some comments
nbbeeken Mar 23, 2022
522408b
reorganize unit tests and add hasEnded false test
nbbeeken Mar 23, 2022
ef98aee
test and throw updates
nbbeeken Mar 24, 2022
fefdfa2
merge boolean checks
nbbeeken Mar 24, 2022
cbadd92
reorganized serverSession getter tests
nbbeeken Mar 25, 2022
34a2ee4
test: scenario where explicit session is separated from its server se…
nbbeeken Mar 25, 2022
b53255c
Boolean Logic++
nbbeeken Mar 25, 2022
c8b1057
Apply suggestions from code review
nbbeeken Mar 25, 2022
5a22be4
test: add check for acquire not being called again
nbbeeken Mar 25, 2022
fc37a3e
test: ended implict session doesn't call acquire
nbbeeken Mar 25, 2022
3570ed6
make sure session id doesn't change
nbbeeken Mar 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 19 additions & 2 deletions test/unit/sessions.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,20 +256,37 @@ describe('Sessions - unit', function () {
describe('from an implicit session', () => {
const session = new ClientSession(topology, serverSessionPool, { explicit: false }); // make an implicit session

it('should throw if the session hasEnded before serverSession was acquired', () => {
it('should throw if the session ended before serverSession was acquired', () => {
expect(session).to.have.property(serverSessionSymbol, null);
session.hasEnded = true;
expect(() => session.serverSession).to.throw(MongoRuntimeError);
});

it('should acquire a serverSession if clientSession.hadEnded is false', () => {
it('should acquire a serverSession if clientSession.hasEnded is false and serverSession is not set', () => {
expect(session).to.have.property(serverSessionSymbol, null);
session.hasEnded = false;
const acquireSpy = sinon.spy(serverSessionPool, 'acquire');
expect(session.serverSession).to.be.instanceOf(ServerSession);
expect(acquireSpy.calledOnce).to.be.true;
acquireSpy.restore();
});

it('should return the existing serverSession and not acquire a new one if one is already set', () => {
expect(session).to.have.property(serverSessionSymbol, null);
const acquireSpy = sinon.spy(serverSessionPool, 'acquire');
expect(session.serverSession).to.be.instanceOf(ServerSession);
expect(acquireSpy.calledOnce).to.be.true;

// call the getter a bunch more times
expect(session.serverSession).to.be.instanceOf(ServerSession);
expect(session.serverSession).to.be.instanceOf(ServerSession);
expect(session.serverSession).to.be.instanceOf(ServerSession);

// acquire never called again
expect(acquireSpy.calledOnce).to.be.true;

acquireSpy.restore();
});
});
});

Expand Down