Summary:
See PHI1692. Currently, the Aphlict log is ridiculously verbose. As an initial pass at improving this:
- When starting in "debug" mode, pass "--debug=1" to Node.
- In Node, separate logging into "log" (lower-volume, more-important messages) and "trace" (higher-volume, less-important messages).
- Only print "trace" messages in "debug" mode.
Test Plan: Ran Aphlict in debug and non-debug modes. Behavior unchanged in debug mode, but log has more sensible verbosity in non-debug mode.
Differential Revision: https://secure.phabricator.com/D21115
Summary: Depends on D19865. Ref T13222. See PHI996. Provide a `bin/aphlict notify --user ... --message ...` workflow for sending test notifications from the CLI.
Test Plan: {F6058287}
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13222
Differential Revision: https://secure.phabricator.com/D19866
Summary:
Fixes T11818. We don't discard output, so once we read more than 2GB of output we'll exceed the maximum size of a string in an internal buffer.
Instead, configure the future so output is discarded.
Test Plan: Added logging to `libphutil/`, saw internal buffer grow steadily before this change and stay constant at 0 after this change.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11818
Differential Revision: https://secure.phabricator.com/D16855
Summary:
Ref T4103. Ref T10078. This puts a user cache in front of notification and message counts.
This reduces the number of queries issued on every page by 4 (2x building the menu, 2x building Quicksand data).
Also fixes some minor issues:
- Daemons could choke on sending mail in the user's translation.
- No-op object updates could fail in the daemons.
- Questionable data access pattern in the file query coming out of the profile file cache.
Test Plan:
- Sent myself notifications. Saw count go up.
- Cleared them by visiting objects and clearing all notifications. Saw count go down.
- Sent myself messages. Saw count go up.
- Cleared them by visiting threads. Saw count go down.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4103, T10078
Differential Revision: https://secure.phabricator.com/D16041
Summary: Fixes T10863. See that task for discussion.
Test Plan:
- Configured `aphlict` with no "logs".
- Started `aphlict`.
- Before change: exception.
- After change: worked.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10863
Differential Revision: https://secure.phabricator.com/D15788
Summary: I've possibly seen a couple of `aphlict` processes exit under suspicious circumstances (maybe?). Make sure any PHP errors get captured into the log.
Test Plan:
- Added an exception after forking.
- Before change: vanished into thin air.
- After change: visible in the log.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D15782
Summary: Fixes T10844. After recent changes to Aphlict (T6915 and T10697), `./bin/status` needs to be aware of the configuration file. As such, it is now necessary to run `./bin/aphlict status --config /path/to/config.json` rather than `./bin/aphlict status`.
Test Plan: Ran `./bin/aphlict start ...` and `./bin/aphlict status` and saw "Aphlict (`$PID`) is running".
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin
Maniphest Tasks: T10844
Differential Revision: https://secure.phabricator.com/D15776
Summary: Ref T10697. I missed this so it isn't reading the new config properly.
Test Plan: Ran `bin/aphlict stop`, saw it read config.
Reviewers: chad, Mnkras
Reviewed By: Mnkras
Subscribers: Mnkras
Maniphest Tasks: T10697
Differential Revision: https://secure.phabricator.com/D15729
Summary:
Ref T10696. By default, `node` uses 1.5GB, which is enormous overkill for this service and can crowd out other services if it's running next to things like a database on the same host.
Provide a configuration option to adjust it via `--max-old-space-size` and default to 256MB. It only seems to need about 30M locally, so this should be plenty of headroom.
Test Plan:
Ran `bin/aphlict debug`, things seemed OK.
It takes a long time (days?) to grow to 1.5GB so I can't easily test this locally without a lot of work, but I'll keep an eye on it in production.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10696
Differential Revision: https://secure.phabricator.com/D15720
Summary:
Ref T6915. This allows multiple notification servers to talk to each other:
- Every server has a list of every other server, including itself.
- Every server generates a unique fingerprint at startup, like "XjeHuPKPBKHUmXkB".
- Every time a server gets a message, it marks it with its personal fingerprint, then sends it to every other server.
- Servers do not retransmit messages that they've already seen (already marked with their fingerprint).
- Servers learn other servers' fingerprints after they send them a message, and stop sending them messages they've already seen.
This is pretty crude, and the first message to a cluster will transmit N^2 times, but N is going to be like 3 or 4 in even the most extreme cases for a very long time.
The fingerprinting stops cycles, and stops servers from sending themselves copies of messages.
We don't need to do anything more sophisticated than this because it's fine if some notifications get lost when a server dies. Clients will reconnect after a short period of time and life will continue.
Test Plan:
- Wrote two server configs.
- Started two servers.
- Told Phabricator about all four services.
- Loaded Chrome and Safari.
- Saw them connect to different servers.
- Sent messages in one, got notifications in the other (magic!).
- Saw the fingerprinting stuff work on the console, no infinite retransmission of messages, etc.
(This pretty much just worked when I ran it the first time so I probably missed something?)
{F1218835}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T6915
Differential Revision: https://secure.phabricator.com/D15711
Summary: Fixes T10806. Although browsers don't seem to care about this, it's more correct to support it, and the new test console uses normal `cURL` and does care.
Test Plan:
- Hit the error case for providing a chain but no key/cert.
- Used `openssl s_client -connect localhost:22280` to connect to local Aphlict servers.
- With SSL but no chain, saw `openssl` fail to verify the remote.
- With SSL and a chain, saw `openssl` verify the identify of the remote.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10806
Differential Revision: https://secure.phabricator.com/D15709
Summary: Ref T10697. Mostly straightforward. Also allow the server to have multiple logs and log options in the future (e.g., different verbosities or separate admin/client logs or whatever). No specific plans for this, but the default log is pretty noisy today.
Test Plan: Set up a couple of logs, started server, saw it log to them.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10697
Differential Revision: https://secure.phabricator.com/D15702
Summary: Ref T10697. This isn't everything but starts generalizing options and moving us toward a cluster-ready state of affairs.
Test Plan: Started server in various configurations, hit most (all?) of the error cases with bad configs, sent test notifications.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10697
Differential Revision: https://secure.phabricator.com/D15701
Summary: Ref T10697. This just improves a couple of minor `bin/aphlict` things: make argument parsing more explicit/consistent, consolidate a little bit of duplicated code.
Test Plan: Ran all `bin/aphlict` commands.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10697
Differential Revision: https://secure.phabricator.com/D15698
Summary: These should be fine to land whenever.
Test Plan: N/A
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14066
Summary: Return `$this` from setter methods for consistency. I started writing a [[https://secure.phabricator.com/differential/diff/32506/ | linter rule]] to detect this, but I don't think it is trivial to do this properly.
Test Plan: Eyeball it.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D13422
Summary: All classes should extend from some other class. See D13275 for some explanation.
Test Plan: `arc unit`
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D13283
Summary: Ref T7573. Unify code to fetch these counts and do some light formatting since we're going to need to do the same thing for some conpherence-specific ajax in the durable column (See T7708).
Test Plan: loaded up two tabs, one with a durable column on and one without. in the without browser, i read some messages, decrementing my unread count. when i navigated again in the durable column browser, the count updated correctly. with no notifications, commented on a task with another user to get a notification and it showed up properly. visited the task by clicking not the notification and the bubble count decremented correctly
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7573
Differential Revision: https://secure.phabricator.com/D12498
Summary: Fixes T6944. Create the Aphlict PID directory if it does not exist. See also D11387.
Test Plan: Started Aphlict... saw PID directory created.
Reviewers: anton.vladimirov, epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Maniphest Tasks: T6944
Differential Revision: https://secure.phabricator.com/D11906
Summary: Ref T6822.
Test Plan: `grep`. This method is only called from within `PhutilArgumentWorkflow::__construct`.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Maniphest Tasks: T6822
Differential Revision: https://secure.phabricator.com/D11415
Summary: Ref T6944. This was not quite implemented correctly in D11387.
Test Plan: Saw no more exceptions about being unable to create `/var/log`.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: maxhodak, Korvin, epriestley
Maniphest Tasks: T6944
Differential Revision: https://secure.phabricator.com/D11397
Summary: Fixes T6944. Attempt to automatically create the log directory for the Aphlict server. If the directory can't be created, throw a helpful exception.
Test Plan:
# Set `notification.log` to `/var/log/aphlict/aphlict.log`.
# Ran `./bin/aphlict debug` and saw an exception (because the user doesn't have permissions to create the `/var/log/aphlict` directory).
# Ran `sudo chmod 777 /var/log`.
# Ran `./bin/aphlict debug` and saw the `/var/log/aphlict` directory created.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Maniphest Tasks: T6944
Differential Revision: https://secure.phabricator.com/D11387
Summary: In my use case, I have `notification.client-uri` set to `https://phabricator.example.com/ws/` (which routes to `nginx`) but I need `aphlict` to listen to port `22280`.
Test Plan: Tested in our install.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11337
Summary:
Yeahhhhhhhh....
- Open a "stream", not a "steam".
- Make error easier for users to understand.
- Write to the log in debug mode so the issue is more apparent.
Test Plan:
- Started server with bad permissions, got usable error message.
- Started server with good permissions, got logfile.
Reviewers: joshuaspence, btrahan
Reviewed By: btrahan
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11339
Summary: If you are running the Aphlict server behind a reverse proxy (such as `nginx`) then there's no need to bind to `0.0.0.0`. Add a `--client-host` flag to `aphlict_server.js` to allow binding to a different hostname. Also changed the other flags for consistency and clarity.
Test Plan: Started, stopped and debug the Aphlict server.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11288
Summary: This was omitted in D11143.
Test Plan: I don't always test, but when I do... I do it in production.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11284
Summary:
Fixes T6559. No more flash, use Websockets. This is less aggressive than the earlier version, and retains more server logic.
- Support "wss".
- Make the client work.
- Remove "notification.user" entirely.
- Seems ok?
Test Plan:
In Safari, Firefox and Chrome, saw the browsers connect. Made a bunch of comments/updates and saw notifications.
Notable holes in the test plan:
- Haven't tested "wss" yet. I'll do this on secure.
- Notifications are //too fast// now, locally. I get them after I hit submit but before the page reloads.
- There are probably some other rough edges, this is a fairly big patch.
Reviewers: joshuaspence, btrahan
Reviewed By: joshuaspence, btrahan
Subscribers: fabe, btrahan, epriestley
Maniphest Tasks: T6713, T6559
Differential Revision: https://secure.phabricator.com/D11143
Summary: Fixes T5278. This isn't completely perfect (if you have the other `node` binary, it will fail to detect that it's wrong) but we can maybe wait for that to happen and devise some kind of "is this binary really node?" test if users actually hit it.
Test Plan: Faked things, hit the error; unfaked them and hit the normal flow.
Reviewers: joshuaspence
Reviewed By: joshuaspence
Subscribers: epriestley
Maniphest Tasks: T5278
Differential Revision: https://secure.phabricator.com/D9419
Summary:
Currently, it is a bit tricky to build the Aphlict client SWF from the ActionScript source. Provide a `./bin/aphlict build` workflow that simplifies this process.
Depends on D9226.
Test Plan:
Executed the workflow:
```
> ./bin/aphlict build
Done.
```
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9338
Summary: Fixes T5126. Provide `start`, `stop`, `restart`, `debug` and `status` workflows for `./bin/aphlict`. This makes it easier to manage Aphlict as if it were a service.
Test Plan:
```
> sudo ./bin/aphlict status
Aphlict is not running.
> sudo ./bin/aphlict stop
Aphlict is not running.
> sudo ./bin/aphlict start
Aphlict Server started.
> sudo ./bin/aphlict status
Aphlict (12880) is running.
> sudo ./bin/aphlict restart
Stopping Aphlict Server (12880)...
Aphlict Server (12880) exited normally.
Aphlict Server started.
> sudo ./bin/aphlict stop
Stopping Aphlict Server (12895)...
Aphlict Server (12895) exited normally.
> sudo ./bin/aphlict debug
Starting Aphlict server in foreground...
Launching server:
$ node '/usr/src/phabricator/src/applications/aphlict/management/../../../../support/aphlict/server/aphlict_server.js' --port='22280' --admin='22281' --host='localhost' --user='aphlict'
[Fri May 30 2014 09:56:14 GMT+0000 (UTC)] Started Server (PID 12911)
```
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: hach-que, epriestley, Korvin
Maniphest Tasks: T5126
Differential Revision: https://secure.phabricator.com/D9226
