Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle edge cases for EvidenceSubmissionWindowTasks #15598

Merged
merged 4 commits into from
Nov 23, 2020

Conversation

hschallhorn
Copy link
Contributor

@hschallhorn hschallhorn commented Nov 10, 2020

Resolves #15245 and hopefully squash all those "incomplete and pending task timer" alerts

Description

The problem: Timers can be created with a date in the past, wild!

Current solution: Our "complete the timers" job that runs every hour will look for timers that should have been completed any time in the last four days!

"But what about timers that should have ended over four days ago?" you might ask: When we create a timer that should have been completed over 4 days ago, we immediately reset the timer to be picked up by the job on its next run!

"So what is the problem?" you may then ask: On occasion, timers are created that should have been completed 3.79 days ago, so they are not caught in the initial check. But by the time the job runs (could be up to an hour later), they are no longer inside of that 4 day window and are not completed by the job.

This PR adds a 1 hour buffer to that initial check so we take into account tasks that will slip out of the 4 day window by the next time the job is run.

Acceptance Criteria

@hschallhorn hschallhorn self-assigned this Nov 10, 2020
@@ -205,7 +206,7 @@ def expired_without_processing?
last_submitted = self[self.class.last_submitted_at_column]
return false unless last_submitted

last_submitted < self.class.requires_processing_until
last_submitted < (self.class.requires_processing_until + self.class::EXPIRATION_WINDOW_BUFFER_HOURS.hours)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Called here:

timer.restart! if timer.expired_without_processing?

ensures we catch any timers that will have slipped outside of this 4 day window by the time the next job is run

# so they are not caught by the initial expired_without_processing? check when they are created. However, by the
# time the job has run, the task should have expired 4.000001 days ago and falls out of our
# expired_without_processing scope. See https://github.com/department-of-veterans-affairs/caseflow/issues/15245
4.days.ago + 30.minutes
Copy link
Contributor Author

Choose a reason for hiding this comment

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

weird time stuff math. + 30.minutes moves the completion time of this task to just under 4 days ago (3.979 days ago to be exact)

@codeclimate
Copy link

codeclimate bot commented Nov 10, 2020

Code Climate has analyzed commit e50b415 and detected 0 issues on this pull request.

View more on Code Climate.

Copy link
Contributor

@ajspotts ajspotts left a comment

Choose a reason for hiding this comment

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

Wow great catch & adjustment here 🤩

@hschallhorn hschallhorn added the Ready-to-Merge This PR is ready to be merged and will be picked up by va-bot to automatically merge to master label Nov 23, 2020
@va-bot va-bot merged commit e84713e into master Nov 23, 2020
@va-bot va-bot deleted the hschallhorn/15245-task-timer-edge-cases branch November 23, 2020 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready-to-Merge This PR is ready to be merged and will be picked up by va-bot to automatically merge to master
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Handle EvidenceSubmissionWindowTasks with last_submitted_at edge cases
3 participants