@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator

Remove retry/failure mechanisms from MetaMTA

Summary:
Fixes T4202. We have old code in MetaMTA which implements gradual backoff and maximum retries.

However, we have more general code in the task queue which does this, too. We can just use the more general stuff in the task queue; it obsoletes the specific stuff in MetaMTA, which is more complex and ran into some kind of issue in T4202.

Remove `retryCount`, `nextRetry` (obsoleted by task queue retry mechanisms) and "simulated failures" (no longer in use).

Generally, modern infrastructure has replaced these mechanisms with more general ones.

Test Plan:
- Sent mail.
- Observed unsendable mail failing in reasonable ways in the queue.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4202

Differential Revision: https://secure.phabricator.com/D8115

+29 -65
+2
resources/sql/autopatches/20140130.mail.1.retry.sql
··· 1 + ALTER TABLE {$NAMESPACE}_metamta.metamta_mail 2 + DROP COLUMN retryCount;
+2
resources/sql/autopatches/20140130.mail.2.next.sql
··· 1 + ALTER TABLE {$NAMESPACE}_metamta.metamta_mail 2 + DROP COLUMN nextRetry;
+16 -19
src/applications/metamta/PhabricatorMetaMTAWorker.php
··· 3 3 final class PhabricatorMetaMTAWorker 4 4 extends PhabricatorWorker { 5 5 6 - private $message; 6 + public function getMaximumRetryCount() { 7 + return 250; 8 + } 7 9 8 10 public function getWaitBeforeRetry(PhabricatorWorkerTask $task) { 9 - $message = $this->loadMessage(); 10 - if (!$message) { 11 - return null; 12 - } 13 - 14 - $wait = max($message->getNextRetry() - time(), 0) + 15; 15 - return $wait; 11 + return ($task->getFailureCount() * 15); 16 12 } 17 13 18 14 public function doWork() { 19 15 $message = $this->loadMessage(); 20 - if (!$message 21 - || $message->getStatus() != PhabricatorMetaMTAMail::STATUS_QUEUE) { 16 + if (!$message) { 17 + throw new PhabricatorWorkerPermanentFailureException( 18 + pht('Unable to load message!')); 19 + } 20 + 21 + if ($message->getStatus() != PhabricatorMetaMTAMail::STATUS_QUEUE) { 22 22 return; 23 23 } 24 + 24 25 $id = $message->getID(); 25 26 $message->sendNow(); 27 + 26 28 // task failed if the message is still queued 27 29 // (instead of sent, void, or failed) 28 30 if ($message->getStatus() == PhabricatorMetaMTAMail::STATUS_QUEUE) { 29 - throw new Exception('Failed to send message'); 31 + throw new Exception( 32 + pht('Failed to send message.')); 30 33 } 31 34 } 32 35 33 36 private function loadMessage() { 34 - if (!$this->message) { 35 - $message_id = $this->getTaskData(); 36 - $this->message = id(new PhabricatorMetaMTAMail())->load($message_id); 37 - if (!$this->message) { 38 - return null; 39 - } 40 - } 41 - return $this->message; 37 + $message_id = $this->getTaskData(); 38 + return id(new PhabricatorMetaMTAMail())->load($message_id); 42 39 } 43 40 44 41 public function renderForDisplay(PhabricatorUser $viewer) {
-3
src/applications/metamta/management/PhabricatorMailManagementResendWorkflow.php
··· 54 54 } 55 55 56 56 $message->setStatus(PhabricatorMetaMTAMail::STATUS_QUEUE); 57 - $message->setRetryCount(0); 58 - $message->setNextRetry(time()); 59 - 60 57 $message->save(); 61 58 62 59 $mailer_task = PhabricatorWorker::scheduleTask(
-2
src/applications/metamta/management/PhabricatorMailManagementShowOutboundWorkflow.php
··· 50 50 $info[] = pht('PROPERTIES'); 51 51 $info[] = pht('ID: %d', $message->getID()); 52 52 $info[] = pht('Status: %s', $message->getStatus()); 53 - $info[] = pht('Retry Count: %s', $message->getRetryCount()); 54 - $info[] = pht('Next Retry: %s', $message->getNextRetry()); 55 53 $info[] = pht('Related PHID: %s', $message->getRelatedPHID()); 56 54 $info[] = pht('Message: %s', $message->getMessage()); 57 55
+9 -41
src/applications/metamta/storage/PhabricatorMetaMTAMail.php
··· 1 1 <?php 2 2 3 3 /** 4 - * See #394445 for an explanation of why this thing even exists. 5 - * 6 4 * @task recipients Managing Recipients 7 5 */ 8 6 final class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO { ··· 12 10 const STATUS_FAIL = 'fail'; 13 11 const STATUS_VOID = 'void'; 14 12 15 - const MAX_RETRIES = 250; 16 13 const RETRY_DELAY = 5; 17 14 18 15 protected $parameters; 19 16 protected $status; 20 17 protected $message; 21 - protected $retryCount; 22 - protected $nextRetry; 23 18 protected $relatedPHID; 24 19 25 20 private $excludePHIDs = array(); ··· 28 23 public function __construct() { 29 24 30 25 $this->status = self::STATUS_QUEUE; 31 - $this->retryCount = 0; 32 - $this->nextRetry = time(); 33 26 $this->parameters = array(); 34 27 35 28 parent::__construct(); ··· 228 221 return $this; 229 222 } 230 223 231 - public function getSimulatedFailureCount() { 232 - return nonempty($this->getParam('simulated-failures'), 0); 233 - } 234 - 235 - public function setSimulatedFailureCount($count) { 236 - $this->setParam('simulated-failures', $count); 237 - return $this; 238 - } 239 - 240 224 public function getWorkerTaskID() { 241 225 return $this->getParam('worker-task'); 242 226 } ··· 338 322 if (!$force_send) { 339 323 if ($this->getStatus() != self::STATUS_QUEUE) { 340 324 throw new Exception("Trying to send an already-sent mail!"); 341 - } 342 - 343 - if (time() < $this->getNextRetry()) { 344 - throw new Exception("Trying to send an email before next retry!"); 345 325 } 346 326 } 347 327 ··· 611 591 return $this->save(); 612 592 } 613 593 614 - if ($this->getRetryCount() < $this->getSimulatedFailureCount()) { 594 + try { 595 + $ok = $mailer->send(); 596 + $error = null; 597 + } catch (PhabricatorMetaMTAPermanentFailureException $ex) { 598 + $this->setStatus(self::STATUS_FAIL); 599 + $this->setMessage($ex->getMessage()); 600 + return $this->save(); 601 + } catch (Exception $ex) { 615 602 $ok = false; 616 - $error = 'Simulated failure.'; 617 - } else { 618 - try { 619 - $ok = $mailer->send(); 620 - $error = null; 621 - } catch (PhabricatorMetaMTAPermanentFailureException $ex) { 622 - $this->setStatus(self::STATUS_FAIL); 623 - $this->setMessage($ex->getMessage()); 624 - return $this->save(); 625 - } catch (Exception $ex) { 626 - $ok = false; 627 - $error = $ex->getMessage()."\n".$ex->getTraceAsString(); 628 - } 603 + $error = $ex->getMessage()."\n".$ex->getTraceAsString(); 629 604 } 630 605 631 606 if (!$ok) { 632 607 $this->setMessage($error); 633 - if ($this->getRetryCount() > self::MAX_RETRIES) { 634 - $this->setStatus(self::STATUS_FAIL); 635 - } else { 636 - $this->setRetryCount($this->getRetryCount() + 1); 637 - $next_retry = time() + ($this->getRetryCount() * self::RETRY_DELAY); 638 - $this->setNextRetry($next_retry); 639 - } 640 608 } else { 641 609 $this->setStatus(self::STATUS_SENT); 642 610 }