mail icon indicating copy to clipboard operation
mail copied to clipboard

feat: implement periodic full sync job to repair cache inconsistencies

Open st3iny opened this issue 1 year ago • 1 comments

Run a background job once a week without any Horde cache to forcibly disable QRESYNC support. This should cause the UID list to be be rebuilt and synced from scratch.

Needs some more testing though.

st3iny avatar Aug 21 '24 16:08 st3iny

TODO

  • [x] Add an entry to a the context menu to run this action from the frontend

st3iny avatar Aug 26 '24 13:08 st3iny

The menu looks like a candidate for a design review ;)

Even without clear cache, because that's hidden unless debug enabled, it's cluttered. And "subscribed" is a bit lost :rofl:

image

Do you think we should limit the repair command for a mailbox to run once per 10 minutes or so? (we could use the UserRateLimit attribute for it).

It was rather quick even with my larger mailbox.

kesselb avatar Sep 25 '24 10:09 kesselb

Do you think we should limit the repair command for a mailbox to run once per 10 minutes or so? (we could use the UserRateLimit attribute for it).

Yeah, that is probably a good idea. Although I wasn't sure how to reflect this in the frontend. Disable the button while the rate limit is active via setTimeout()?

It was rather quick even with my larger mailbox.

I was able to confirm this using a very large test mailbox as well. The request itself should not cost that much processing time. It's comparable to a regular sync and we don't have a rate limit there.


PS: I fully agree about the menu desperately requiring a redesign :laughing:

st3iny avatar Sep 30 '24 07:09 st3iny

/backport to stable4.0

st3iny avatar Sep 30 '24 08:09 st3iny

Yeah, that is probably a good idea. Although I wasn't sure how to reflect this in the frontend. Disable the button while the rate limit is active via setTimeout()?

With the UserRateLimit attribute, the request will fail with a different status code. It would be nice to show a toast in that case, "hello, please wait 60 minutes before the next repair run". I don't think it's necessary to check that right away when building the menu or disable the button.

Would it be possible to disable the button or show a loading animation while the request is running?

kesselb avatar Sep 30 '24 09:09 kesselb

I'll add a rate limit annotation and add a toast.

Would it be possible to disable the button or show a loading animation while the request is running?

The button is disabled during the repair. I didn't add a loading animation though.

st3iny avatar Sep 30 '24 10:09 st3iny

Hmm, on second thought, this will limit repairing multiple distinct mailboxes. I ajdusted the rate limit to 3 attempts every 10 minutes.

st3iny avatar Sep 30 '24 11:09 st3iny

Hmm, on second thought, this will limit repairing multiple distinct mailboxes. I ajdusted the rate limit to 3 attempts every 10 minutes.

Sure! 10 attempts in 10 minutes is also okay. It's just to have a minimal protection against abusing the feature.

kesselb avatar Sep 30 '24 11:09 kesselb

I was able to confirm this using a very large test mailbox as well. The request itself should not cost that much processing time. It's comparable to a regular sync and we don't have a rate limit there.

Sorry, I overread that part. Also, okay for me to not apply a user rate limit if you think we don't need it.

kesselb avatar Sep 30 '24 11:09 kesselb

I'll add a more lax rate limit and then this is done.

st3iny avatar Sep 30 '24 14:09 st3iny

Sorry to ask, but is it intended that everyone has the ability to approve changes?

pabloeisenhut avatar Oct 01 '24 08:10 pabloeisenhut

Sorry to ask, but is it intended that everyone has the ability to approve changes?

Yes, that is a GitHub feature. However, we have branch protections in place that enforce approvals by developers with write permissions before merging. That is why your check mark is shown greyed out.

st3iny avatar Oct 01 '24 08:10 st3iny

Sorry to ask, but is it intended that everyone has the ability to approve changes?

Yes, that is a GitHub feature. However, we have branch protections in place that enforce approvals by developers with write permissions before merging. That is why your check mark is shown greyed out.

Oh, perfect, I was wondering, thanks for the clarification! :)

pabloeisenhut avatar Oct 01 '24 18:10 pabloeisenhut

/backport to stable3.7

st3iny avatar Oct 02 '24 09:10 st3iny