fix: conditions reset does not work if the service is down at the triggering time

[juliev0]

This fix uses the cron syntax specified in the Condition Reset to determine the last time the trigger should have fired prior to application start up. If any messages arrive with a timestamp prior to that time, they are acknowledged.

[whynowy]

Great work!

[juliev0]

Here's a description of how I tested this:

1. created a Sensor with a dependency of A && B && C and a Condition Reset and tested the following cases:
   1a. per the bug, I scaled the deployment down to 0, waited for the CR time to pass, then scaled it back up to 1 to verify that the reset occurred for all dependencies
   1b. I repeated the same test but not during the CR time to verify that no reset would occur
   1c. standard Condition Reset logic: cron fired and reset occurred for all dependencies (to verify I didn't break this)
   1d. standard logic: all 3 dependencies were sent (to verify I didn't break this)
2. also, to verify I didn't break anything I tried creating a Sensor with just one dependency and triggered it

[whynowy]

This should be protected by a lock, e.g. we could utilize setLastResetTime().

[juliev0]

okay, fixed this

[whynowy]

Same here.

[whynowy]

Can we share the same lock and simply call setLastResetTime(0) instead of setRestTimeout(0)? It should be safe to set lastResetTime to 0, right?

mh.lock.Lock()
defer mh.lock.Unlock()
mh.lastResetTime = t
if t == 0 {
  mh.resetTime = 0
} else {
  mh.resetTime = t + 60
}

[juliev0]

No, one of the changes I made as part of this whole thing is that I never set lastResetTime to 0. lastResetTime now always represents an actual time. This ended up being necessary in a certain scenario.

[whynowy]

Certain scenario such as?

[juliev0]

Say you have condition A && B && C,
Say A and B both occur
Say the pod goes down while the condition reset time occurs
So, we want to be able to ack both A and B when they arrive but we don't: this is because if A arrives, we call reset(), which ends up doing this:
if mh.isCleanedUp() { mh.setLastResetTime(0) }
That is because the pod went down and B is no longer stored in the eventSourceMessageHolder's memory.
Then when B arrives we fail to ack it, because we would skip the logic which checks lastResetTime.

[whynowy]

Good Point!

[whynowy]

LGTM, great work!