Page MenuHomePhabricator
Create Task
Maniphest T11952

Daemons don't notice changes to global default settings
Closed, ResolvedPublic
Actions

Description

I turned off emails for "A commit is created" and "A commit has auditors added" in the global default settings, but continued to receive emails about those events until I restarted the daemons. Running on Phacility.

To reproduce:

  1. Create a Phacility instance (non-test, since it needs to send emails)
  2. Import a repository.
  3. Create and join a project.
  4. Create a global Herald rule to create an audit for commits in that repository, assigned to the project.
  5. Edit the global default settings and turn off emails for "A commit is created" and "A commit has auditors added".
  6. Push a commit to the repository.

You should not get an email, but you do.

Related Objects

Event Timeline

nornagon created this task.Dec 5 2016, 10:04 PM
nornagon updated the task description. (Show Details)Dec 5 2016, 10:07 PM
chad added a subscriber: chad.Dec 5 2016, 10:10 PM
This comment was removed by chad.
chad added a comment.Dec 5 2016, 10:22 PM

Why does the mail say you received it, see /mail/

epriestley added a subscriber: epriestley.Dec 5 2016, 10:23 PM

I'm not immediately able to reproduce this. Here's what I did:

  • I added this logic to the daemon to dump the preference values it was using:
diff --git a/src/applications/metamta/storage/PhabricatorMetaMTAMail.php b/src/applications/metamta/storage/PhabricatorMetaMTAMail.php
index 0c90e4383..be2fdeb17 100644
--- a/src/applications/metamta/storage/PhabricatorMetaMTAMail.php
+++ b/src/applications/metamta/storage/PhabricatorMetaMTAMail.php
@@ -892,6 +892,12 @@ final class PhabricatorMetaMTAMail
         $user_mailtags = $prefs->getSettingValue(
           PhabricatorEmailTagsSetting::SETTINGKEY);
 
+        $v = array();
+        foreach ($user_mailtags as $tag => $value) {
+          $v[] = "{$tag}={$value}";
+        }
+        echo "{$phid} = ".implode(', ', $v);
+
         // The user must have elected to receive mail for at least one
         // of the mailtags.
         $send = false;
  • I deleted all of my user account's email preference settings.
  • I ran bin/phd debug task to execute the taskmaster in the foreground.
  • I commented on a task, observing the active preferences in the log:
2016-12-05 4:18:16 PM [INIT] Starting process.
2016-12-05 4:18:21 PM [STDO] PHID-USER-cvfydnwadpdj7vdon36z = differential-review-request=1, differential-updated=1, differential-comment=1, differential-committed=1, differential-reviewers=1, differential-cc=1, differential-other=1, fund.backer=1, fund.status=1, fund.other=1, maniphest-status=1, maniphest-owner=1, maniphest-priority=1, maniphest-cc=1, maniphest-projects=1, maniphest-unblock=1, maniphest-column=1, maniphest-comment=1, maniphest-other=1, audit-commit=2, audit-action-concern=2, audit-action-accept=2, audit-action-resign=2, audit-action-close=2, audit-add-auditors=2, audit-add-ccs=2, audit-projects=2, audit-comment=2, audit-other=2, badges:details=1, badges:comment=1, badges:other=1, calendar-content=1, calendar-reschedule=1, calendar-other=1, countdown:details=1, countdown:comment=1, countdown:other=1, paste-content=1, paste-comment=1, paste-other=1, phurl-details=1, project-metadata=1, project-members=1, project-watchers=1, project-other=1, vote:details=1, vote:responses=1, vote:vote=1, phame-blog-details=1, phame-blog-subscribers=1, phame-blog-other=1, phame-post-content=1, phame-post-subscribers=1, phame-post-comment=1, phame-post-other=1, pholio-status=1, pholio-comment=1, pholio-updated=1, pholio-other=1, phriction-title=1, phriction-content=1, phriction-delete=1, phriction-subscribers=1, phriction-other=1, question:details=1, question:answer=1, question:comment=1, question:other=1
  • I changed the global settings, disabling all of the "Differential" email.
  • I commented on the task again, without restarting the daemon.
  • This time, the values were different (and correctly reflected my changes -- note that "differential-*" stuff is now set to "2"):
2016-12-05 4:18:56 PM [STDO] PHID-USER-cvfydnwadpdj7vdon36z = differential-review-request=2, differential-updated=2, differential-comment=2, differential-committed=2, differential-reviewers=2, differential-cc=2, differential-other=2, fund.backer=1, fund.status=1, fund.other=1, maniphest-status=1, maniphest-owner=1, maniphest-priority=1, maniphest-cc=1, maniphest-projects=1, maniphest-unblock=1, maniphest-column=1, maniphest-comment=1, maniphest-other=1, audit-commit=2, audit-action-concern=2, audit-action-accept=2, audit-action-resign=2, audit-action-close=2, audit-add-auditors=2, audit-add-ccs=2, audit-projects=2, audit-comment=2, audit-other=2, badges:details=2, badges:comment=2, badges:other=2, calendar-content=1, calendar-reschedule=1, calendar-other=1, countdown:details=1, countdown:comment=1, countdown:other=1, paste-content=1, paste-comment=1, paste-other=1, phurl-details=1, project-metadata=1, project-members=1, project-watchers=1, project-other=1, vote:details=1, vote:responses=1, vote:vote=1, phame-blog-details=1, phame-blog-subscribers=1, phame-blog-other=1, phame-post-content=1, phame-post-subscribers=1, phame-post-comment=1, phame-post-other=1, pholio-status=1, pholio-comment=1, pholio-updated=1, pholio-other=1, phriction-title=1, phriction-content=1, phriction-delete=1, phriction-subscribers=1, phriction-other=1, question:details=1, question:answer=1, question:comment=1, question:other=1

From examining the code, I can't immediately find any static caches which could be preventing this from working correctly.

Note that if you have changed ANY of the settings, all of your personal defaults override the global defaults. There is no "Default (Use Server Default)" option for these preferences, and they're all-or-nothing. As soon as you change anything, your version of the setting is the one that is permanently in use for your account.

That said, this isn't consistent with your observations if you used the "Restart Daemons" console action and saw different behavior after it took effect. I'll attempt to reproduce this more specifically in the cluster by following your instructions.

nornagon added a comment.Dec 5 2016, 10:32 PM

Your logic there w/ dumping the prefs is definitely convincing. Perhaps I hallucinated this and it was actually that I did something in a different order, e.g. I disabled "A commit is created", then restarted the daemons, then disabled "A commit has auditors added" and misattributed causality (where the real issue was that disabling "A commit is created" is not sufficient to stop receiving emails when a commit is created that you're an auditor on).

epriestley added a comment.Dec 5 2016, 10:32 PM

I can't reproduce this in the Phacility cluster, either. Here's what I did:

  • Launched a new test instance.
  • Created a new Git repository and gave it a name. I changed no other settings, then activated it.
  • I added my SSH public key.
  • I cloned the repository.
  • I created a commit and pushed it.

Since test instances run in silent mode this didn't actually send me an email, but normally would have according to the Mail console (the "Metadata" tab explains that silent mode ultimately stopped mail delivery):

Screen Shot 2016-12-05 at 2.28.31 PM.png (983×1 px, 113 KB)

  • I created global settings and disabled all "Audit" mail:

Screen Shot 2016-12-05 at 2.29.59 PM.png (983×1 px, 208 KB)

  • Without restarting the daemons, I pushed another commit.
  • This mail was correctly voided by the global settings change:

Screen Shot 2016-12-05 at 2.30.55 PM.png (983×1 px, 123 KB)

I'm not sure what else to do to try to reproduce this. Is it possible that this is just confusion about global settings not applying to user accounts if they've customized any of the options on that page?

epriestley added a comment.Dec 5 2016, 10:35 PM

We also made some changes to this recently (D16936, D16983) and it could conceivably be a version issue, but neither of those changes replaced a static cache with a RequestCache or similar, and D16936 is correctly using the RequestCache (which is cleared when the taskmaster de-queues a new task).

nornagon added a comment.Dec 5 2016, 11:59 PM

Sounds like this can safely be closed as PEBKAC

epriestley closed this task as Resolved.Dec 6 2016, 12:01 AM
epriestley claimed this task.
epriestley mentioned this in D17003: [WIP] Allow Home to take a PanelEngine.Dec 7 2016, 8:17 PM
Phabricator · Built by Phacility