Some help please

[wasab]

8 minutes ago, Eigenvektor said:

Oh, I see, you need items not users, and for each item you need the associated user? Then I suppose a Join would be your best option.

 

In any case, your problem right now is that you replaced the MongoDB "find" with a JavaScript "find", which is very slow: https://nikitahl.com/how-to-find-an-item-in-a-javascript-array/. You haven't reduced the number of queries and added more overhead by repeatedly searching through the results.

That makes sense. I currently have 4,000 users and around 10k items in my database thanks to grading script spamming requests. I imagine that's why the latency jumped suddenly from 1-4k ms all the way to 25kish. 

[Eigenvektor]

2 minutes ago, wasab said:

That makes sense. I currently have 4,000 users and around 10k items in my database thanks grading script spamming requests. I imagine that's why the latency jumped suddenly from 1-4k ms all the way to 25kish. 

I'm not really familiar with JavaScript, but couldn't you convert the user-list to a hash map? In case a join is not an option, I'm thinking about something like this:

let userIds = items.map(i => i._userId);
let users = await User.find({ _id: { $in: userIds } });
let userByKey = // create a hashmap that contains id => user;

let responseItems = [];
for (let i = 0; i < items.length; i++) {
	let current_item = items[i];
	let current_item_user = // get user from hashmap by id;

 

[A51UK]

What about added a cache layer between the database and your app?  On the first request you could load most or all of the user in RAM (cache) if only small amount of users, every other request just read from the cache not the database. This would lower I/O cost. The first request will be the slower than ever other request.  When you add a new user you update the cache. 

[camjocotem]

6 hours ago, Eigenvektor said:

You still have multiple queries this way and I/O is expensive. Plus, you now have the overhead of searching through your list every iteration with:

userList.find(user => user._id.equals(current_item._userId));

which is most likely even slower than:

await User.findOne({_id: current_item._userId});

So you've essentially traded one query each iteration with all the queries upfront and one lookup each iteration.

 

Doing multiple queries upfront doesn't net you much, if you have to wait until they're all done, because you're essentially back to serial operation. Plus, as I said, you're now iterating over your list of users each iteration to get the user from your list instead of the database.

 

I would definitely try the single-query variant and, if possible, combine it with the sort operation directly.

While it's true I/O is expensive, NodeJS uses a dedicated thread for I/O operations, and MongoDB is hella fast.

 

 

My gut feeling is it could have something to do with how the connection is managed?

MongoDB wants to initialise the connection once at app startup and have you reuse the same db object.

 

Behind the scenes are you opening/closing connections for each request?

 

(For this scenario with users speed-wise I would definitely choose $in over running individual queries but I didn't want to fundamentally change the way you do your code )

[camjocotem]

Additionally, if there wouldn't be many items per user, you can embed the Items into the User documents.

I don't know the scenario but that is an option

 

Denormalisation with mongo is actually encouraged

[Eigenvektor]

50 minutes ago, camjocotem said:

While it's true I/O is expensive, NodeJS uses a dedicated thread for I/O operations, and MongoDB is hella fast.

 

 

My gut feeling is it could have something to do with how the connection is managed?

MongoDB wants to initialise the connection once at app startup and have you reuse the same db object.

 

Behind the scenes are you opening/closing connections for each request?

 

(For this scenario with users speed-wise I would definitely choose $in over running individual queries but I didn't want to fundamentally change the way you do your code )

Sure, MongoDB is fast, but a single query is still going to beat multiple queries. Plus, you've replaced an indexed lookup of a user with a search through an array, which is extremely slow.

 

Initial solution

for(let i = 0; i < items.length; i++){
	let current_item_user = await User.findOne({_id: current_item._userId});

One database lookup per item. Assuming the database is indexed, this should have O(log n) complexity. Since multiple items can have the same user, you're needlessly querying the same user multiple times.

This solution is known to take about 1,700 ms

 

Promise solution

items.forEach((item) => {
	promiseList.push(User.findOne({_id: item._userId}));
})

let userList = await Promise.all(promiseList);

for (let i = 0; i < items.length; i++) { 
	let current_item_user = userList.find(user => user._id === current_item._userId);

Still one database lookup per item. Requests now happen asynchronously, which should improve performance slightly. However, it now uses array.find to get the user from the resulting in-memory array, which is extremely slow. For each iteration, you're now iterating over all users, which is O(n), to find the one that you need, which is a lot slower than simply querying the user in the first place.

This solution is known to take about 25,000 ms (i.e. significantly worse than initial solution)

 

Single query with hash map

let userIds = items.map(i => i._userId);
let users = await User.find({ _id: { $in: userIds } });
let userByKey = // create a hashmap that contains id => user;

let responseItems = [];
for (let i = 0; i < items.length; i++) {
	let current_item = items[i];
	let current_item_user = // get user from hashmap by id;

Single query to get all users in one go. Overhead to create a hash map as a lookup table. Overhead to get user from hash map, but should be less than a lookup in an array.

 

Single query with join

This should get exactly the data you need, without the need for additional in-memory lookup tables etc. This should perform best, but unknown how to achieve so far.