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

Model.syncIndexes() fails with MongoError: a collection 'dbname.tablename' already exists #10420

Closed
rideddy84 opened this issue Jul 5, 2021 · 14 comments
Labels
confirmed-bug We've confirmed this is a bug in Mongoose and will fix it.
Milestone

Comments

@rideddy84
Copy link

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

What is the current behavior?
run Model.syncIndexes(); for existing model.

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

https://gist.github.com/rideddy84/a189d839cbef093f719a0e850f2c5ff7.js

{
  "compilerOptions": {
    "allowSyntheticDefaultImports": true,
    "esModuleInterop": true,
    "lib": ["esnext"],
    "module": "commonjs",
    "moduleResolution": "node",
    "noUnusedLocals": true,
    "noUnusedParameters": true,
    "outDir": "lib",
    "sourceMap": true,
    "target": "esnext"
  },
  "exclude": ["node_modules"]
}

What is the expected behavior?
Indexes are created if they weren't before.

What are the versions of Node.js, Mongoose and MongoDB you are using? Note that "latest" is not a version.
"mongoose": "5.13.2"
Node.js v14.17.0

@IslandRhythms IslandRhythms added the needs repro script Maybe a bug, but no repro script. The issue reporter should create a script that demos the issue label Jul 6, 2021
@IslandRhythms
Copy link
Collaborator

If you could provide a readable repro script, that would be very helpful.

@IslandRhythms
Copy link
Collaborator

const mongoose = require('mongoose');
const {Schema} = mongoose;


const schema = new Schema({ name: { type: String, unique: true } });
const Customer = mongoose.model('Customer', schema);

async function test() {
    await mongoose.connect('mongodb://localhost:27017/test', {
        useNewUrlParser: true,
        useUnifiedTopology: true
      });
    await mongoose.connection.dropDatabase();
    await Customer.collection.createIndex({ age: 1 }); // Index is not in schema

    Customer.collection.getIndexes().then(indexes => {
        console.log(indexes);
    })
    // Will drop the 'age' index and create an index on `name`
    await Customer.syncIndexes();

    Customer.collection.getIndexes().then(indexes => {
        console.log(indexes);
    })
}

test();

The above code runs fine. Copy it and change it to demonstrate your issue. @rideddy84

@rideddy84
Copy link
Author

Thanks for your help. @IslandRhythms
I read source code of syncIndexes. It doesn't have to create collection before cleanIndexes because I want to sync indexes for existing collections.
What could be appropriate approach in this case?

mongoose/lib/model.js

Lines 1384 to 1409 in c03cacb

Model.syncIndexes = function syncIndexes(options, callback) {
_checkContext(this, 'syncIndexes');
callback = this.$handleCallbackError(callback);
return this.db.base._promiseOrCallback(callback, cb => {
cb = this.$wrapCallback(cb);
this.createCollection(err => {
if (err != null && err.codeName !== 'NamespaceExists') {
return cb(err);
}
this.cleanIndexes((err, dropped) => {
if (err != null) {
return cb(err);
}
this.createIndexes(options, err => {
if (err != null) {
return cb(err);
}
cb(null, dropped);
});
});
});
}, this.events);
};

@IslandRhythms IslandRhythms added help This issue can likely be resolved in GitHub issues. No bug fixes, features, or docs necessary and removed needs repro script Maybe a bug, but no repro script. The issue reporter should create a script that demos the issue labels Jul 9, 2021
@IslandRhythms
Copy link
Collaborator

What do you mean by sync indexes for existing collections? Do you want to create an index on every collection?

@vkarpov15 vkarpov15 added this to the 5.13.3 milestone Jul 11, 2021
@Spown
Copy link

Spown commented Jul 14, 2021

I have been trying to solve this problem since I've updated from @5.11.19 to @5.12.8 and now with @5.13.2 it is still there. The error is introduced through mongodb@3.6 (or somewhere around this ver) and occurs on .syncIndexes() only if the raw DB (pre initializing mongoose) already contains the table. My guess your tests do cleanup before and after and drop test DB hence everything seems fine. winstonjs "fixed" it by ignoring it. You probably should too.

@Spown
Copy link

Spown commented Jul 14, 2021

What do you mean by sync indexes for existing collections? Do you want to create an index on every collection?

I think he, just like me, wants to force reindex on every app init. Because out-of-the-box, if the raw DB tables have indexes defined, mongoose ignores index settings from mongoose.model(), so you either need to drop indexes before registering a model (this is what I was doing a couple years ago) or run .syncIndexes(). Regardless whether they were changed or not this insures that raw DB and model settings match indexes-wise.

@rideddy84
Copy link
Author

What do you mean by sync indexes for existing collections? Do you want to create an index on every collection?

In my cases,

  1. Person A writes a model definition with one index. At first deployment, It is created by .syncIndexes() with its collection.
  2. Person B writes one more column with its index. On second deployment, I want to create new index with .syncIndexes() or something useful. But since collection is already exists, .syncIndexes() fails.
  3. 2 often occurs since application gets more complicated time goes by.

I want to ensure the production MongoDB has exactly same indexes with model definition codes.

@vkarpov15 vkarpov15 modified the milestones: 5.13.3, 5.13.4 Jul 16, 2021
@vkarpov15 vkarpov15 modified the milestones: 5.13.4, 5.13.5, 5.13.6 Jul 28, 2021
@vkarpov15 vkarpov15 modified the milestones: 5.13.6, 5.13.7 Aug 9, 2021
@vkarpov15
Copy link
Collaborator

@Spown @rideddy84 the below script shows that syncIndexes() works fine when the underlying collection already exists in MongoDB. The source code in #10420 (comment) shows that Mongoose ignores NamespaceExists errors when running createCollection() in syncIndexes(). Can you please modify the below script (same as what @IslandRhythms posted) to demonstrate your issue?

const mongoose = require('mongoose');
const {Schema} = mongoose;


const schema = new Schema({ name: { type: String, unique: true } });
const Customer = mongoose.model('Customer', schema);

async function test() {
    await mongoose.connect('mongodb://localhost:27017/test', {
        useNewUrlParser: true,
        useUnifiedTopology: true
      });
    await mongoose.connection.dropDatabase();
    await Customer.collection.createIndex({ age: 1 }); // Index is not in schema

    Customer.collection.getIndexes().then(indexes => {
        console.log(indexes);
    });
    // Will drop the 'age' index and create an index on `name`
    await Customer.syncIndexes();

    Customer.collection.getIndexes().then(indexes => {
        console.log(indexes);
    });
    console.log('Done');
}

test();

I also tried running the below script repeatedly, no errors

const mongoose = require('mongoose');
const { Schema } = mongoose;

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

const orderSchema = new Schema({
  orderNo: {
    type: String,
    required: true,
  },
  createdBy: { type: String, required: true, ref: "User" },
  createdAt: { type: Date, default: Date.now },
});

orderSchema.index({ createdAt: -1 });
orderSchema.index({ orderNo: 1 }, { unique: true });

orderSchema.virtual("review", {
  ref: "Review",
  localField: "_id",
  foreignField: "order",
  justOne: true,
});

orderSchema.set("toObject", { virtuals: true });
orderSchema.set("toJSON", { virtuals: true });

const name = "Order";
const collection = "orders";
const orders = mongoose.model(name, orderSchema, collection);

orders.syncIndexes().then(() => console.log('Done'));

@vkarpov15 vkarpov15 removed this from the 5.13.7 milestone Aug 10, 2021
@vkarpov15 vkarpov15 added can't reproduce Mongoose devs have been unable to reproduce this issue. Close after 14 days of inactivity. and removed help This issue can likely be resolved in GitHub issues. No bug fixes, features, or docs necessary labels Aug 10, 2021
@rideddy84
Copy link
Author

@vkarpov15 Thanks for review. But actually Mongoose cannot ignore because this.db is node-mongodb-native and it doesn't return error like it.

If fires "MongoError" without invoking callback that checks NamespaceExists.

https://github.com/mongodb/node-mongodb-native/blob/e69d9925713ede3bd80d7d23a6df60c6dd4542ef/src/db.ts#L243-L255

@vkarpov15
Copy link
Collaborator

Can you please elaborate on what you mean by "If fires "MongoError" without invoking callback that checks NamespaceExists."?

@rideddy84
Copy link
Author

this.db.createCollection(collection, options, cb);

Ok. You are expecting NamespaceExists when createCollection raises exception.

But actually, cb cannot be called because the native driver doesn't call cb when it throws an exception.
https://github.com/mongodb/node-mongodb-native/blob/e69d9925713ede3bd80d7d23a6df60c6dd4542ef/src/db.ts#L243-L255

@Spown
Copy link

Spown commented Aug 12, 2021

the below script shows that syncIndexes() works fine when the underlying collection already exists in MongoDB.

well, and I get the MongoError: collection already exists error. Maybe mongoose reacts differently depending on the underlying MongoDB engine version? I use 3.2

(node:19652) DeprecationWarning: collection.ensureIndex is deprecated. Use createIndexes instead.
(Use `node --trace-deprecation ...` to show where the warning was created)
(node:19652) UnhandledPromiseRejectionWarning: MongoError: collection already exists
    at MessageStream.messageHandler (d:\Projects\mongo_se_test\node_modules\mongodb\lib\cmap\connection.js:272:20)
    at MessageStream.emit (events.js:400:28)
    at processIncomingData (d:\Projects\mongo_se_test\node_modules\mongodb\lib\cmap\message_stream.js:144:12)
    at MessageStream._write (d:\Projects\mongo_se_test\node_modules\mongodb\lib\cmap\message_stream.js:42:5)
    at writeOrBuffer (internal/streams/writable.js:358:12)
    at MessageStream.Writable.write (internal/streams/writable.js:303:10)
    at Socket.ondata (internal/streams/readable.js:726:22)
    at Socket.emit (events.js:400:28)
    at addChunk (internal/streams/readable.js:290:12)
    at readableAddChunk (internal/streams/readable.js:265:9)
    at Socket.Readable.push (internal/streams/readable.js:204:10)
    at TCP.onStreamRead (internal/stream_base_commons.js:188:23)
    at TCP.callbackTrampoline (internal/async_hooks.js:131:17)
(node:19652) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)
(node:19652) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
{_id_: Array(1), age_1: Array(1)}

@vkarpov15 vkarpov15 added this to the 5.13.8 milestone Aug 14, 2021
@vkarpov15 vkarpov15 added needs repro script Maybe a bug, but no repro script. The issue reporter should create a script that demos the issue and removed can't reproduce Mongoose devs have been unable to reproduce this issue. Close after 14 days of inactivity. labels Aug 14, 2021
@vkarpov15
Copy link
Collaborator

@Spown you're right, this issue only happens in MongoDB 3.2 and earlier. Thanks for pointing that out - that's why we ask for MongoDB version in the issue template :)

@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 Aug 17, 2021
@rideddy84
Copy link
Author

@vkarpov15 I'm sorry for missing out the MongoDB version in my case. It is AWS docdb 4.0.0. Thanks.

@Automattic Automattic locked and limited conversation to collaborators Aug 23, 2021
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

4 participants