Add secondary scheduled date checks when claiming actions (DBStore) | #634

[barryhughes]

🎫 #634 → Double check action scheduled date in case they're processing too early

Closes #634

😖 The Problem

We can broadly divide pending scheduled actions into two categories:

• Those that are due now.
• Those that are not due until some future point in time.

When the Queue Runner claims a set of actions it does so with a query that is designed to ignore those actions that are not yet due, which makes sense (as a commenter in the linked issue noted, we want to make sure the scheduled date is not in the future before running it).

However, there's a body of reported evidence suggesting that—under unusual conditions, such as if MySQL finds it is out of disk space—the WHERE conditions we use to enforce this are seemingly not respected. To combat this, we need an extra set of checks that run at PHP-level.

🛠️ This Changeset

We were already specifying a $before_date when claiming actions (defaulting to 'now') ... with this change, we also test at PHP-level after fetching the individual actions to make sure that the $before_date condition was respected (we don't just rely on MySQL).

This way, besides adding extra safety for common cases, if some custom code wishes to stake a claim that explicitly includes future pending actions (by specifying a $before_date in the future) then that custom code should still function as previously.

📝 Some Notes

• I have focused on the DBStore in this PR. Let's confirm we're happy with this approach/refine and adjust as needed, and I can follow-up with a corresponding change for the wpPostStore.
• Try as I might, I could not actually replicate the problem so I've done my best to simulate it in the test suite by intercepting the claim staking query and modifying it so that it also claims future actions. My thinking is this replicates a situation where the DBStore builds a query that should ignore actions that are not yet due, but where MySQL returns them anyway).

[rrennick]

under unusual conditions, such as if MySQL finds it is out of disk space—the WHERE conditions we use to enforce this are seemingly not respected.

The specific situation where this was observed is that MySQL was out of temporary disk space (which is required when joining tables). When that occurred during a claim query, MySQL ignored the WHERE clause which requires a join and retrieved action IDs according to the order of the PRIMARY KEY and the LIMIT clause of the query.

[barryhughes]

📝 I'm using a private field to share this information rather than modify any of the method signatures (this is for backwards compatibility reasons...if we altered some of the methods to add a new optional param, and passed the information that way, then under PHP 8.0+ we'd face errors if third-party sub-classes didn't update in time).

[claudiosanches]

@barryhughes Great work solving this issue, special kudos for the unit test introduced, nice way to try to reproduce it. I agree that's a hard one to reproduce.

@rrennick do you like to also review this change?

[claudiosanches]

I left some notes about small tweaks that we could do in this PR.

[barryhughes]

Updated, thank you!

[claudiosanches]

Looks great!

[rrennick]

Looks great for me too. Okay to merge once it clears Travis.