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

fix: Increase process recurrence timeout #9665

Merged
merged 1 commit into from Apr 25, 2024

Conversation

Dschoordsch
Copy link
Contributor

Fixes #9664

We know this might be long running, so adding a separate long running timeout. We're not reusing the adhoc timeout as this is excessive and might obfuscate issues in the query.

Testing scenarios

  • add debug output to
    const timeout = isAdHoc ? ADHOC_TIMEOUT : longRunning ? LONG_TIMEOUT : STANDARD_TIMEOUT
  • add CHRONOS_PROCESS_RECURRENCE='* * * * *' to your .env
  • run yarn build && yarn start
  • check the output

Final checklist

  • I checked the code review guidelines
  • I have added Metrics Representative as reviewer(s) if my PR invovles metrics/data/analytics related changes
  • I have performed a self-review of my code, the same way I'd do it for any other team member
  • I have tested all cases I listed in the testing scenarios and I haven't found any issues or regressions
  • Whenever I took a non-obvious choice I added a comment explaining why I did it this way
  • I added the label Skip Maintainer Review Indicating the PR only requires reviewer review and can be merged right after it's approved if the PR introduces only minor changes, does not contain any architectural changes or does not introduce any new patterns and I think one review is sufficient'
  • PR title is human readable and could be used in changelog

We know this might be long running, so adding a separate long running
timeout. We're not reusing the adhoc timeout as this is excessive and
might obfuscate issues in the query.
@rafaelromcar-parabol
Copy link
Contributor

I'm testing in an on-demand-env and I'm still seeing some error 🤔

  • Namespace and branch names are od-chronos-recurrence.
  • Chronos is running and has the following in the logs
Trace: SEND TO SENTRY TIMEOUT {"query":"\n          mutation ProcessRecurrence {\n          "} undefined                                                                                                           
    at sendToSentry (/home/node/parabol/dist/chronos.js:17376:11)                                                                                                                                                  
    at publishWebhookGQL (/home/node/parabol/dist/chronos.js:17347:28)

Docker image is built using our Github Action.

@Dschoordsch
Copy link
Contributor Author

Can I somehow see the datadog logs of that instance?

@rafaelromcar-parabol
Copy link
Contributor

We do not have Datadog logs on that environment to save costs. You can see the logs using k9s! You have already do that in the past I think!

@Dschoordsch
Copy link
Contributor Author

That doesn't show me the graphql traces sadly.

@rafaelromcar-parabol
Copy link
Contributor

You could also use Google Console logging here for example

@Dschoordsch
Copy link
Contributor Author

It seems the job runs every second, that's a bit much. I suggest setting it to run every minute at most and then we can check again.

@rafaelromcar-parabol
Copy link
Contributor

Do the changes yourself on the on-demand-env branch, apply using the pipeline and test!

@Dschoordsch
Copy link
Contributor Author

Seems to work just fine with the lower frequency.

@Dschoordsch Dschoordsch merged commit f4e0cda into master Apr 25, 2024
7 checks passed
@Dschoordsch Dschoordsch deleted the fix/9664/increaseProcessRecurrenceTimeout branch April 25, 2024 10:52
@github-actions github-actions bot mentioned this pull request Apr 25, 2024
24 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Chronos error on mutation ProcessRecurrence
2 participants