Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SqlParallel - Enable more tests. Fix compatiblity with release_time #22326

Merged
merged 1 commit into from
Dec 31, 2021

Conversation

totten
Copy link
Member

@totten totten commented Dec 27, 2021

Overview

The unit-test CRM_Queue_QueueTest provides some generic test-coverage for a couple different backends (Sql, Memory). This expands it to also test SqlParallel.

One test case, testTimeoutRelease(), was failing when run with SqlParallel. The PR updates it to pass.

@artfulrobot I can't recall if there is a reason why SqlParallel changed the release_time filter?

Before

  • In Sql and Memory-backed queues, the claimItem() logic will select items with any of these three dispositions:
    • New item, ready to go immediately (release_time=null)
    • New item, marked for future execution (release_time=NNN)
    • Previously attempted item that did not complete (eg it crashed; never finished; used up the full lease; release_time=NNN).
  • In SqlParallel, claimItem() will only select items in this case:
    • New item, not previously attempted (release_time=null)

After

  • In Sql, Memory, and SqlParallel, the release_time works the same way. It should support all 3 scenarios.
    • New item, ready to go immediately (release_time=null)
    • New item, marked for future execution (release_time=NNN)
    • Previously attempted item that did not complete (eg it crashed; never finished; used up the

The scenario `CRM_Queue_QueueTest::testTimeoutRelease()` ensures that
queue-items can be released for re-try after some period of time.

The test was not running with `SqlParallel` backend -- and once I tried
enableing it, it failed.

The patch enables testing for `SqlParallel` and gets it to pass.
@civibot
Copy link

civibot bot commented Dec 27, 2021

(Standard links)

@civibot civibot bot added the master label Dec 27, 2021
@colemanw colemanw merged commit ded7073 into civicrm:master Dec 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants