Page MenuHomePhabricator
Files F15574970

D19006.id45571.diff
No OneTemporary
Actions

D19006.id45571.diff
View Options

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -3178,6 +3178,7 @@
'PhabricatorMacroTransactionQuery' => 'applications/macro/query/PhabricatorMacroTransactionQuery.php',
'PhabricatorMacroTransactionType' => 'applications/macro/xaction/PhabricatorMacroTransactionType.php',
'PhabricatorMacroViewController' => 'applications/macro/controller/PhabricatorMacroViewController.php',
+ 'PhabricatorMailConfigTestCase' => 'applications/metamta/storage/__tests__/PhabricatorMailConfigTestCase.php',
'PhabricatorMailEmailHeraldField' => 'applications/metamta/herald/PhabricatorMailEmailHeraldField.php',
'PhabricatorMailEmailHeraldFieldGroup' => 'applications/metamta/herald/PhabricatorMailEmailHeraldFieldGroup.php',
'PhabricatorMailEmailSubjectHeraldField' => 'applications/metamta/herald/PhabricatorMailEmailSubjectHeraldField.php',
@@ -8680,6 +8681,7 @@
'PhabricatorMacroTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
'PhabricatorMacroTransactionType' => 'PhabricatorModularTransactionType',
'PhabricatorMacroViewController' => 'PhabricatorMacroController',
+ 'PhabricatorMailConfigTestCase' => 'PhabricatorTestCase',
'PhabricatorMailEmailHeraldField' => 'HeraldField',
'PhabricatorMailEmailHeraldFieldGroup' => 'HeraldFieldGroup',
'PhabricatorMailEmailSubjectHeraldField' => 'PhabricatorMailEmailHeraldField',
diff --git a/src/applications/metamta/storage/PhabricatorMetaMTAMail.php b/src/applications/metamta/storage/PhabricatorMetaMTAMail.php
--- a/src/applications/metamta/storage/PhabricatorMetaMTAMail.php
+++ b/src/applications/metamta/storage/PhabricatorMetaMTAMail.php
@@ -513,12 +513,14 @@
$defaults = $mailer->newDefaultOptions();
$options = idx($spec, 'options', array()) + $defaults;
$mailer->setOptions($options);
+
+ $mailers[] = $mailer;
}
}
$sorted = array();
$groups = mgroup($mailers, 'getPriority');
- ksort($groups);
+ krsort($groups);
foreach ($groups as $group) {
// Reorder services within the same priority group randomly.
shuffle($group);
diff --git a/src/applications/metamta/storage/__tests__/PhabricatorMailConfigTestCase.php b/src/applications/metamta/storage/__tests__/PhabricatorMailConfigTestCase.php
new file mode 100644
--- /dev/null
+++ b/src/applications/metamta/storage/__tests__/PhabricatorMailConfigTestCase.php
@@ -0,0 +1,131 @@
+<?php
+
+final class PhabricatorMailConfigTestCase
+ extends PhabricatorTestCase {
+
+ public function testMailerPriorities() {
+ $mailers = $this->newMailersWithConfig(
+ array(
+ array(
+ 'key' => 'A',
+ 'type' => 'test',
+ ),
+ array(
+ 'key' => 'B',
+ 'type' => 'test',
+ ),
+ ));
+
+ $this->assertEqual(
+ array('A', 'B'),
+ mpull($mailers, 'getKey'));
+
+ $mailers = $this->newMailersWithConfig(
+ array(
+ array(
+ 'key' => 'A',
+ 'priority' => 1,
+ 'type' => 'test',
+ ),
+ array(
+ 'key' => 'B',
+ 'priority' => 2,
+ 'type' => 'test',
+ ),
+ ));
+
+ $this->assertEqual(
+ array('B', 'A'),
+ mpull($mailers, 'getKey'));
+
+ $mailers = $this->newMailersWithConfig(
+ array(
+ array(
+ 'key' => 'A1',
+ 'priority' => 300,
+ 'type' => 'test',
+ ),
+ array(
+ 'key' => 'A2',
+ 'priority' => 300,
+ 'type' => 'test',
+ ),
+ array(
+ 'key' => 'B',
+ 'type' => 'test',
+ ),
+ array(
+ 'key' => 'C',
+ 'priority' => 400,
+ 'type' => 'test',
+ ),
+ array(
+ 'key' => 'D',
+ 'type' => 'test',
+ ),
+ ));
+
+ // The "A" servers should be shuffled randomly, so either outcome is
+ // acceptable.
+ $option_1 = array('C', 'A1', 'A2', 'B', 'D');
+ $option_2 = array('C', 'A2', 'A1', 'B', 'D');
+ $actual = mpull($mailers, 'getKey');
+
+ $this->assertTrue(($actual === $option_1) || ($actual === $option_2));
+
+ // Make sure that when we're load balancing we actually send traffic to
+ // both servers reasonably often.
+ $saw_a1 = false;
+ $saw_a2 = false;
+ $attempts = 0;
+ while (true) {
+ $mailers = $this->newMailersWithConfig(
+ array(
+ array(
+ 'key' => 'A1',
+ 'priority' => 300,
+ 'type' => 'test',
+ ),
+ array(
+ 'key' => 'A2',
+ 'priority' => 300,
+ 'type' => 'test',
+ ),
+ ));
+
+ $first_key = head($mailers)->getKey();
+
+ if ($first_key == 'A1') {
+ $saw_a1 = true;
+ }
+
+ if ($first_key == 'A2') {
+ $saw_a2 = true;
+ }
+
+ if ($saw_a1 && $saw_a2) {
+ break;
+ }
+
+ if ($attempts++ > 1024) {
+ throw new Exception(
+ pht(
+ 'Load balancing between two mail servers did not select both '.
+ 'servers after an absurd number of attempts.'));
+ }
+ }
+
+ $this->assertTrue($saw_a1 && $saw_a2);
+ }
+
+ private function newMailersWithConfig(array $config) {
+ $env = PhabricatorEnv::beginScopedEnv();
+ $env->overrideEnvConfig('cluster.mailers', $config);
+
+ $mailers = PhabricatorMetaMTAMail::newMailers();
+
+ unset($env);
+ return $mailers;
+ }
+
+}

File Metadata

Mime Type
text/plain
Expires
Wed, May 7, 4:19 AM (15 h, 22 m ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7406436
Default Alt Text
D19006.id45571.diff (5 KB)

Event Timeline

Phabricator · Built by Phacility