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

MinimongoError: The positional operator did not find the match needed from the query #9

Open
jasongrishkoff opened this issue Feb 11, 2021 · 28 comments

Comments

@jasongrishkoff
Copy link

jasongrishkoff commented Feb 11, 2021

Here's the code throwing this error:

        Meteor.defer(() => {
            Campaigns.update(
                {_id:nowPlaying,detail:{$elemMatch:detail}},
                {$set:{'detail.$.listened_for':length}}
            )
        })

This is executed server-side, not on client.

Here's the full log:

gxbsm
2021-02-11 19:40:53+02:00Exception in defer callback: MinimongoError: The positional operator did not find the match needed from the querygxbsm
2021-02-11 19:40:53+02:00 at MinimongoError (packages/minimongo/common.js:1087:17)gxbsm
2021-02-11 19:40:53+02:00 at findModTarget (packages/minimongo/local_collection.js:1947:17)gxbsm
2021-02-11 19:40:53+02:00 at packages/minimongo/local_collection.js:1160:24gxbsm
2021-02-11 19:40:53+02:00 at Array.forEach (<anonymous>)gxbsm
2021-02-11 19:40:53+02:00 at packages/minimongo/local_collection.js:1144:28gxbsm
2021-02-11 19:40:53+02:00 at Array.forEach (<anonymous>)gxbsm
2021-02-11 19:40:53+02:00 at Function.LocalCollection._modify (packages/minimongo/local_collection.js:1134:27)gxbsm
2021-02-11 19:40:53+02:00 at packages/zegenie:redis-oplog/lib/mongo/mutator.js:119:27gxbsm
2021-02-11 19:40:53+02:00 at Array.forEach (<anonymous>)gxbsm
2021-02-11 19:40:53+02:00 at Mongo.Collection.update (packages/zegenie:redis-oplog/lib/mongo/mutator.js:117:14)gxbsm
2021-02-11 19:40:53+02:00 at Mongo.Collection.update (packages/zegenie:redis-oplog/lib/mongo/extendMongoCollection.js:60:29)gxbsm
2021-02-11 19:40:53+02:00 at server/methods/listens.jsx:129:23gxbsm
2021-02-11 19:40:53+02:00 at Meteor.EnvironmentVariable.EVp.withValue (packages/meteor.js:1234:12)gxbsm
2021-02-11 19:40:53+02:00 at packages/meteor.js:550:25gxbsm
2021-02-11 19:40:53+02:00 at runWithEnvironment (packages/meteor.js:1286:24)
@ramezrafla
Copy link
Owner

Aaaah - positional operators! Meteor has an incomplete implementation in mini-mongo
How come it worked in dev, that call was never made?

@ramezrafla
Copy link
Owner

I am taking a closer look as we speak - FYI

@jasongrishkoff
Copy link
Author

Great, thanks! Gonna add a few more errors. I didn't actually test this one properly it seems. I've been laser-focused on the chats not updating, and those were all working pretty smooth hehe.

@ramezrafla
Copy link
Owner

Can you check your packages folder for the commit your are using of this lib?
mutator.js:117 has nothing offending in it in the current version
Thanks

@ramezrafla
Copy link
Owner

Sorry spoke too soon -- this is the offending line
LocalCollection._modify(after, modifier)

@jasongrishkoff
Copy link
Author

Currently on 2.0.15. Only installed it for the first time a few hours ago :)

@icellan
Copy link

icellan commented Jul 1, 2021

This is blocking me from using this package in a project. The offending query on my side is:

collection.update({
      _id: {
        $in: notificationIds,
      },
      'notifications.userId': this.userId,
    }, {
      $set: {
        'notifications.$.readOn': new Date(),
      },
    },{
      multi: true,
    });

Also tried with $elemMatch in the query:

collection.update({
      _id: {
        $in: notificationIds,
      },
      notifications: {
        $elemMatch: {
          userId: this.userId,
          readOn: {
            $exists: false
          }
        }
      }
    },{
      $set: {
        'notifications.$.readOn': new Date()
      }
    }, {
      multi: true
    });

Without the _id part in the query, so only the notifications part, it does work properly.

Had to revert to cultofcoders:redis-oplog to make this work.

Any progress on this issue yet @ramezrafla ?

Error in my case:

Exception while invoking method 'markNotificationRead' MinimongoError: The positional operator did not find the match needed from the query
    at MinimongoError (packages/minimongo/common.js:1087:17)
    at findModTarget (packages/minimongo/local_collection.js:1965:17)
    at packages/minimongo/local_collection.js:1160:24
    at Array.forEach (<anonymous>)
    at packages/minimongo/local_collection.js:1144:28
    at Array.forEach (<anonymous>)
    at Function.LocalCollection._modify (packages/minimongo/local_collection.js:1134:27)
    at packages/zegenie:redis-oplog/lib/mongo/mutator.js:116:27
    at Array.forEach (<anonymous>)
    at ns.Collection.update (packages/zegenie:redis-oplog/lib/mongo/mutator.js:114:14)
    at ns.Collection.update (packages/zegenie:redis-oplog/lib/mongo/extendMongoCollection.js:60:29)
    at ns.Collection.Mongo.Collection.<computed> [as update] (packages/aldeed:collection2/collection2.js:217:19)
    at MethodInvocation.markNotificationRead (imports/api/methods/notifications.js:12:31)
    at maybeAuditArgumentChecks (packages/ddp-server/livedata_server.js:1771:12)
    at packages/ddp-server/livedata_server.js:719:19
    at Meteor.EnvironmentVariable.EVp.withValue (packages/meteor.js:1234:12)
    at packages/ddp-server/livedata_server.js:717:46
    at Meteor.EnvironmentVariable.EVp.withValue (packages/meteor.js:1234:12)
    at packages/ddp-server/livedata_server.js:715:46
    at new Promise (<anonymous>)
    at Session.method (packages/ddp-server/livedata_server.js:689:23)
    at packages/ddp-server/livedata_server.js:559:43
RedisOplog: RaceDetectionManager started

@ramezrafla
Copy link
Owner

@jasongrishkoff
Can you test pls with the new minimongo?

@jasongrishkoff
Copy link
Author

Hi @ramezrafla I actually stopped using this package back in February - there were too many issues, so I just went back to plain old oplog and scaled everything up. Unfortunately I can't test this for you :(

@Miraell
Copy link

Miraell commented Dec 3, 2021

@ramezrafla it still does not work. I made a quick monkey patch (works for only simple case) for myself which looks like this:

mutator.js

116           let options = {};
117           
118           const positionalKey = Object.keys(updateSelector).filter(value => value.split('.').length > 1);
119           if (positionalKey.length === 1) {
120             const split = positionalKey[0].split('.');
121             const valueToSearch = updateSelector[positionalKey];
122             options.arrayIndices = [before[split[0]].findIndex(value => value[split[1]] === valueToSearch)];
123           }   
124             
125           LocalCollection._modify(after, modifier, options);

@jasongrishkoff
Copy link
Author

FYI, was playing around with the package and can confirm this still isn't working for me either.

@ramezrafla
Copy link
Owner

@jasongrishkoff
If I implement @Miraell 's fix above, will it work for you?

@jasongrishkoff
Copy link
Author

Same error with the monkey patch. Code I'm firing looks kinda like this:

            var detail = {type:'listen',user:this.userId}
            History.update(
                {_id:id,detail:{$elemMatch:detail}},
                {$set:{
                    'detail.$.seconds':120,
                }}
            )

@JackAdams
Copy link

JackAdams commented Oct 24, 2022

I'm having problems with the positional operator server side with:

Groups.update({"members.user_id": "asdfkjslea3q489"}, {$set: {"members.$.name": "New Name"}}, {multi: true})

The error is:

Error [MinimongoError]: The positional operator did not find the match needed from the query

Going to need to revert to cult-of-coders:redis-oplog for the time being, as there are too many positional operators used in my codebase. Pity, because zegenie:redis-oplog gets the merging of published documents with different sets of fields right (which cult-of-coders:redis-oplog doesn't).

@ramezrafla
Copy link
Owner

@JackAdams
Could you test the monkey patch above (#9 (comment))
I don't use positional operators so can't test them.

If it works, I'll make the chance and push to production

@JackAdams
Copy link

JackAdams commented Oct 24, 2022

Seems to have fixed the problem for me; no error and correct update made in the document in the db. This being the case, I'm going to forge ahead with zegie:redis-oplog for now. Thanks for all the work on this @ramezrafla , I really appreciate it.
(I notice that @Miraell mentioned it only works in simple cases. I think most/all of the cases I use the position operator in would be simple cases that the patch covers. It might be worth mentioning in the docs, though, that the positional operator only supports basic use cases. Feel free to use that line of code in my comment above as an example.)

@ramezrafla
Copy link
Owner

Great!
We have been using this version of redis-oplog in production with tens of thousands of users simultaneously. So we know it works for our application. As new use cases come up, I'll try to incorporate without jeopardizing its mission to be scalable.

Will update and push to production shortly.

@JackAdams
Copy link

JackAdams commented Oct 24, 2022

Out of interest, @ramezrafla , have you managed to have tens of thousands of concurrent users, with multiple subscriptions each, making frequent writes, and still use mongo unsharded?

@ramezrafla
Copy link
Owner

Yes! That's the purpose of this package. No sharding yet. The cache within zegenie:redis-oplog dramatically reduces hits to the DB. Since each user is connected via websocket, we can keep their recurrent data. When we write to the DB, we don't need to repull (like original redis-oplog), we simply update the local cache.

Which brings the next question, with positional operators in place, please check that the data in the client is also updated (not just DB).

@JackAdams
Copy link

JackAdams commented Oct 24, 2022

Happy to confirm that changes are being correctly propagated to the client after an update using the positional operator.

@JackAdams
Copy link

JackAdams commented Oct 24, 2022

Super happy to hear about the scalability. Even with cult-of-coders:redis-oplog the database was really hurting with a lot of users making frequent writes. (Full respect to Theo and his team, too, by the way -- redis-oplog was a huge step forward for scalability of Meteor apps with a lot of pub/sub.)

@ramezrafla
Copy link
Owner

Excellent! Theo handled a lot of the interfacing with redis and the internals of Meteor. There is a bit of hacking in there to override internal pub/subs. So he did all the heavy lifting.

@JackAdams
Copy link

JackAdams commented Oct 24, 2022

@ramezrafla . Maybe hold off pushing that patch to production. I just did and have a lot of users reporting issues. Will update when I know more.

@JackAdams
Copy link

JackAdams commented Oct 24, 2022

Been testing, and the fix for the issues I was seeing is to make sure the type is an array when trying to find the array index to use for the positional operator. (This is because I was using a string of dot separated fields to query nested objects, not arrays, which is a common enough practice, so the suggested fix to the patch below if (_.isArray(before[split[0]])) should definitely be used if this patch is going to production.)

let options = {};
const positionalKey = Object.keys(updateSelector).filter(value => value.split('.').length > 1);
if (positionalKey.length === 1) {
  const split = positionalKey[0].split('.');
  const valueToSearch = updateSelector[positionalKey];
  if (_.isArray(before[split[0]])) {
    options.arrayIndices = [before[split[0]].findIndex(value => value[split[1]] === valueToSearch)];
  }
}
LocalCollection._modify(after, modifier, options);

This code block replaces:

LocalCollection._modify(after, modifier)

Are you still using the original test suite, @ramezrafla ?

@ramezrafla
Copy link
Owner

Excellent @JackAdams
If that works, we can push in production.

I am not using the test suites, to simplify things. In hindsight, maybe a mistake.

@JackAdams
Copy link

I've pushed that modified patch to production and so far, so good. I'll let you know if we run into any problems.

@JackAdams
Copy link

JackAdams commented Oct 25, 2022

Okay. Better not put that patch into production. It seems to be giving some false positives for the old version of alanning:roles, which was a popular package and may still be in production in some apps. Given the way the data for the roles is structured in user docs I can sort of guess what's happening. I need to test a bit more. (It also means that any queries on nested docs using dot delimited strings probably aren't working as advertised!)
Edit: possibly a false alarm. I'll let you know when I've finished investigating.

@JackAdams
Copy link

JackAdams commented Oct 26, 2022

I don't think it was anything to do with the patch, in the end. But this is really weird. On the Mongo console:

meteor:PRIMARY> db.users.findOne({_id:"ZqRoBNapiGcQTtTBT", nonExistentField: "nonExistentValue"})
null

Which is what I expected. But in the Meteor shell and in my app code:

> Meteor.users.findOne({_id:"ZqRoBNapiGcQTtTBT", nonExistentField: "nonExistentValue"})
{
  _id: 'ZqRoBNapiGcQTtTBT',

Which shouldn't be happening at all. This only happens when I've got Meteor.users.startCaching() in the server code.
Any idea what's going on? (Note: this happens with or without the patch.)
I've just tested this with other collections and that seems to be consistent. As long as there's an _id that matches, it doesn't matter what other fields there are, the findOne method returns the document. This is not the case for find, which works as expected. This strange behaviour of the findOne method only happens on the server for collections with caching started; it works as expected on the client.

Edit: sorry, I'll split this out into a separate issue (#25)

The positional operator patch (with the guard condition with _.isArray) remains good for production, as far as I know. I've got a few too many findOne instances server side that I need to work consistently with mongo to keep using the zegenie:redis-oplog package in production though, as much as I'd like to. :-(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants