796743 - [calendar] Recurring Events will not expand beyond 6 months or 500 instances.

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::Calendar, defect, P1)

Description

[GH to BZ]

[GitHub issue by lightsofapollo on 2012-09-26T07:07:41Z, https://github.com/mozilla-b2g/gaia/issues/5201]
Right now recurring events only get expanded when the event is initially added, we expand up to 1000 occurrences or 1 year from the current date.

Obviously this won't scale well. The api's for processing recurring information are all in place but need to be hooked up to the db and we need to re-arrange the component data for better performance/memory management.

Solution:

1. Exceptions don't need their own copy of component data create new ProviderItem store to store
   the raw data of the ical components (right now these are called remote.icalComponent in each event)

2. Utilize the serialized recurrence iterator.

2.1. Create Recurring store put serialized iterator there with last expanded time.
       if event expansion is complete (noted by the recurring event event from the caldav service)
       remove the item from the store as it is now completely expanded.

2.2. Create RecurringService which will (anytime we move the calendar or sync) check if the last expansion is _before_ the current position (or some other related date) in the calendar if so then trigger another expansion of the recurrence rules using the stored iterator (call expandOccurrences in the CaldavProvider with the iterator option). Repeat until there are no more recurring rules that lag behind the current position in the calendar.

Comment 1

[GH to BZ]

[GitHub comment by lightsofapollo on 2012-09-26T07:09:46Z]
@autonome We absolutely need this for v1 but I would like to punt this out until weekend/next week so I can work on some user facing stuff and smaller bugs. This is about another day of work but will add no major difference to a end user aside from if they want to view a recurring event 12 months in the future (assuming it started sometime in the past).

Comment 2

[James Lal [:lightsofapollo]]

Attachment: Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/6146

Pointer to Github pull-request

Comment 3

[Jason Smith [:jsmith]]

I take it the way to test this is to use a bunch of reoccurring events in some imported calendars and see how well we do memory wise.

Btw - During the work week next week, I intend to lock myself into a room and burn through a lot of these calendar bugs. So expect activity next week.

Comment 4

[James Lal [:lightsofapollo]]

Sorry this patch was 1 of 2, the bug is not yet closed.

Comment 5

[Michał Budzyński (:michalbe)]

Oh, I'm sorry, I've checked that it works and I thought that it was all. My  mistake.

Comment 6

[Dietrich Ayala (:dietrich)]

Been nearly a month - what else needs to happen for this to close?

Comment 7

[James Lal [:lightsofapollo]]

P3 = bottom of queue, we implemented the refactoring pieces here but the incremental expansion is still incomplete. All the supporting pieces are done/landed over the last month. Once P1/P2 are complete we will complete this bug.

Comment 8

[Alex Keybl [:akeybl]]

Moving to P4/b-b- until we have reproducible steps where this becomes a major problem. Adding qawanted to see if we can find that instance, in which case please re-nominate for b-b. Until that point, this is mostly speculative.

Comment 9

[James Lal [:lightsofapollo]]

I have renamed the bug to reflect the real problem here. (Which should make it much easier to re-triage)

Comment 10

[Jason Smith [:jsmith]]

Renoming based on new information to see what happens in triage.

Comment 11

[Jason Smith [:jsmith]]

I'm going to guess a way to reproduce this bug basically would be creating a reoccurring event that goes on forever in a google or yahoo account. Then, import the calendar and observe the reoccurring event over a year's timeframe.

Comment 12

[James Lal [:lightsofapollo]]

Yes, this is easily verifiable with the calendar data you provided me.

Comment 13

[Alex Keybl [:akeybl]]

Mass Modify: All un-milestoned, unresolved blocking-basecamp+ bugs are being moved into the C3 milestone. Note that the target milestone does not mean that these bugs can't be resolved prior to 12/10, rather C2 bugs should be prioritized ahead of C3 bugs.

Comment 14

[Faramarz Rashed]

hey james... what's the progress being made here?  do you have an update?

Comment 15

[James Lal [:lightsofapollo]]

No progress made directly here. With the timezone changes we also have improved recurring event expansion time (by 7-10x) that work is done in the ical.js repo.

Those changes in addition to the incremental logic we will implement here should dramatically reduce overall sync time.

Comment 16

[Lucas Adamski [:ladamski]]

Is this still basecamp blocking?  Not sure what specific issue this will resolve for the 1.0 timeframe that we couldn't address in a subsequent release (i.e. if the calendar is valid for a year out then we could fix this in the 1.1. timeframe, right?)

Comment 17

[Jason Smith [:jsmith]]

(In reply to Lucas Adamski from comment #16)
> Is this still basecamp blocking?  Not sure what specific issue this will
> resolve for the 1.0 timeframe that we couldn't address in a subsequent
> release (i.e. if the calendar is valid for a year out then we could fix this
> in the 1.1. timeframe, right?)

Yes. Well for one, if you create a reoccurring event that goes indefinitely in an imported calendar 6 months ago, then it won't work correctly when you import it.

This would also be the key reason why my personal calendar doesn't work right now :(.

Comment 18

[James Lal [:lightsofapollo]]

This is not too difficult and (IMO) is critical to complete prior to v1.

We will have significant performance gains by completing this bug as the method to fix the issue is incremental expansion. Without this we will have missing calendar events... A good example is a recurring event that started 3 years ago we will never show any of those events to the user.

I would suggest re-prioritizing this as a P1 so we can knock this out.

Comment 19

[Dietrich Ayala (:dietrich)]

Driver retriage: Perceived dataloss, blocking+ P1.

Comment 20

[Dylan Oliver [:doliver]]

Please provide an update on the current status of this bug.

Comment 21

[James Lal [:lightsofapollo]]

Should be in review tonight wrapping up the "THISANDFUTURE" bug first so we can do one final DB schema change before the end of c3

Comment 22

[James Lal [:lightsofapollo]]

Attachment: Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/7305

Pointer to Github pull-request

Comment 23

[James Lal [:lightsofapollo]]

Attachment: Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/7305

Pointer to Github pull-request

Comment 24

[James Lal [:lightsofapollo]]

Comment on attachment 677686 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/6146

This patch includes the ICAL code you reviewed.

I need to test this out on the wifi later but this is ready for initial review.

Comment 25

[James Lal [:lightsofapollo]]

OK, there where a few more changes needed and now that those are done this is ready to go. There are various performance improvements here but there is probably more work to be done but it seems acceptable on my calendar test cases now.

Comment 26

[James Lal [:lightsofapollo]]

https://github.com/mozilla-b2g/gaia/commit/73bf017bb0e399484bf6d38628d466f83bcd1665

Comment 27

[Jason Smith [:jsmith]]

Verified through a sanity check on my deadly gmail calendar - the reoccurring events looked okay.