Skip to content

Increase report timeout, mark as long running#5544

Merged
zachmargolis merged 1 commit intomainfrom
margolis-long-running-reports
Oct 26, 2021
Merged

Increase report timeout, mark as long running#5544
zachmargolis merged 1 commit intomainfrom
margolis-long-running-reports

Conversation

@zachmargolis
Copy link
Contributor

I had to re-run the Reports::AgencyInvoiceIaaSupplementReport report (since it has been failing out), and this was the config that I needed to get it there. A few queries took close to 100,000ms, but the when I set the timeout to 100k, queries still got canclled

A more ideal fix would obviously be to optimize the underlying queries, but I think this is acceptable in the meantime:

  • the queries run against a read replica
  • they're already run in background jobs
  • we've been missing reports for weeks/months

Also updates the report base class to be the long_running queue, because that means we won't alert on long running jobs for it... because they are expected to take a long time

recurring_jobs_disabled_names: "[]"
redis_throttle_url: redis://redis.login.gov.internal:6379/1
redis_url: redis://redis.login.gov.internal:6379
report_timeout: 1_000_000
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was suprised to learn that this is valid YAML:

require 'yaml'
YAML.load <<-STR
some_key: 1_000_000
STR
# => {"some_key"=>1000000}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also valid Ruby 🙂

irb(main):001:0> 1_000_000
=> 1000000

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes! and valid Java these days too I just didn't realize YAML was also in the underscores club

module Reports
class BaseReport < ApplicationJob
queue_as :low
queue_as :long_running
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great 👍🏼 👍🏼 👍🏼 👍🏼 👍🏼

recurring_jobs_disabled_names: "[]"
redis_throttle_url: redis://redis.login.gov.internal:6379/1
redis_url: redis://redis.login.gov.internal:6379
report_timeout: 1_000_000
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also valid Ruby 🙂

irb(main):001:0> 1_000_000
=> 1000000

@zachmargolis zachmargolis merged commit c8c1de7 into main Oct 26, 2021
@zachmargolis zachmargolis deleted the margolis-long-running-reports branch October 26, 2021 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants