Promise.all has real world advantages when it comes to IO wait or DB query operations, where you are waiting and not doing anything - It's super easy to demonstrate too when tracing a request that uses Promise.all VS a normal for-each loop. With an await for-each, the execution would wait for a database query to resolve before processing with the next one, causing the execution time to be way higher.

In our situations, the database/cache queries are inside classes, with each class objects sometime requiring data that is shared with other objects. The previous callbackFiller function handled that use case perfectly for us and would coalesce those identical cache.wrap().

I agree that in compute-only cases, Promise.all actually slows things down with no real benefits, since you won't get more CPU cycles to do those compute tasks, and Promise.all will just add more overhead.

So I understand you're not interested in re-implementing this and we should look at implementing this outside of the caching library?

What does Rust multithreading have to do with this issue?

Unhelpful responses IMO

It's to explain that only one call is made at a time per thread.

But Rust is not NodeJS, is still doesn't make sense to use that as an example.

Therefore, if a parallel call occurs, it is because it has been called like this, calling the same function twice does not make sense in the same call. The only way to match 2 equal queries to the database is if they are called from different threads. This can't be since nodejs is a single threaded runtime.

That is simply not true. NodeJS has indeed a single threaded event loop, but the asynchronous nature of NodeJS allows it to process other things while waiting for an event/callback. That means that it is 100% possible to process multiple database queries at the (almost) same time while the previous one(s) is still not resolved.

That is a trace on my current NodeJS application, where I do a Promise.all on a multi-cache wrap using the current version. See how I'm able to effectively do start a bunch of queries while the first one is not even completed, and hasn't got it's result back yet :

If we don't coalesce the cache calls, we end up calling the cache database multiple times for the same exact query because we are indeed calling that cache multiple times before it's completed. It's especially relevant when you use a multi-layer-cache approach and use a memory cache. You would expect to only do a single query to the database, and then serve the object from memory, but what happens is just a bunch of queries to the database.

Yes, the original reason for this was to handle a scenario like:

1. 10 simultaneous requests are received for user 123, which isn't cached yet.
2. Instead of trying to fetch the same key from the cache 10 times in parallel, and have each one miss and then have to fetch from the database, we instead put the requests in a "callback filler" queue, so only one request actually calls the cache and database.
3. When the single request comes back, it sends the data back in its callback and each of the other 9 callbacks.

So the whole point is to avoid trying to make parallel calls to the cache or database for the same key.

@Maelstrom96 My bad, you are right.

Hi @jaredwray, I see this issue has been closed but we still experience this behavior change in v5.2.3. We recently upgraded from v4.1.0.

As @Maelstrom96 described originally, without the "locking" or "coalescing" functionality of the CallbackFiller from the v4.x line, then when multiple calls to wrap for the same key occur close together then all those requests have their generator function invoked, which pummel databases and downstream apis.

Does the team intend to bring this functionality back or is this issue closed as "won't fix"?

Thanks!

@douglascayers - thanks for the response and at this time we do not plan to re-implement this. I am more than happy to look at a pull request around this.

@douglascayers - thanks for the response and at this time we do not plan to re-implement this. I am more than happy to look at a pull request around this.

Thanks for the quick follow up! I'm working on porting the logic into our app, and would be happy to submit a pull request for your all's consideration for this project.

That would be great. I would also love to see some unit tests that replicate this so we know we have a workable solution.

✅ Step 1: create https://www.npmjs.com/package/promise-coalesce

✅ Step 2: create pull request to update the wrap function to use the coalescing logic