Fatal flaw in Arisa's "catching up" logic

[violine1101]

The Bug

Arisa has a feature where it "catches up" on all recent things that happened on Mojira after a downtime. However I believe the way some modules are written, they have the incorrect assumption that all changes in the ticket happened after lastRun.

Now, Arisa goes through the tickets by "updated" value. This means that it'll only look at the tickets that were last updated during the last run. Some modules assume that all changes before the last run have already been checked. For example, CommandModule has a check so that it only checks commands that have been updated after the last run.

It appears not all modules are affected by this though; however I did encounter this while testing a new module that I'm in the process of writing.

Consider this timeline:

• Arisa shuts off
• A comment with a command gets left on ticket MC-1234
• 20 minutes later, an attachment is added to the ticket
• Arisa is started again
• When Arisa sees the ticket, it won't act on the command comment because it's older than lastRun

To fix this, we'd need to store the time when we've seen the ticket for the last time, for every ticket. Not sure if that's worth it.

[SPGoding]

When Arisa sees the ticket, it won't act on the command comment because it's older than lastRun

I'm slightly confused. Shouldn't lastRun be a time before Arisa shuts down in this case, which means the command is going to be checked?

[violine1101]

Ah yeah, important for this is that the catching up happens in 10 minute intervals, meaning that Arisa will query the first 10 minutes of the downtime, then execute the modules on the returned tickets, then update last run, and then continue on with the next 10 minutes.
I guess changing that logic would be an easier solution, or splitting last run into "last run where bot was caught up" and "last run in total".

[NeunEinser]

I believe back when I worked with Arisa, there were no intervals. It probably was changed to not spam the tracker with requests or something?
In any case, last run should only be updated after all intervals have been successfully executed then, as that's what was the case when these modules were written and when I introduced lastRun.

[urielsalis]

+1 to only updating lastRun after catching up with all tickets
And I think storing the last time we seen each ticket is not a bad approach either, if we use a proper embedded DB for it (which I wanted to do for a while to move lastRun and similar there) it should be only 3mb for all tickets in the MC project. Make it store only a run number instead of a full timestamp and its even less