I have a query that I’m running from the server as I’m doing to server side api calls that should / can run while the app is not active on the client side. Basically geo-coding some addresses.
Anyway, I have a query I can run in Meteor shell and it works just as it should, but when it’s run from the server app it gives back results inconsistent with the specified criteria.
let addInfo = Addresses.find({ geoCodable: true, geoCoded: false }, { limit: 1000 }).fetch();
When I run this in Meteor Shell it does give my the results matching the criteria.
But from the server It gives me addresses with geoCoded: true, geoCodable: fasle, and mixes.
Any thoughts on why, or what I might have incorrect?
Thanks, for any help,
Can you share the method code that has this line in it?
Sure, it’s ugly though, so don’t judge. I have some inherent setTimeouts to keep from flooding the api endpoint with requests for coordinates.
'geocode' (g) {
// First we'll get rid of junk that just gums up what we are trying to do...like addresses with no block number.
// let's first query for all the address strings.
let addInfo = Addresses.find({ geoCodable: true, geoCoded: false }, { limit: 1000}).fetch(); // <-- the .fecth() turns the returned result into an array
// now let's get the length of the array of addresses (this is a lot of data).
let addressInfoSize = addInfo.length;
if (addressInfoSize <= 0) {
console.log("");
console.log("----------------------------------");
console.log("Nothing left to Geocode");
console.log("----------------------------------");
console.log("");
return;
}
console.log('Address Length of Array: ' + addressInfoSize);
let counter = 0;
var runGeoCodeCountUp = function(counter) {
if (counter < 1000) {
if (g < 1000) {
console.log("round " + g);
let addString = addInfo[g].addressString;
if (typeof addString == 'undefined' || addString == null || addString == "") {
// console.log('No Address String');
// console.log('--------------------------------');
g = g + 1;
} else {
if (typeof addInfo[g].latitude != 'undefined' && addInfo[g].latitude != 0 && addInfo[g].latitude != null) {
// console.log('Latitude exists already.');
// console.log('--------------------------------');
g = g + 1;
} else {
// console.log('GeoCoding this one.');
// console.log('!!!! ---------------- !!!!');
Meteor.call('getXYCallLocationInfo', addString);
g = g + 1;
Meteor.setTimeout(function() {
// console.log("");
// console.log("---------------------------------");
// console.log("Waiting 1 second.");
// console.log("---------------------------------");
// console.log("");
}, 1000);
}
}
}
counter = counter + 1;
Meteor.setTimeout(function() {
runGeoCodeCountUp(counter);
}, 2000)
} else {
Meteor.call('geocode', 0);
}
}
runGeoCodeCountUp(counter);
},
When you say “Meteor Shell”, do you mean MongoDB shell (meteor mongo)? The Meteor shell is the server app.
No, I mean meteor shell. I run that, then import my Addresses from my /imports/api/ directory, and run the query through there just like it’s written in my server side code. That’s how i know it’s working at least somewhere. Just not in my code on my server when run through the actual code. Does that make sense?
How frequently does this method get called? And is the collection updated elsewhere? I’m wondering if there is a race condition somewhere. Similarly, how do you know you’re getting bad data (I assume you’re logging it somewhere?)
The method is called technically by a click on the UI side. Then it runs on the server in a loop. Then it runs the 1000 addresses it pulled, and after that will run again. The 1000 addresses can take anywhere from 10, to 2000 seconds to run, depending on whether each one is geo-coded or not.
I take it slow, again, so as not to bombard the endpoint api with requests. Also, so that I can monitor the results I get back.
I use the http method built into meteor (i think, but maybe it’s a package I added), and call the API. The result is sent back and it’s just logged out when that happens.
When that function is called, I first check my Addresses to see if the address being requests already has coordinates (aka. is already geo-coded), and if so, I don’t call the api, but just use what’s there. This keeps me from hitting the api over and over and over for the same address.
I can see this logged out in the console. And I can see that the geoCoded value is true, which means the query grabbed a bunch of addresses it shouldn’t have according to the criteria.
I can’t see where you’d get console log output of the geoCoded being true - you don’t seem to log that anywhere. I would say, that clicking the button twice in quick succession on the UI would cause you to geocode every address twice - pulling Addresses at the begining of a long running function might not be the way to go. If your goal here is to implement a kind of queue, you should “lock” all the addresses at the begining, something like this:
let addInfo = Addresses.find({ locked: {$exists: false}, geoCodable: true, geoCoded: false }, { limit: 1000}).fetch(); // <-- the .fecth() turns the returned result into an array
Addresses.update({ _id: {$in: addInfo.map(a => a._id)}}, {$set: {locked: true}}, { multi: true})
This still has a race condition, but it is much smaller
I call a few other functions throughout this process to do a few things. I use these functions in a couple of places in my application in a more ‘on-the-fly’ way for the user. This is just my start of an attempt to “bulk” geo-code addresses in the background so that eventually, the system just uses the coordinates from the collection, instead of waiting on / depending on / an api call.
The logging happens in a different function call.
I recall seeing a post about async / await and promises from @robfallows a while back for the server side, so I think I may need to go look that up and see if I can make sure the query is done before I continue with the rest of the function.
See, as I cycle through the results of the query, I update the document as either geoCoded: true and / or geoCodable: false (seem the same, but slightly different for reasons in the UI again). This should, in theory allow me to eventually work all the way through the collection and complete the geo-coding of any / all addresses that can be geo-coded.
You will eventually touch each document for sure, but as you are discovering you’ll hit some of them multiple times. If you are sure you only call this method from the UI exactly once, then your race condition is something else, since you do call this method “recursively” async/await (or at a minimum, futures) is a good place to start - I never call a meteor method from inside another meteor method (I always extract common functionality) so I’m not sure if this is sync or not, but my guess would be that its async.
1 Like
Me too. However, Meteor.call on the server is sync (no callback) or async (with callback) depending on how it’s called. Even if it’s sync (which the code above suggests), there may be out-of-sequence running if this.unblock() has been used in the methods. I say “may” because I’ve never tried this particular variant!
It may be worth looking at using findOneAndUpdate() to ensure atomic operations on a document. For which, you will find it easier to use async/await syntax.
1 Like
The use of setTimeout will also cause it to possibly run out-of-order, as the method runs to completion and the actual work runs in a new separate context
The use of setTimeouts here is a bad sign in general, and removing them would both simplify the function and save you from bugs.
You could use Meteor._sleepForMs instead to keep the execution context alive
1 Like
Good spot. I totally missed that! That setTimeout is actually doing nothing except spawning a bunch of closures. It’s not actually delaying anything.