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.

Sign up for GitHub

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

Jump to bottom

Bug: processRecurrence is slow #9722

Closed
Dschoordsch opened this issue May 7, 2024 · 6 comments · Fixed by #10181
Closed

Bug: processRecurrence is slow #9722

Dschoordsch opened this issue May 7, 2024 · 6 comments · Fixed by #10181
Assignees
@mattkrick
Labels
bug p1 Needs to be addressed fast. Such an issue significantly impacts the majority of users

Comments

@Dschoordsch
Copy link
Contributor

Dschoordsch commented May 7, 2024 •
edited
Loading

The mutation takes >15s, see logs 🔒 We only run it every 5min so it's not an issue at the moment, but it should be faster.
Also testing processRecurrence times out sometimes https://github.com/ParabolInc/parabol/actions/runs/8982173873/job/24669192689?pr=9720
@Dschoordsch Dschoordsch added bug p2 Needs to be addressed on a reasonable timescale labels May 7, 2024
This was referenced May 7, 2024
Merged
Merged
Merged
@jordanh jordanh added this to Product May 20, 2024
@github-project-automation github-project-automation bot moved this to To triage in Product May 20, 2024
@Dschoordsch
Copy link
Contributor Author

RRule.between is the slow part, see traces 🔒

@Dschoordsch
Copy link
Contributor Author

The slowness comes from the time zone processing https://github.com/jkbrzt/rrule/blob/9f2061febeeb363d03352efe33d30c33073a0242/src/datewithzone.ts#L32
When processing all RRules locally (~1100) takes 18s with tzid and only 1s when stripping tzid.
Also RRules need to always iterrate from the start to the current between args, so it will only get worse over time.

@mattkrick
Copy link
Member

what in the world is that package doing internally?!?!
this is the same package that crashed our servers by creating an infinite loop 🤣

@mattkrick
Copy link
Member

Elevating to P1 since this is a synchronous call & it lasts 23 seconds, which would cause any requests that hit the same server to timeout

@mattkrick mattkrick added p1 Needs to be addressed fast. Such an issue significantly impacts the majority of users and removed p2 Needs to be addressed on a reasonable timescale labels Jul 11, 2024
@mattkrick
Copy link
Member

https://github.com/lsndr/rrule-rust

@mattkrick mattkrick moved this from Backlog to In progress in Product Aug 30, 2024
@mattkrick
Copy link
Member

this is basically crashing every request that comes in while processRecurrence runs.
Gonna focus on this

@mattkrick mattkrick mentioned this issue Sep 6, 2024
Merged
3 tasks
@github-project-automation github-project-automation bot moved this from In progress to Done in Product Sep 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mattkrick mattkrick

Labels
bug p1 Needs to be addressed fast. Such an issue significantly impacts the majority of users
Projects
Product
Status: Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: move to rrule-rust ParabolInc/parabol
2 participants
@mattkrick @Dschoordsch