Skip to content
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.

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

startSession() hangs on primary failover of a replica set #8319

Closed
lahiruchandima opened this issue Nov 10, 2019 · 2 comments
Closed

startSession() hangs on primary failover of a replica set #8319

lahiruchandima opened this issue Nov 10, 2019 · 2 comments
Labels
confirmed-bug We've confirmed this is a bug in Mongoose and will fix it.
Milestone

Comments

@lahiruchandima
Copy link

lahiruchandima commented Nov 10, 2019

I opened this ticket for an issue I had been facing for a while now, but it had been closed without a fix. I attached a reproduce script to this ticket, where issue can be easily recreated.

Reproduce script:
https://drive.google.com/open?id=1rN0nBTOwkEnLzPMCpR3pyhLd19fQB64i

const mongoose = require('mongoose');
const dbUrl = 'mongodb+srv://myuser:mypass@myapp.mongodb.net/mydb?retryWrites=true&w=majority'; // TODO: add a valid mongodb cluster

mongoose.connect(dbUrl, {useNewUrlParser: true, useUnifiedTopology: false}, error => {
    if (error) {
        console.log('Error occurred when connecting to database. ' + error);
    }
});

function testStartSession() {
    const startSessionTimeout = setTimeout(() => {
        console.log('Start session timeout exceeded. quitting.');
        process.exit(0);
    }, 60000);

    const time = Date.now();
    mongoose.startSession()
        .then(mongoSession => {
            console.log('start session successful. took ' + (Date.now() - time));
            clearTimeout(startSessionTimeout);
            mongoSession.endSession();
            setTimeout(() => testStartSession(), 1000);
        })
        .catch(reason => {
            console.log('start session failed. took ' + (Date.now() - time) + '. reason: ' + reason);
            clearTimeout(startSessionTimeout);
            setTimeout(() => testStartSession(), 1000);
        });
}

mongoose.connection.once('open', () => {
    console.log('Connected to database');
    testStartSession();
});

In above code, whenever the primary instance of the mongodb replica set is stopped, mongoose.startSession() neither succeeds, nor fails. It just hangs.

I tested with both useUnifiedTopology enabled and disabled, and the issue exists in both cases.

I tested with a DB query operation instead of mongoose.startSession(), and when a primary failover occurs, the immediate next read operation fails (doesn't hang like startSession), and consequent read operations succeed, which is acceptable.

@lahiruchandima
Copy link
Author

lahiruchandima commented Nov 13, 2019

The problem seems to lie in _wrapConnHelper() function in connection.js of mongoose. If the connection is not in STATES.connected state, function waits until open event to get fired, but it never gets fired on a re-connection. Instead, reconnect event gets fired on a re-connection.

I modified the function to handle reconnect event too, and it now works on a reconnection. Following is the function after the modification.

function _wrapConnHelper(fn) {
  return function() {
    const cb = arguments.length > 0 ? arguments[arguments.length - 1] : null;
    const argsWithoutCb = typeof cb === 'function' ?
      Array.prototype.slice.call(arguments, 0, arguments.length - 1) :
      Array.prototype.slice.call(arguments);
    return utils.promiseOrCallback(cb, cb => {
      if (this.readyState !== STATES.connected) {
        this.once('open', function() {
          fn.apply(this, argsWithoutCb.concat([cb]));
        });
        
        /// START Newly added part
        this.once('reconnect', function() {
          fn.apply(this, argsWithoutCb.concat([cb]));
        });
        /// END Newly added part
      } else {
        fn.apply(this, argsWithoutCb.concat([cb]));
      }
    });
  };
}

The fix is not ideal since it doesn't protect fn.apply() from getting called twice, once in open event and again in reconnect event. It would be great if someone who is familiar with the code can give a proper fix soon.

@vkarpov15 vkarpov15 added the needs repro script Maybe a bug, but no repro script. The issue reporter should create a script that demos the issue label Nov 16, 2019
@vkarpov15 vkarpov15 added this to the 5.7.12 milestone Nov 16, 2019
@vkarpov15
Copy link
Collaborator

Should be fixed by 757579d

@vkarpov15 vkarpov15 added confirmed-bug We've confirmed this is a bug in Mongoose and will fix it. and removed needs repro script Maybe a bug, but no repro script. The issue reporter should create a script that demos the issue labels Nov 19, 2019
@Automattic Automattic locked as resolved and limited conversation to collaborators Dec 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
confirmed-bug We've confirmed this is a bug in Mongoose and will fix it.
Projects
None yet
Development

No branches or pull requests

2 participants