add catch JQStore_JobNotFoundException exception to detectHungJobs, c…

[woofyman99]

…ontinue

---

This change is

[apinstein]

Reviewed 1 of 1 files at r1. Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

src/JQJobs/JQStore/Propel.php, line 98 [r1] (raw file):

        $c->add($this->options['jobMaxRuntimeSecondsColName'], NULL, Criteria::ISNOTNULL);
        $c->add($this->options['jobMaxRuntimeSecondsColName'], "{$this->options['jobStartDtsColName']} + ({$this->options['jobMaxRuntimeSecondsColName']}||' seconds')::interval < now()", Criteria::CUSTOM);
        $possiblyHungJobs = call_user_func(array("{$this->propelClassName}Peer", 'doSelect'), $c, $this->con);

I think if you hack in, for testing purposes, a "delete all jobs from db" at this point, you'll get to test your code below in the actual situation...

---

src/JQJobs/JQStore/Propel.php, line 118 [r1] (raw file):

            } catch(JQStore_JobNotFoundException $e) {
                // the job already finished, continue the loop
                continue;

do you need to abort the tx? make sure you test that if you have 10 jobs, and there are more in the loop after experiencing the notfound, that the next iterations still work. i would be concerned that the $con might be borked b/c the "begin" and "abort/commit" aren't matched...

also I wonder if you would want to PRINT something in this catch to eval whether it's working in prod (at least temporarily)

---

Comments from Reviewable

[woofyman99]

src/JQJobs/JQStore/Propel.php, line 98 [r1] (raw file):

Previously, apinstein (Alan Pinstein) wrote…

> I think if you hack in, for testing purposes, a "delete all jobs from db" at this point, you'll get to test your code below in the actual situation... >

Yes, that worked.

---

Comments from Reviewable

[woofyman99]

Review status: 0 of 1 files reviewed at latest revision, 2 unresolved discussions.

---

src/JQJobs/JQStore/Propel.php, line 118 [r1] (raw file):

Previously, apinstein (Alan Pinstein) wrote…

> do you need to abort the tx? make sure you test that if you have 10 jobs, and there are more in the loop after experiencing the notfound, that the next iterations still work. i would be concerned that the `$con` might be borked b/c the "begin" and "abort/commit" aren't matched... > > also I wonder if you would want to PRINT something in this catch to eval whether it's working in prod (at least temporarily) >

Done.

---

Comments from Reviewable

[woofyman99]

ready for review

[apinstein]

---

Reviewed 1 of 1 files at r2. Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[apinstein]

Review status: all files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.

---

src/JQJobs/JQStore/Propel.php, line 114 [r2] (raw file):

                $this->con->commit();
            } catch(JQStore_JobNotFoundException $e) {
                // the job already finished, continue the loop

actually will you note that this is specifically to be graceful in the "race condition of job was overdue but finished by the time we got to it in the loop" -- I fear that if we have to debug in future that "job already finished" won't be clear enough to remember what this solves

---

Comments from Reviewable