Add concurrency limit to QueryRunner and DataSourceSettings

[Kevin-Chant]

Features and Changes

Creates a new field in DataSourceSettings: maxConcurrentQueries. This field is respected by QueryRunner's startQuery and startReadyQueries entrypoints, which now check the number of running queries before committing to starting a new query.

Added a helper to QueryModel to grab the count of running queries & a new section to the ConnectionSettings modal to allow for editing of this setting (and create a place for other future shared settings to live).

Note: the limit is only respected across multiple experiments (i.e. one experiment which runs N queries where N is over the concurrency limit will not trigger the protections) due to race conditions with queries starting simultaneously.

Testing

1. Set up at least one experiment which makes multiple queries
2. Edit the settings for the datastore and provide a concurrency limit (probably 1)
3. Open the datastore queries list page in a new tab
4. Force update the results for the experiment(s) and check the queries list page to see that some have been queued.
   a. (If the delay isn't noticeable, you can change the default currentTimeout: number = 500 to 8000.)

Screenshots

Here's a screen recording of the UX with a concurrency limit of 1

And here's a corresponding recording of what's happening on the backend with a few extra console.logs added for clarity.

[github-actions]

Your preview environment pr-2513-bttf has been deployed.

Preview environment endpoints are available at:

• https://growthbook-3000-pr-2513-bttf.growthbook.okteto.dev
• https://growthbook-3100-pr-2513-bttf.growthbook.okteto.dev

[jdorn]

In startExperimentResultQueries, we currently start all queries at once and do a Promise.all. This is causing the race condition you mention in your PR description.

We can fix this fairly easily by changing it to await each promise before starting the next query.

It's really only the experiment query that has this issue since all of the other ones fire 1 (or maybe 2) queries total.

[jdorn]

One thing I can see happening is someone's experiment being stuck in a queued state because all of the slots are being hogged by a few super long running queries.

What would be really cool is to pair this with #2323 so you can easily see a list of all of your running queries and where they are coming from and maybe even kill them directly from that list.

[Kevin-Chant]

One thing I can see happening is someone's experiment being stuck in a queued state because all of the slots are being hogged by a few super long running queries.

What would be really cool is to pair this with #2323 so you can easily see a list of all of your running queries and where they are coming from and maybe even kill them directly from that list.

This is one thing I was thinking about; didn't know that we have a page for that now! Do you think showing a link to that page while data is updating would do a good enough job at explaining what's happening?

[jdorn]

Yeah, in AsyncQueriesModal, if there are any queries that are "queued", we can show an alert banner that links to the datasource queries list page.

The page isn't super useful right now (it doesn't link to the experiment that spawned the query and there's no way to cancel), but it's still helpful and we can improve it over time.

[Kevin-Chant]

In startExperimentResultQueries, we currently start all queries at once and do a Promise.all. This is causing the race condition you mention in your PR description.

We can fix this fairly easily by changing it to await each promise before starting the next query.

It's really only the experiment query that has this issue since all of the other ones fire 1 (or maybe 2) queries total.

Makes sense. I tried rewriting the promise logic in ExperimentResultsQueryRunner but ran into some problems where the queries weren't being set up properly (erroring out with Run callbacks not found). I'm leaning towards just shipping it as-is since it seems unlikely that a single experiment will overload the db so we should get most of the benefit even with this edge case exceeding the concurrency limit. Does that sound good to you?

[Kevin-Chant]

The ES6 for loop format allows for awaiting within each iteration of the loop without going to the next, so we can drop the concurrency granularity to 1

[Kevin-Chant]

This is definitely an optional addition but I found myself wanting to check the queued time after re-running the queries so I thought I'd toss it in here. Let me know whether this seems useful to keep or if I should get rid of it.

[Kevin-Chant]

@jdorn I fixed the issue I had before & the queries now go one at a time. I updated the gifs with the new functionality and dropped the wait time by half because it's much more impactful now with a low concurrency limit. Should be ready for a review again once you're done with the 3.0 stuff