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

unique indexes not being created with autoIndex=true #8786

Closed
sgpinkus opened this issue Apr 12, 2020 · 9 comments
Closed

unique indexes not being created with autoIndex=true #8786

sgpinkus opened this issue Apr 12, 2020 · 9 comments
Labels
can't reproduce Mongoose devs have been unable to reproduce this issue. Close after 14 days of inactivity.

Comments

@sgpinkus
Copy link
Contributor

sgpinkus commented Apr 12, 2020

Do you want to request a feature or report a bug?
Bug.

What is the current behavior?
Setting unique on schema field does not create a unique index in a wide range of cases.

If the current behavior is a bug, please provide the steps to reproduce.

const mongoose = require('mongoose');

const testSchema = new mongoose.Schema({
  name: { type: String, unique: true, required: true, index: true },
});

async function main() {
  mongoose.set('autoIndex', true);
  mongoose.connect('mongodb://localhost/test', {
    useNewUrlParser: true,
    useCreateIndex: true,
    useUnifiedTopology: true,
    autoIndex: true,
  });
  await new Promise((resolve, reject) => {
    mongoose.connection
      .on('error', (err) => reject(err))
      .once('open', () => { console.debug('Mongooose open'), resolve(true); })
  });
  // #1 This next line has to come after the above connection open:
  const Test = mongoose.model('Test', testSchema);
  // #2 W/o this next line the index is never created:
  // await new Promise((resolve, reject) => { Test.init(resolve); });
  await Test.insertMany([{ name: 'x' }, { name: 'x' }, { name: 'x' }],  { runValidators: true });
  console.log((await Test.find({})));
}

main()
  .then(() => process.exit())
  .catch((e) => { console.error(e); process.exit(); })

What is the expected behavior?
I shouldn't need to do (1) and (2) (see above code) to have the index created. There is nothing mentioned about that in the docs that I can see. It's not intuitive and fragile. Model.init() is also described an alternative to autoIndex=true, not required for indexing.

What are the versions of Node.js, Mongoose and MongoDB you are using? Note that "latest" is not a version.
5.9.7

Welcome to Node.js v12.16.2.
Type ".help" for more information.
> require('mongoose').version
'5.9.7'
@AbdelrahmanHafez AbdelrahmanHafez added has repro script There is a repro script, the Mongoose devs need to confirm that it reproduces the issue can't reproduce Mongoose devs have been unable to reproduce this issue. Close after 14 days of inactivity. and removed has repro script There is a repro script, the Mongoose devs need to confirm that it reproduces the issue labels Apr 13, 2020
@AbdelrahmanHafez
Copy link
Collaborator

Unless you changed the default MongoDB port, you have a typo:
localhost:27019/test => localhost:27017/test

Running the script you provided with the my MongoDB port (27017) throws an error:

    at OrderedBulkOperation.handleWriteError (\node_modules\mongoose\node_modules\mongo
db\lib\bulk\common.js:1210:11)                                                                                                        
    at resultHandler (\node_modules\mongoose\node_modules\mongodb\lib\bulk\common.js:51
9:23)                                                                                                                                 
    at handler (\node_modules\mongoose\node_modules\mongodb\lib\core\sdam\topology.js:9
13:24)                                                                                                                                
    at \node_modules\mongoose\node_modules\mongodb\lib\cmap\connection_pool.js:352:13  
    at handleOperationResult (\node_modules\mongoose\node_modules\mongodb\lib\core\sdam
\server.js:487:5)                                                                                                                     
    at MessageStream.messageHandler (\node_modules\mongoose\node_modules\mongodb\lib\cm
ap\connection.js:270:5)                                                                                                               
    at MessageStream.emit (events.js:311:20)                                                                                          
    at processIncomingData (\node_modules\mongoose\node_modules\mongodb\lib\cmap\messag
e_stream.js:144:12)                                                                                                                   
    at MessageStream._write (\node_modules\mongoose\node_modules\mongodb\lib\cmap\messa
ge_stream.js:42:5)                                                                                                                    
    at doWrite (_stream_writable.js:441:12)                                                                                           
    at writeOrBuffer (_stream_writable.js:425:5)                                                                                      
    at MessageStream.Writable.write (_stream_writable.js:316:11)                                                                      
    at Socket.ondata (_stream_readable.js:714:22)                                                                                     
    at Socket.emit (events.js:311:20)                                                                                                 
    at addChunk (_stream_readable.js:294:12)                                                                                          
    at readableAddChunk (_stream_readable.js:275:11) {                                                                                
  index: 0,                                                                                                                           
  code: 11000,                                                                                                                        
  errmsg: 'E11000 duplicate key error collection: test.tests index: name_1 dup key: { : "x" }',                                       
  op: { _id: 5e9499c6a2b5fa2d085564dd, name: 'x', __v: 0 },                                                                           
  name: 'BulkWriteError',                                                                                                             
  driver: true,                                                                                                                       
  err: {                                                                                                                              
    index: 0,                                                                                                                         
    code: 11000,                                                                                                                      
    errmsg: 'E11000 duplicate key error collection: test.tests index: name_1 dup key: { : "x" }',                                     
    op: { _id: 5e9499c6a2b5fa2d085564dd, name: 'x', __v: 0 }                                                                          
  },                                                                                                                                  
  result: BulkWriteResult {                                                                                                           
    result: {                                                                                                                         
      ok: 1,                                                                                                                          
      writeErrors: [Array],                                                                                                           
      writeConcernErrors: [],                                                                                                         
      insertedIds: [Array],                                                                                                           
      nInserted: 0,                                                                                                                   
      nUpserted: 0,                                                                                                                   
      nMatched: 0,                                                                                                                    
      nModified: 0,                                                                                                                   
      nRemoved: 0,                                                                                                                    
      upserted: []                                                                                                                    
    }                                                                                                                                 
  },                                                                                                                                  
  [Symbol(mongoErrorContextSymbol)]: {}                                                                                               
}                                                                                                                                     

@sgpinkus
Copy link
Contributor Author

Yeah I changed the port. Well actually I have three MongoDB servers running for testing on 27017-19. Obviously you have to have MongoDB running on the configured port. I will update the OP so people don't get confused.

@AbdelrahmanHafez
Copy link
Collaborator

I modified the script a little bit, so we start on a fresh database every time which may affect the test.

Does this still pass silently?

// 8786
const mongoose = require('mongoose');

async function main () {
  mongoose.set('autoIndex', true);

  await mongoose.connect('mongodb://localhost/test', {
    useNewUrlParser: true,
    useCreateIndex: true,
    useUnifiedTopology: true,
    autoIndex: true
  });

  console.log('Mongoose open');

  await mongoose.connection.dropDatabase();

  const testSchema = new mongoose.Schema({
    name: { type: String, unique: true, required: true, index: true }
  });

  const Test = mongoose.model('Test', testSchema);
  await Test.insertMany([{ name: 'x' }, { name: 'x' }, { name: 'x' }], { runValidators: true });
}

main()
  .then(() => process.exit())
  .catch((err) => { console.error(err); process.exit(); });

@sgpinkus
Copy link
Contributor Author

sgpinkus commented Apr 14, 2020

The error does not occur with the above script. If I remove the dropDatabase() the error does reoccur.

In my initial testing I was using mongo shell to drop the collection: db.tests.drop() I even tried dropping the index first too db.tests.dropIndexes() (even though the index is not actually present). I thought I tried restarting the mongod server as well last time, but I've just tried that again and this also seems to consistently fix the issue (without the dropDatabase()).

What could be going on? Some type of caching?


UPDATE: Also noticed just doing a find() first causes the index to be successfully created without the delete:

// 8786
const mongoose = require('mongoose');

async function main () {
  mongoose.set('autoIndex', true);

  await mongoose.connect('mongodb://localhost:27019/test', {
    useNewUrlParser: true,
    useCreateIndex: true,
    useUnifiedTopology: true,
    autoIndex: true
  });

  console.log('Mongoose open');

  //await mongoose.connection.dropDatabase();

  const testSchema = new mongoose.Schema({
    name: { type: String, unique: true, required: true, index: true }
  });

  const Test = mongoose.model('Test', testSchema);
  console.log((await Test.find({})));
  await Test.insertMany([{ name: 'x' }, { name: 'x' }, { name: 'x' }], { runValidators: true });
  console.log((await Test.find({})));
}

main()
  .then(() => process.exit())
  .catch((err) => { console.error(err); process.exit(); });

UPDATE 2: I was reading the [ensureIndex()] doc. For that method index creation errors are emitted as an "index" event so I added this to the initial version with errors just after creating Test model:

  Test.on('index', function (err) {
    if (err) console.error(err); // error occurred during index creation
  })

It shows there is an index creation error because duplicates exist:

MongoError: E11000 duplicate key error collection: test.tests index: name_1 dup key: { name: "x" }

But they don't exist at the time the connection opened or the Model is created. So maybe this some kind of race condition? Based on that hypothesis, this is another fix confirmed to work:

  await new Promise((resolve, reject) => {
    Test.on('index', function (err) {
      if (err) reject(err); // error occurred during index creation
      resolve();
    });
    Test.ensureIndexes();
  })

@AbdelrahmanHafez
Copy link
Collaborator

You might find that helpful: https://mongoosejs.com/docs/faq.html#unique-doesnt-work

@AbdelrahmanHafez
Copy link
Collaborator

As a side note, most mongoose functions return a thenable, so you can await them without manually creating a promise.
for example:
await mongoose.connect(...)
await Test.init() instead of await new Promise((resolve, reject) => { Test.init(resolve); });

@sgpinkus
Copy link
Contributor Author

OK so this is not actually a bug. It's just complicated and not very well documented :(.

@sgpinkus
Copy link
Contributor Author

sgpinkus commented Apr 15, 2020

As a side note, most mongoose functions return a thenable, so you can await them without manually creating a promise.

@AbdelrahmanHafez out of interest we're you talking about this part from OP?:

  mongoose.connect('mongodb://localhost/test', {...});
  await new Promise((resolve, reject) => {
    mongoose.connection
      .on('error', (err) => reject(err))
      .once('open', () => { console.debug('Mongooose open'), resolve(true); })
  });

I can just do this?:

await mongoose.connect('mongodb://localhost/test', {...});

UPDATE: Oh yeah you can ... Ugh all this time :).

@vkarpov15
Copy link
Collaborator

@sgpinkus yes you can do await mongoose.connect('mongodb://localhost/test', {...}). That's the approach we recommend. Unfortunately a lot of older Mongoose tutorials use the event emitter pattern.

Here's the docs on Model.init(): https://mongoosejs.com/docs/api/model.html#model_Model.init . The validation docs go into this a little bit when explaining that unique is not a validator: https://mongoosejs.com/docs/validation.html#the-unique-option-is-not-a-validator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
can't reproduce Mongoose devs have been unable to reproduce this issue. Close after 14 days of inactivity.
Projects
None yet
Development

No branches or pull requests

3 participants