Proposal: Batching requests in an interval of time about dataloader HOT 12 CLOSED

If you add this, I recommend not making it time-based, but instead make it signal-based. Something like:

// mimic current behavior
new DataLoader(batchLoadFn, { batchSignal: () => Promise.resolve() });

// time-based batch
new DataLoader(batchLoadFn, { batchSignal: () => Promise.delay(10) });

// signal-based
new DataLoader(batchLoadFn, { batchSignal: () => waitForSomeSignalToArrive() });

// only wait 10ms for the first batch.
// Subsequent batches wait just one tick (to prevent cascading delays)
const batchSignal = Promise.delay(10);
new DataLoader(batchLoadFn, { batchSignal: () => batchSignal });

Effectively this would make the batching strategy injectable.

from dataloader.

DataLoader intentionally does not support this because it introduces a floor of latency to all loads which can quickly and unintuitively add up to very slow performance on servers.

Apollo client uses a delayed approach because it is used in an environment where it is common for load operations to be spread across many frames in a short window of time and for ticks to occur very often (~60fps clients), and the data they are loading (graphql queries) are not interdependent thus don't add up latency.

In contrast DataLoader is used in environments (servers) where load operations happen in bursts, where ticks only occur when scheduled, and are very often dependent on other load operations which results in the adding up of latency.

The primary concern is that if you were to define a 10ms interval, that any request to the loader would have to wait 10ms before actually sending. If you have a request dependent on the data of another request dependent on the data of an initial request, then you add up those intervals.

from dataloader.

I can work on a PR if the behavior is OK. I just ran into a situation where this would be desirable as well.

from dataloader.

I would consider a PR which added this, but I think it definitely needs real numbers to prove that it would improve performance and not hurt performance, both to warrant adding the feature and as future guidance of how others should use it.

from dataloader.

@leebyron I have a query-heavy (hence DataLoader-heavy) part of my app I can try this on, but I could really use a hint as to where I can swap out the logic & come back with some real metrics.

from dataloader.

Sure. enqueuePostPromiseJob (https://github.com/facebook/dataloader/blob/master/src/index.js#L208) is what is responsible for scheduling work at the end of the promise queue for the current tick. If you replaced the Promise-queue mechanisms with something like setTimeout you would get the time-based rather than tick-based behavior.

from dataloader.

In contrast DataLoader is used in environments (servers) where load operations happen in bursts, where ticks only occur when scheduled

Any suggestions how to trace down whats forcing a tick to happen?

I have this query:

{
  movies(date: "2017-04-06", coordinates: {latitude: 57.149911, longitude: -2.078433}, first: 3) {
    edges {
      node {
        venues(first: 10) {
          edges {
            node {
              events {
                id
              }
            }
          }
        }
      }
    }
  }
}

I am expecting all event records to be fetched in the same query, though instead I am getting hundreds of queries fetching 1-10 events per query.

from dataloader.

I suggest logging when your upstream promises are being created and when they complete. Sounds like a missed opportunity to batch in a prior set of promises is resulting in those resolving on different ticks and creating separate batches.

from dataloader.

For others going the perilous ways of debugging a similar issue: if you are using console.log to debug the issue then you must pipe the program output in order to ensure that it is non-blocking operation. That caused me a bit of confusion.

from dataloader.

this https://github.com/calebmer/graphql-resolve-batch could help

from dataloader.

I like the idea of @bman654 here #58 (comment):

I am using a pool of db connections and would like to collect batches as long as all db connections are busy, but as soon as a db connection gets released back into the pool the next batched query should/could start.

that should be doable by using the batchSignal: () => waitForSomeSignalToArrive() approach, right?

from dataloader.

Closing this issue since time-based intervals are counter to the goals of DataLoader (to reduce latency and load times), but I would definitely suggest building complimentary packages which have these more specific batching strategies. For example, DataLoader's batching function might itself call to an intermediate API that is more aware of the actual database connection state and able to perform its own delayed batching strategies that are optimized for its specific conditions.

from dataloader.