Support max_concurrency for running Looker queries

[iserko]

Situation was that once we ran Spectacles on our Looker instance
with 128 explores in our model, it killed the instance in a way that
it stopped responding and eventually crashed. I've diagnosed that it
was merely the number of running queries at a time that caused it.
It scheduled pretty much all 128 queries immediately.

So this adds a flag --max-concurrency which you can use to tweak
and set how much load you want to put on your Looker instance.

I've tried our staging server with 10 and it works nicely and doesn't
crash anything.

Of the things I did:

• made the validate SQL function async and moved the loop code in the parent function
• got rid of creating an asyncio loop as I don’t believe you need it (get_event_loop creates it if one does not exist)
• when you specify max_concurrency the code will only create <max_concurrency> queries as will not queue up more until the previous ones are finished

[codecov]

Codecov Report

Merging #110 into master will decrease coverage by 0.97%.
The diff coverage is 31.74%.

@@            Coverage Diff             @@
##           master     #110      +/-   ##
==========================================
- Coverage   66.89%   65.92%   -0.98%     
==========================================
  Files           9        9              
  Lines         716      719       +3     
==========================================
- Hits          479      474       -5     
- Misses        237      245       +8

Impacted Files	Coverage Δ
spectacles/client.py	58.33% <25%> (+1.29%)	⬆️
spectacles/validators.py	56.04% <30.9%> (-1.59%)	⬇️
spectacles/runner.py	47.36% <50%> (ø)	⬆️
spectacles/cli.py	77.39% <50%> (+0.19%)	⬆️
spectacles/exceptions.py	52.17% <0%> (-26.09%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e875b3c...bd289f7. Read the comment docs.

[joshtemple]

Thanks @iserko, this is really awesome and will be extremely important so people don't overload their instances. This is my first time writing async code in Python, so would love it if you could elaborate on the general approach here and if/why it's better so I can learn.

I want to benchmark this a bit because I have a hunch it will be faster, but I'm getting an error with --max_concurrency 0. Added a few more comments as well.

[joshtemple]

It would be good to set the default to a higher number (10? 20?) so that a user who doesn't read the documentation avoids maxing out their instance accidentally. Better to provide a performance benefit for an informed user than a performance risk for an uniformed one. Open to suggestions on what that number should be.

[joshtemple]

Let's keep the variable name consistent throughout.

[joshtemple]

When I run this with max_concurrency = 0, I get stuck in this loop forever and queries are never launched. query_task_ids is empty as defined above, so it continually waits for non-existent query task IDs to complete. Any idea what's going on here?

I wonder if you need to change if len(query_task_ids) >= max_concurrency: to a >?

[DylanBaker]

One thing I realised here is that I think we lose the 250 limit on fetching queries? I can test this later on a big enough explore, but I think we will get errors when fetching the queries if it does over 293 or whatever that number is?

[joshtemple]

@iserko, inspired by your PR, I did a bunch of digging on asyncio and found some ways to improve this approach.

A few major changes:

• I've added a asyncio.BoundedSemaphore object to the validator which is used to block when the maximum number of queries are running against the data warehouse (releasing a slot once we get the result back)
• I've used an asyncio.Queue object to handle tracking of the running queries. This allows us to periodically check for query results as a coroutine as well, pulling all Looker query task IDs, extracting results, and pushing the running or pending queries back onto the Queue.
• I renamed --max-concurrency to --query-slots, which I think is a more intuitive name for the flag. I set the default to 10. Interested to hear your and @DylanBaker's thoughts on this though, the name is open for discussion for sure.

Let me know what you think of these changes.