Reset the failure flag in workers on successful runs. #76

ansv7779 · 2025-01-10T13:14:34+01:00

ansv7779 commented

2025-01-10 13:14:34 +01:00

When the switch from Guice to Spring happened all workers became singletons because that's the default in Spring. In Guice they were "prototype" scoped and therefore the worker object was re-created before each execution which reset the successfulWorker field to true.

Now that they're singletons the field is never reset to true after a failure and the worker will be stuck in a failed state even after a subsequent successful run.

This is an issues primarily for the RejectedThesisWorker that relies on the timestamp of the last successful run to determine if there are any new rejections that it should handle. This is necessary because the Daisy API only allow filtering by date (and not time) and there's no other identifying information in the rejection data other than a timestamp.

Fixes #75 by assuming the job will be successful by setting the flag to true before execution. If the job fails (via en exception) the flag is set back to false as before. The reason it is not set to success after the job has completed without an exception is because some jobs will manually flag themselves as unsuccessful (by setting the flag to false) even if they throw no exceptions.

When the switch from Guice to Spring happened all workers became singletons because that's the default in Spring. In Guice they were "prototype" scoped and therefore the worker object was re-created before each execution which reset the successfulWorker field to true. Now that they're singletons the field is never reset to true after a failure and the worker will be stuck in a failed state even after a subsequent successful run. This is an issues primarily for the RejectedThesisWorker that relies on the timestamp of the last successful run to determine if there are any new rejections that it should handle. This is necessary because the Daisy API only allow filtering by date (and not time) and there's no other identifying information in the rejection data other than a timestamp. Fixes #75 by assuming the job will be successful by setting the flag to `true` before execution. If the job fails (via en exception) the flag is set back to `false` as before. The reason it is not set to success *after* the job has completed without an exception is because some jobs will manually flag themselves as unsuccessful (by setting the flag to `false`) even if they throw no exceptions.

ansv7779 added 1 commit 2025-01-10 13:14:35 +01:00

Reset the failure flag in workers on successful runs.

Deploy to branch.dsv.su.se / deploy (pull_request) Successful in 2m16s

Details

Build and test / build-and-test (push) Successful in 15m37s

Details

377c99021d

When the switch from Guice to Spring happened all workers became singletons because that's the default in Spring. In Guice they were "prototype" scoped and therefore the worker object was re-created before each execution which reset the successfulWorker field to true.

Now that they're singletons the field is never reset to true after a failure and the worker will be stuck in a failed state even after a subsequent successful run.

This is an issues primarily for the RejectedThesisWorker that relies on the timestamp of the last successful run to determine if there are any new rejections that it should handle. This is necessary because the Daisy API only allow filtering by date (and not time) and there's no other identifying information in the rejection data other than a timestamp.

gitea-actions commented

2025-01-10 13:16:51 +01:00

First-time contributor

Deployed to https://scipro-fix-worker-stuck-in-failed-state.branch.dsv.su.se

tozh4728 requested review from tozh4728 2025-01-13 10:55:49 +01:00

tozh4728 commented

2025-01-13 10:59:51 +01:00

I would rather see a solution that state (class attribute) 'successfulWorker' is removed. It's hard to reason the current behavior. But I understand this issue needs to be address now.

Can we at least add a comment with TODO-prefix?

There are also unused imports:

import jakarta.inject.Provider;
import jakarta.persistence.EntityManager;
import jakarta.persistence.EntityTransaction;

I would rather see a solution that state (class attribute) 'successfulWorker' is removed. It's hard to reason the current behavior. But I understand this issue needs to be address now. Can we at least add a comment with TODO-prefix? There are also unused imports: import jakarta.inject.Provider; import jakarta.persistence.EntityManager; import jakarta.persistence.EntityTransaction;

ansv7779 added 2 commits 2025-01-14 11:46:33 +01:00

Add a TODO item that the successful flag should be removed to simply behaviour 2ae13d8461

Clean up unused imports

Deploy to branch.dsv.su.se / deploy (pull_request) Successful in 2m26s

Details

Build and test / build-and-test (push) Successful in 17m18s

Details

c5ac8d32e8

tozh4728 approved these changes 2025-01-14 14:27:17 +01:00

tozh4728 added 1 commit 2025-01-14 14:27:30 +01:00

Merge branch 'develop' into fix-worker-stuck-in-failed-state

Build and test / build-and-test (push) Successful in 17m57s