Reduce running time for Quidel indicator

[krivard]

The omicron signals appear to have substantially increased the running time for the Quidel indicator. Here's a plot of the running time for the Quidel indicator over the last 30 days, relative to the other indicators. The dotted threshold line is at 1 hour.

Because we run acquisition once an hour, indicators that take longer than an hour to complete delay the acquisition schedule no matter what; because typical non-empty acquisition takes between 7 and 37 minutes to run, indicators that take longer than 24 minutes may delay the acquisition schedule depending on how much data is in receiving.

This is not an emergency in itself, but each long-running indicator we have increases the likelihood that the sum total of scheduled tasks in Automation will exceed 24 hours, causing cascading delays and unpredictable results.

From a quick linear fit (data attached) it looks like Quidel running time is increasing by ~10s each day, so we've got a month or so before it actually exceeds an hour. Ideally we want to bring it down to ~18 minutes or less if we can.

Max runtime by indicator and date.csv

[rzats]

Just finished running profiling on the Quidel indicator and here's what the results look like!

On a smaller with a limited subset of data (2023-05-01 to 2023-06-01), the methods associated with the highest percentage of the indicator's runtime are as follows:

The bulk of the processing power here is taken up by the generate_sensor_for_parent_geo method, which calls a couple of other computation-intensive operations - add_megacounties, fill_dates and a dataframe merge; all of these rely heavily on Pandas computations.

On a larger run with the bulk of the data (2020-05-26 to 2023-06-01), however, the runtime distribution changes significantly:

At this point, the bulk of the runtime is taken not by actual data transformations but read_csv operations - in fact, they take up ~2/3 of the overall runtime (43 minutes out of the total 67). The actual S3 API calls that fetch the data from the input bucket only take 4.5 minutes. Additionally, the compute-intensive methods from the previous run, and their underlying Pandas operations, still take up the majority of the runtime.

I see a couple options that could be used to optimize this indicator:

• Switch from CSV files to another data format - as demonstrated in this blog, writing data to Parquet instead of CSV can be used to optimize further reads of the dataset significantly.

• Switch from Pandas to Dask - a parallelized alternative to Pandas with an API which is generally compatible with pd's, and better performance when it comes to reading files in particular, which appears to be this indicator's bottleneck.

Any thoughts? @krivard @neul3 @dshemetov

[melange396]

how did you run these? are those date ranges what you specified for the received dates filter on input files? if so, im not sure the profiles are necessarily representative, since a typical run only pulls for a single day. with that in mind, your one month analysis is likely to be a closer approximation than the three year. it looks to me like you might be able to save a lot of time by parallelizing the sensor generation and csv export steps.

[krivard]

@melange396 see Slack thread for additional context

[melange396]

so, good news and less good news!

good news:

total runtime appears to be effectively halved by #1881, from ~1400s to ~700s. dashboard metrics like validation alert counts and number of csvs exported are steady.

less good news:

there are a bunch of exceptions showing up in the logs. they are not fatal to the execution of the process, but they are causing elastalert to make big red complaints in the #system-monitoring slack channel (and maybe other alerts somewhere else too that i havent noticed yet?). they might also make it harder to sift through and find useful things in elastic, but thats probably not a big deal if this is only temporary. im surprised they didnt show up in @rzats's testing ¯\_(ツ)_/¯ ? regardless, this should be fixed by these lines in #1885.

[rzats]

@melange396 thanks for the update, and the issue report!

Does one of these exceptions show up for every thread which gets closed? I only see three of these in #system-monitoring, which doesn't seem to match the amount of CPUs on the runners.

Also, would you like to make & deploy a quick "hotfix" PR with these lines to suppress the alerts, or just wait for #1885 to be merged?

[melange396]

its one for each call to apply_async().

no need for a hotfix, as this isnt breaking anything and its not worth it to do a Friday release. hopefully #1885 gets merged before the release window next week, and the phenomenon should go away.

[melange396]

with the latest release, the exceptions are no longer being logged. feel free to close this unless theres something more youd like to do with it!