1. 13 Apr, 2016 4 commits
    • epriestley's avatar
      Allow public users to make intracluster API requests · 99be132e
      epriestley authored
      Summary:
      Ref T10784. On `secure`, logged-out users currently can't browse repositories when cluster/service mode is enabled because they aren't permitted to make intracluster requests.
      
      We don't allow totally public external requests (they're hard to rate limit and users might write bots that polled `feed.query` or whatever which we'd have no way to easily disable) but it's fine to allow intracluster public requests.
      
      Test Plan: Browsed a clustered repository while logged out locally.
      
      Reviewers: chad
      
      Reviewed By: chad
      
      Maniphest Tasks: T10784
      
      Differential Revision: https://secure.phabricator.com/D15695
      99be132e
    • Chad Little's avatar
      Fix Passphrase Credential dialog · abf37aa9
      Chad Little authored
      Summary: Fixes T10772, not sure why this fails, but reverting the code back to old dialog call works.
      
      Test Plan:
        - Try to add a new credential when importing a repository.
        - Also created a new credential normally, via Passphrase.
        - Also edited a credential.
      
      Reviewers: chad
      
      Reviewed By: chad
      
      Subscribers: Korvin
      
      Maniphest Tasks: T10772
      
      Differential Revision: https://secure.phabricator.com/D15691
      abf37aa9
    • epriestley's avatar
      Clean up some old cluster-ish documentation · afb0f7c7
      epriestley authored
      Summary:
      Ref T10751. We currently have a placeholder Almanac document, and a fairly-bad-advice section in Daemons.
      
      Pull these into the modern cluster documentation.
      
      Test Plan: 17 phabricator PHDs
      
      Reviewers: chad
      
      Reviewed By: chad
      
      Maniphest Tasks: T10751
      
      Differential Revision: https://secure.phabricator.com/D15689
      afb0f7c7
    • epriestley's avatar
      Ignore post-write repository synchronization if no devices are configured · 33060d16
      epriestley authored
      Summary: Fixes T10789. If we aren't configured with a device, we never grabbed a lock in the first place, and should not expect one to be held.
      
      Test Plan: Pushed non-cluster-configured Git SSH repository.
      
      Reviewers: chad
      
      Reviewed By: chad
      
      Maniphest Tasks: T10789
      
      Differential Revision: https://secure.phabricator.com/D15692
      33060d16
  2. 12 Apr, 2016 5 commits
    • epriestley's avatar
      Fix pretty drawings · 110223c1
      epriestley authored
      Summary: Changes elsewhere which support spaces before "|" when defining a table so that tables quote properly also accidentally changed these beautiful drawings into remarkup tables.
      
      Test Plan: (( o.O ))
      
      Reviewers: chad
      
      Reviewed By: chad
      
      Differential Revision: https://secure.phabricator.com/D15690
      110223c1
    • epriestley's avatar
      Move toward multi-master replicated repositories · 4244cad9
      epriestley authored
      Summary:
      Ref T4292. This mostly implements the locking/versioning logic for multi-master repositories. It is only active on Git SSH pathways, and doesn't actually do anything useful yet: it just does bookkeeping so far.
      
      When we read (e.g., `git fetch`) the logic goes like this:
      
      - Get the read lock (unique to device + repository).
        - Read all the versions of the repository on every other device.
        - If any node has a newer version:
          - Fetch the newer version.
          - Increment our version to be the same as the version we fetched.
      - Release the read lock.
      - Actually do the fetch.
      
      This makes sure that any time you do a read, you always read the most recently acknowledged write. You may have to wait for an internal fetch to happen (this isn't actually implemented yet) but the operation will always work like you expect it to.
      
      When we write (e.g., `git push`) the logic goes like this:
      
      - Get the write lock (unique to the repository).
        - Do all the read steps so we're up to date.
        - Mark a write pending.
          - Do the actual write.
        - Bump our version and mark our write finished.
      - Release the write lock.
      
      This allows you to write to any replica. Again, you might have to wait for a fetch first, but everything will work like you expect.
      
      There's one notable failure mode here: if the network connection between the repository node and the database fails during the write, the write lock might be released even though a write is ongoing.
      
      The "isWriting" column protects against that, by staying locked if we lose our connection to the database. This will currently "freeze" the repository (prevent any new writes) until an administrator can sort things out, since it'd dangerous to continue doing writes (we may lose data).
      
      (Since we won't actually acknowledge the write, I think, we could probably smooth this out a bit and make it self-healing //most// of the time: basically, have the broken node rewind itself by updating from another good node. But that's a little more complex.)
      
      Test Plan:
        - Pushed changes to a cluster-mode repository.
        - Viewed web interface, saw "writing" flag and version changes.
        - Pulled changes.
        - Faked various failures, got sensible states.
      
      Reviewers: chad
      
      Reviewed By: chad
      
      Maniphest Tasks: T4292
      
      Differential Revision: https://secure.phabricator.com/D15688
      4244cad9
    • epriestley's avatar
      Rough cut of repository cluster status panel · 58eef68b
      epriestley authored
      Summary:
      Ref T4292. This adds some very basic cluster/device data to the new management view. Nothing interesting yet.
      
      Also deal with disabled bindings a little more cleanly.
      
      Test Plan: {F1214619}
      
      Reviewers: chad
      
      Reviewed By: chad
      
      Maniphest Tasks: T4292
      
      Differential Revision: https://secure.phabricator.com/D15685
      58eef68b
    • epriestley's avatar
      Rough cut at new "pro" Diffusion edit UI skeleton · 8a153c1f
      epriestley authored
      Summary:
      Ref T4292. This puts a very rough skeleton in place for the new "Manage Repository" UI, somewhat similar to the "Settings" UI.
      
      Right now, it has one panel with no content, and is not reachable from the UI.
      
      Test Plan: {F1214525}
      
      Reviewers: chad
      
      Reviewed By: chad
      
      Maniphest Tasks: T4292
      
      Differential Revision: https://secure.phabricator.com/D15683
      8a153c1f
    • epriestley's avatar
      Make PullLocal smart about which repositories it should pull · 0216fac3
      epriestley authored
      Summary:
      Ref T10756. When repositories are properly configured for the cluster (which is hard to set up today), be smart about which repositories are expected to exist on the current host, and only pull them.
      
      This generally allows daemons to pretty much do the right thing no matter how many copies are running, although there may still be some lock contention issues that need to be sorted out.
      
      Test Plan: {F1214483}
      
      Reviewers: chad
      
      Reviewed By: chad
      
      Maniphest Tasks: T10756
      
      Differential Revision: https://secure.phabricator.com/D15682
      0216fac3
  3. 11 Apr, 2016 7 commits
    • Chad Little's avatar
      Tidy up project panel errors · 3f9e8e67
      Chad Little authored
      Summary: Makes the fallback a little cleaner on long titles. Wow we have a lot of error states here.
      
      Test Plan: New Milestone with no tasks.
      
      Reviewers: epriestley
      
      Reviewed By: epriestley
      
      Subscribers: Korvin
      
      Differential Revision: https://secure.phabricator.com/D15686
      3f9e8e67
    • Chad Little's avatar
      Update to FontAwesome 4.6.0 · 75ba6058
      Chad Little authored
      Summary: Updates to the latest icon packages
      
      Test Plan: View UIExamples, find new Icons
      
      Reviewers: epriestley
      
      Reviewed By: epriestley
      
      Subscribers: Korvin
      
      Differential Revision: https://secure.phabricator.com/D15684
      75ba6058
    • lkassianik's avatar
      First stab at a badges typeahead · 85d2fda0
      lkassianik authored
      Summary: Ref T10702
      
      Test Plan: Open a user profile, attempt to award an archived or previously awarded badge, badges dialog should provide a typeahead, and the suggestions should offer details about whether a badge is archived or already awarded.
      
      Reviewers: epriestley, #blessed_reviewers
      
      Reviewed By: epriestley, #blessed_reviewers
      
      Subscribers: Korvin
      
      Maniphest Tasks: T10702
      
      Differential Revision: https://secure.phabricator.com/D15665
      85d2fda0
    • Chad Little's avatar
      Fix spelling error · 6b40cfaa
      Chad Little authored
      Summary: Ran into this, correct spelling.
      
      Test Plan: read
      
      Reviewers: epriestley
      
      Reviewed By: epriestley
      
      Subscribers: Korvin
      
      Differential Revision: https://secure.phabricator.com/D15681
      6b40cfaa
    • epriestley's avatar
      Never sever non-cluster database; write more read-only documentation · ac35246d
      epriestley authored
      Summary:
      Ref T4571. Write more of the missing documentation sections and clarify a few things.
      
      Since the "replicating master" check needs a special permission, imposes a performance penalty, is probably very difficult to misconfigure, and likely not a big deal anyway, just drop the idea of trying to automatically detect + prevent it. We still show if it's an issue on the status page, provided we have permission to check.
      
      When you don't have any cluster databases configured, never stop trying to connect to the default master database. We might want to do this eventually as load reduction, but just don't muddy the waters too much for now while things stabilize.
      
      Test Plan:
        - Tested functionality in cluster, non-cluster, and degraded-cluster modes.
        - Used status console to monitor a health check cycle.
        - Read docs.
      
      Reviewers: chad
      
      Reviewed By: chad
      
      Maniphest Tasks: T4571
      
      Differential Revision: https://secure.phabricator.com/D15679
      ac35246d
    • epriestley's avatar
      Automatically sever databases after prolonged unreachability · ebff07d0
      epriestley authored
      Summary:
      Ref T4571. When a database goes down briefly, we fall back to replicas.
      
      However, this fallback is slow (not good for users) and keeps sending a lot of traffic to the master (might be bad if the root cause is load-related).
      
      Keep track of recent connections and fully degrade into "severed" mode if we see a sequence of failures over a reasonable period of time. In this mode, we send much less traffic to the master (faster for users; less load for the database).
      
      We do send a little bit of traffic still, and if the master recovers we'll recover back into normal mode seeing several connections in a row succeed.
      
      This is similar to what most load balancers do when pulling web servers in and out of pools.
      
      For now, the specific numbers are:
      
        - We do at most one health check every 3 seconds.
        - If 5 checks in a row fail or succeed, we sever or un-sever the database (so it takes about 15 seconds to switch modes).
        - If the database is currently marked unhealthy, we reduce timeouts and retries when connecting to it.
      
      Test Plan:
        - Configured a bad `master`.
        - Browsed around for a bit, initially saw "unrechable master" errors.
        - After about 15 seconds, saw "major interruption" errors instead.
        - Fixed the config for `master`.
        - Browsed around for a while longer.
        - After about 15 seconds, things recovered.
        - Used "Cluster Databases" console to keep an eye on health checks: it now shows how many recent health checks were good:
      
      {F1213397}
      
      Reviewers: chad
      
      Reviewed By: chad
      
      Maniphest Tasks: T4571
      
      Differential Revision: https://secure.phabricator.com/D15677
      ebff07d0
    • epriestley's avatar
      Fix an issue with date parsing when viewer timezone differs from server timezone · 5cf09f56
      epriestley authored
      Summary:
      The way `DateTime` works with epochs is weird, I goofed this by having my server/viewer timezone the same and not noticing.
      
      Also fix an issue where you do `?epoch=...` and then manually fiddle with the control: the control should win.
      
      Test Plan:
        - Set viewer and server timezone to different vlaues.
        - Created a countdown using `?epoch=...`.
        - Created a countdown using `?epoch=...` and fiddling with date controls.
        - Created and edited a countdown using date/time control.
        - Poked around Calendar to make sure I didn't ruin anything this time (browsed, created event, edited event).
      
      Reviewers: lpriestley, chad
      
      Reviewed By: chad
      
      Differential Revision: https://secure.phabricator.com/D15680
      5cf09f56
  4. 10 Apr, 2016 6 commits
    • epriestley's avatar
      Automatically degrade to read-only mode when unable to connect to the master · 146fb646
      epriestley authored
      Summary:
      Ref T4571. If we fail to connect to the master, automatically try to degrade into a temporary read-only mode ("UNREACHABLE") for the remainder of the request, if possible.
      
      If the request was something like "load the homepage", that'll work fine. If it was something like "submit a comment", there's nothing we can do and we just have to fail.
      
      Detecting this condition imposes a performance penalty: every request checks the connection and gives the database a long time to respond, since we don't want to drop writes unless we have to. So the degraded mode works, but it's really slow, and may perpetuate the problem if the root issue is load-related.
      
      This lays the groundwork for improving this case by degrading futher into a "SEVERED" mode which will persist across requests. In the future, if several requests in a short period of time fail, we'll sever the database host and refuse to try to connect to it for a little while, connecting directly to replicas instead (basically, we're "health checking" the master, like a load balancer would health check a web application server). This will give us a better (much faster) degraded mode in a major service disruption, and reduce load on the master if the root cause is load-related, giving it a better chance of recovering on its own.
      
      Test Plan:
        - Disabled master in config by changing the host/username, got degraded automatically to UNREACAHBLE mode immediately.
        - Faked full SEVERED mode, requests hit replicas and put me in the mode properly.
        - Made stuff work, hit some good pages.
        - Hit some non-cluster pages.
      
      Reviewers: chad
      
      Reviewed By: chad
      
      Maniphest Tasks: T4571
      
      Differential Revision: https://secure.phabricator.com/D15674
      146fb646
    • epriestley's avatar
      When no master database is configured, automatically degrade to read-only mode · e0a8cac7
      epriestley authored
      Summary: Ref T4571. If `cluster.databases` is configured but only has replicas, implicitly drop to read-only mode and send writes to a replica.
      
      Test Plan:
        - Disabled the `master`, saw Phabricator automatically degrade into read-only mode against replicas.
        - (Also tested: explicit read-only mode, non-cluster mode, properly configured cluster mode).
      
      Reviewers: chad
      
      Reviewed By: chad
      
      Maniphest Tasks: T4571
      
      Differential Revision: https://secure.phabricator.com/D15672
      e0a8cac7
    • epriestley's avatar
      When Phabricator is in read-only mode, explain why · 071741c6
      epriestley authored
      Summary:
      Ref T4571. Allows users to click the "read-only mode" notification to get more information about why an install is in read-only mode.
      
      Installs can be in this mode for several reasons (explicit administrative action, no masters defined, no masters reachable), and it's useful to be able to tell the difference.
      
      Test Plan: {F1212930}
      
      Reviewers: chad
      
      Reviewed By: chad
      
      Maniphest Tasks: T4571
      
      Differential Revision: https://secure.phabricator.com/D15671
      071741c6
    • epriestley's avatar
      Use new first-class MySQL timeout support in Phabricator · c178f29c
      epriestley authored
      Summary: Fixes T6710. After D15669, we support a proper timeout parameter, so we don't need this hack anymore.
      
      Test Plan: See D15669: forced a MySQL connector, set a low timeout, set a bad database, saw fast failures.
      
      Reviewers: chad
      
      Reviewed By: chad
      
      Maniphest Tasks: T6710
      
      Differential Revision: https://secure.phabricator.com/D15670
      c178f29c
    • epriestley's avatar
      When `cluster.databases` is configured, read the master connection from it · 6a4a9bb2
      epriestley authored
      Summary:
      Ref T4571. Ref T10759. Ref T10758. This isn't complete, but gets most of the job done:
      
        - When `cluster.databases` is set up, most things ignore `mysql.host` now.
        - You can `bin/storage upgrade` and stuff works.
        - You can browse around in the web UI and stuff works.
      
      There's still a lot of weird tricky stuff to navigate, and this has real no advantages over configuring a single server yet (no automatic failover, etc).
      
      Test Plan:
        - Configured `cluster.databases` to point at my `t1.micro` hosts in EC2 (master + replica).
        - Ran `bin/storage upgrade`, got a new install setup on them properly.
        - Survived setup warnings, browsed around.
        - Switched back to local config, ran `bin/storage upgrade`, browsed around, went through setup checks.
        - Intentionally broke config (bad hosts, no masters) and things seemed to react reasonably well.
      
      Reviewers: chad
      
      Reviewed By: chad
      
      Maniphest Tasks: T4571, T10758, T10759
      
      Differential Revision: https://secure.phabricator.com/D15668
      6a4a9bb2
    • epriestley's avatar
      Add a "Database Cluster Status" console in Config · 0439645d
      epriestley authored
      Summary: Ref T4571. The configuration option still doesn't do anything, but add a status panel for basic setup monitoring.
      
      Test Plan:
      Here's what a good version looks like:
      
      {F1212291}
      
      Also faked most of the errors it can detect and got helpful diagnostic messages like this:
      
      {F1212292}
      
      Reviewers: chad
      
      Reviewed By: chad
      
      Maniphest Tasks: T4571
      
      Differential Revision: https://secure.phabricator.com/D15667
      0439645d
  5. 09 Apr, 2016 4 commits
    • epriestley's avatar
      Lay `cluster.databases` configuration groundwork for database clustering · 3f51b785
      epriestley authored
      Summary:
      Ref T4571. This adds a new option which allows you to upgrade your one-host configuration to a multi-host configuration by configuring it.
      
      Doing this currently does nothing. I wrote a lot of words about what it is //supposed// to do in the future, though.
      
      Test Plan:
        - Tried to configure the option in all the possible bad ways, got errors.
        - Read documentation.
      
      Reviewers: chad
      
      Reviewed By: chad
      
      Subscribers: eadler
      
      Maniphest Tasks: T4571
      
      Differential Revision: https://secure.phabricator.com/D15663
      3f51b785
    • epriestley's avatar
      Add a `cluster.read-only` option · 49d93dcf
      epriestley authored
      Summary:
      Ref T4571. There will be a very long path beyond this, but add a basic read-only mode. You can explicitly enable this to put Phabricator in a sort of "maintenance" mode today if you're swapping databases or something.
      
      In the long term, we'll automatically degrade into this mode if the master database is down.
      
      Test Plan:
        - Enabled read-only mode.
        - Browsed around.
        - Didn't immediately see anything that was totally 100% broken.
      
      Most stuff is 80-90% broken right now. For example:
      
        - Stuff like submitting comments doesn't work, and gives you a confusing, unhelpful error.
        - None of the UI really knows that it's read-only. EditEngine stuff should all hide itself and say "you can't add new comments while an install is in read-only mode", for example, but currently does not.
      
      Reviewers: chad
      
      Reviewed By: chad
      
      Maniphest Tasks: T4571
      
      Differential Revision: https://secure.phabricator.com/D15662
      49d93dcf
    • Chad Little's avatar
      Set time and date on Calendar Date Control form · 2bdf8ae5
      Chad Little authored
      Summary: Recurring events will fatal a Calendar with this not set. `newDateTime` requires a date and time to be called property. I think this is correct fix? Fixes T10766
      
      Test Plan: Build a recurring event, pull up /calendar/, see recurring events as expected. Previously, fatal.
      
      Reviewers: lpriestley, epriestley
      
      Reviewed By: epriestley
      
      Subscribers: CodeMouse92, Korvin
      
      Maniphest Tasks: T10766
      
      Differential Revision: https://secure.phabricator.com/D15666
      2bdf8ae5
    • epriestley's avatar
      When proxying cluster HTTP requests, forward only selected headers · 997460f1
      epriestley authored
      In the live cluster, some subset of the forwarded headers are creating
      some issues for HTTP repository operations.
      997460f1
  6. 08 Apr, 2016 2 commits
    • Chad Little's avatar
      Update Settings/Config UI · 57e606b3
      Chad Little authored
      Summary: Testing out a new 'nav' layout in Settings / Config. Spent a few days here and couldn't find much better overall.
      
      Test Plan: View each page in Settings and in Config. Save some config options. Test mobile, desktop, tablet.
      
      Reviewers: epriestley
      
      Reviewed By: epriestley
      
      Subscribers: Korvin
      
      Differential Revision: https://secure.phabricator.com/D15659
      57e606b3
    • epriestley's avatar
      Fix an issue with passing HTTP headers through in proxied cluster requests · 60e91d39
      epriestley authored
      Summary:
      I think this fixes the Mercurial + HTTP cluster issue. PHP adds `HTTP_` but we were not stripping it, so we would convert an `X-Whatever-Zebra` header into an `Http-X-Whatever-Zebra` header.
      
      I don't think this behavior has changed? So maybe it just never worked? Git is more popular than Mercurial and SSH is easier to configure than HTTP, so it's plausible. I'll keep a careful eye on this when it deploys.
      
      Test Plan:
        - Set up local service-based Mercurial repository.
        - Tried to clone, got similar error to cluster.
        - Applied patch, clean clone.
      
      Reviewers: chad
      
      Reviewed By: chad
      
      Differential Revision: https://secure.phabricator.com/D15660
      60e91d39
  7. 07 Apr, 2016 7 commits
  8. 06 Apr, 2016 5 commits
    • epriestley's avatar
      Don't dead-end users with out-of-date links to files · 5938d768
      epriestley authored
      Summary: Ref T10262. Instead of dumping an unhelpful 403 "ACCESS DENIED" page on users, explain the most likely cause of the issue and give them a link to return to the file detail page to learn more or get an up-to-date link.
      
      Test Plan: Hit both errors, had a lovely experience with the helpful dialog text.
      
      Reviewers: chad
      
      Reviewed By: chad
      
      Maniphest Tasks: T10262
      
      Differential Revision: https://secure.phabricator.com/D15650
      5938d768
    • epriestley's avatar
      Provide nicer string for trying to move a task to its current columns · 39dfcf4c
      epriestley authored
      Summary: Ref T6027. We got a not-very-user-friendly default string before.
      
      Test Plan: Selected "Move", didn't change the dropdown, hit submit. Now, got a nice human-readable description of the issue.
      
      Reviewers: chad
      
      Reviewed By: chad
      
      Maniphest Tasks: T6027
      
      Differential Revision: https://secure.phabricator.com/D15649
      39dfcf4c
    • Chad Little's avatar
      Bump font size on property headers · 8f67d59d
      Chad Little authored
      Summary: Bumps to 14px, fixes some on Differential
      
      Test Plan: view various headers in Differential
      
      Reviewers: epriestley
      
      Reviewed By: epriestley
      
      Subscribers: Korvin
      
      Differential Revision: https://secure.phabricator.com/D15647
      8f67d59d
    • epriestley's avatar
      Fix getInterestingMoves() fatal? · 0650f725
      epriestley authored
      Summary: Fixes T10740. Probably?
      
      Test Plan: No you
      
      Reviewers: chad
      
      Reviewed By: chad
      
      Maniphest Tasks: T10740
      
      Differential Revision: https://secure.phabricator.com/D15648
      0650f725
    • epriestley's avatar
      Reduce thumbnail flickering in comment previews · 5664c838
      epriestley authored
      Summary:
      Ref T10262. Currently, we always render a tag like this when you `{F123}` an image in remarkup:
      
      ```
      <img src="/xform/preview/abcdef/" />
      ```
      
      This either generates the preview or redirects to an existing preview. This is a good behavior in general, because the preview may take a while to generate and we don't want to wait for it to generate on the server side.
      
      However, this flickers a lot in Safari. We might be able to cache this, but we really shouldn't, since the preview URI isn't a legitimately stable/permanent one.
      
      Instead, do a (cheap) server-side check to see if the preview already exists. If it does, return a direct URI. This gives us a stable thumbnail in Safari.
      
      Test Plan:
        - Dragged a dog picture into comment box.
        - Typed text.
        - Thing didn't flicker like crazy all the time in Safari.
      
      Reviewers: chad
      
      Reviewed By: chad
      
      Maniphest Tasks: T10262
      
      Differential Revision: https://secure.phabricator.com/D15646
      5664c838