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

Queue mechanism to stop processing jobs #24

Open
wants to merge 7 commits into
base: development
Choose a base branch
from
Open

Conversation

ani2amigos
Copy link
Collaborator

No description provided.

@ani2amigos ani2amigos requested a review from 2amjsouza March 22, 2024 13:01
Copy link

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
-7.64% (target: -1.00%) 10.53%
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (c8b28e7) 596 579 97.15%
Head commit (837afd9) 648 (+52) 580 (+1) 89.51% (-7.64%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#24) 57 6 10.53%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

You may notice some variations in coverage metrics with the latest Coverage engine update. For more details, visit the documentation

.env.example Outdated
@@ -35,3 +35,5 @@ SENDMAIL_DSN=sendmail://default
MAILER_DSN=native://default

MAIL_CHARSET=utf-8

MAX_ATTEMPTS_DEFAULT=10
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

10 its a lot of to be set as default, you can change this to be 3

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ani2amigos as we have a config file to deal with the envs and load them all at once, you can move this $_ENV['MAX_ATTEMPTS_DEFAULT'] there so we can easily have access to all env setup


$timestamp = $mailJob->getTimeToSend();
$delay = max(0, $timestamp - time());
$pheanstalk->delete($mailJob->getPheanstalkJob());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pheanstalk has a method to re-queue the job, the release as it was set.
set the algorithm to be like:

  • if not finished, increase the attempt number;
  • if not overtaking the number of attempts defined, use release method;
  • if it's finished or reached the attempts limit, delete

$mailJob->incrementAttempt();
if (!$mailJob->isCompleted() && $mailJob->getAttempt() <= $_ENV['MAX_ATTEMPTS_DEFAULT']) {
$timestamp = $mailJob->getTimeToSend();
$delay = max(0, $timestamp - time());
// add back to the queue as it wasn't completed maybe due to some transitory error
Copy link
Collaborator

@2amjsouza 2amjsouza Apr 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove any comment from code not related to docs

$sql = sprintf($sqlText, $this->tableName);
$query = $this->getConnection()->getInstance()->prepare($sql);
$query->bindValue(':id', $mailJob->getId());
} else {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

avoid using else when writing the algs, return query execute instead, them outside the if, the other case:

if($mailJob->getAttempt() > $_ENV['MAX_ATTEMPTS_DEFAULT']){
            $sqlText = 'DELETE FROM mail_queue WHERE id = :max_attempt;';
            $sql = sprintf($sqlText, $this->tableName);
            $query = $this->getConnection()->getInstance()->prepare($sql);
            $query->bindValue(':id', $mailJob->getId());

            return $query->execute();
        }

$sqlText = 'UPDATE `%s`
                SET `attempt`=:attempt, `state`=:state, `timeToSend`=:timeToSend, `sentTime`=:sentTime
                WHERE `id`=:id';

... rest of your code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants