1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-27 01:02:42 +01:00
Commit graph

11 commits

Author SHA1 Message Date
Chad Little
3b14c1fdd1 Update AphlictClientServer to support ws2 or ws3
Summary: This lets us support either ws2 or ws3. Fixes T12755

Test Plan: Update server to version 3, send message, watch debug log. Downgrade to 2.x, send messages, watch debug log. Everything seems OK

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12755

Differential Revision: https://secure.phabricator.com/D18411
2017-08-11 15:17:07 -07:00
epriestley
7fd98a5e86 Every so often, ask the Aphict server how things are going
Summary: Ref T12573. This sends a "ping" to the server, and a "pong" back to the client, every 15 seconds. This tricks ELBs into thinking we're doing something useful and productive.

Test Plan: Ran `bin/aphlict debug`, loaded Phabricator, saw ping/pong in logs.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12573

Differential Revision: https://secure.phabricator.com/D17717
2017-04-17 20:33:43 -07:00
epriestley
02194f0fc8 After Aphlict reconnects, ask the server to replay recent messages
Summary:
Fixes T12563. If we've ever seen an "open", mark all future connections as reconnects. When we reconnect, replay recent history.

(Until duplicate messages (T12564) are handled better this may cause some notification duplication.)

Also emit a reconnect event (for T12566) but don't use it yet.

Test Plan: {F4912044}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12563

Differential Revision: https://secure.phabricator.com/D17708
2017-04-17 15:54:51 -07:00
epriestley
d4bf2a147b Make paths and Aphlict instance names less ambiguous
Summary:
Fixes T10783 (what little of it remains). Ref T10697.

Aphlict currently uses request paths for two different things:

  - multi-tenant instancing in the Phacility cluster (each instance gets its own namespace within an Aphlict server);
  - some users configure nginx and apache to do proxying or SSL termination based on the path.

Currently, these can collide.

Put a "~" before the instance name to make it unambiguous. At some point we can possibly just use a GET parameter, but I think there was some reason I didn't do that originally and this sequence of changes is disruptive enough already.

Test Plan: Saw local Aphlict unambiguously recognize "local.phacility.com" as instance "local", with a "~"-style URI.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10697, T10783

Differential Revision: https://secure.phabricator.com/D15705
2016-04-14 04:57:21 -07:00
epriestley
2930733ac9 Complete modernization of Aphlict configuration
Summary:
Fixes T10697. This finishes bringing the rest of the config up to cluster power levels.

Phabricator is now given an arbitrarily long list of notification servers.

Each Aphlict server is given an arbitrarily long list of ports to run services on.

Users are free to make them meet in the middle by proxying whatever they want to whatever else they want.

This should also accommodate clustering fairly easily in the future.

Also rewrote the status UI and changed a million other things. 🐗

Test Plan:
{F1217864}

{F1217865}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10697

Differential Revision: https://secure.phabricator.com/D15703
2016-04-14 04:57:00 -07:00
epriestley
e32ce529d7 Begin generalizing Aphlict server to prepare for clustering/sensible config file
Summary:
Ref T10697. Currently, `aphlict` takes a ton of command line flags to configure exactly one admin server and exactly one client server.

I want to replace this with a config file. Additionally, I plan to support:

  - arbitrary numbers of listening client ports;
  - arbitrary numbers of listening admin ports;
  - SSL on any port.

For now, just transform the arguments to look like they're a config file. In the future, I'll load from a config file instead.

This greater generality will allow you to do stuff like run separate HTTP and HTTPS admin ports if you really want. I don't think there's a ton of use for this, but it tends to make the code cleaner anyway and there may be some weird cross-datacneter cases for it. Certainly, we undershot with the initial design and lots of users want to terminate SSL in nginx and run only HTTP on this server.

(Some sort-of-plausible use cases are running separate HTTP and HTTPS client servers, if your Phabricator install supports both, or running multiple HTTPS servers with different certificates if you have a bizarre VPN.)

Test Plan: Started Aphlict, connected to it, sent myself test notifications, viewed status page, reviewed logfile.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10697

Differential Revision: https://secure.phabricator.com/D15700
2016-04-14 04:54:20 -07:00
epriestley
ebcab8edb6 Namespace Aphlict clients by request path, plus other fixes
Summary:
Fixes T7130. Fixes T7041. Fixes T7012.

Major change here is partitioning clients. In the Phacility cluster, being able to get a huge pile of instances on a single server -- without needing to run a process per instance -- is desirable.

To accomplish this, just bucket clients by the path they connect with. This will let us set client URIs to `/instancename/` and then route connections to a small set of servers. This degrades cleanly in the common case and has no effect on installs which don't do instancing.

Also fix two unrelated issues:

  - Fix the timeouts, which were incorrectly initializing in `open()` (which is called during reconnect, causing them to reset every time). Instead, initialize in the constructor. Cap timeout at 5 minutes.
  - Probably fix subscriptions, which were using a property with an object definition. Since this is by-ref, all concrete instances of the object share the same property, so all users would be subscribed to everything. Probably.

Test Plan:
  - Hit notification status page, saw version bump and instance/path name.
  - Saw instance/path name in client and server logs.
  - Stopped server, saw reconnects after 2, 4, 16, ... seconds.
  - Sent test notification; received test notification.
  - Didn't explicitly test the subscription thing but it should be obvious by looking at `/notification/status/` shortly after a push.

Reviewers: joshuaspence, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7041, T7012, T7130

Differential Revision: https://secure.phabricator.com/D11769
2015-02-16 11:31:15 -08:00
Pierre Moreau
4af1fd2a79 Aphlict - remove listeners when clients close the connection
Summary: Ref T7110. Listeners are now removed when clients close the connection to avoid stacking a never ending number of unused listeners.

Test Plan: Using `wscat` to connect to the Aphlict server; when closing the connection a 'Diconnected.' will appear in the logs and the number of active listeners is decreased by one.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T7110

Differential Revision: https://secure.phabricator.com/D11634
2015-02-02 14:57:20 -08:00
Joshua Spence
53834d1471 Enable "strict" mode for NodeJS
Summary:
In particular, this changes the behavior of NodeJS in the following ways:

- Any attempt to get or modify the global object will result in an error.
- `null` values of `this` will no longer be evaluated to the global object and primitive values of this will not be converted to wrapper objects.
- Writing or deleting properties which have there writeable or configurable attributes set to false will now throw an error instead of failing silently.
- Adding a property to an object whose extensible attribute is false will also throw an error now.
- A functions arguments are not writeable so attempting to change them will now throw an error `arguments = [...]`.
- `with(){}` statements are gone.
- Use of `eval` is effectively banned.
- `eval` and `arguments` are not allowed as variable or function identifiers in any scope.
- The identifiers `implements`, `interface`, `let`, `package`, `private`, `protected`, `public`, `static` and `yield` are all now reserved for future use (roll on ES6).

Test Plan: Verified that Aphlict was still functional.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11430
2015-01-20 07:43:10 +11:00
epriestley
4636833f3d Fix module imports in Aphlict server
Summary:
This was broken in D11383. Basically, I had the `ws` module installed globally whilst testing, but the changes made do not work if the `ws` module is installed locally (i.e. in the `./support/aphlict/server/node_modules` directory). After poking around, it seems that this is due to the sandboxing that is done by `JX.require`.

A quick fix is to just //not// use `JX.require`, although you may have a better idea?

The error that is occurring is as follows:

```
<<< UNCAUGHT EXCEPTION! >>>

Error: Cannot find module 'ws'
    at Function.Module._resolveFilename (module.js:338:15)
    at Function.Module._load (module.js:280:25)
    at Module.require (module.js:364:17)
    at require (module.js:380:17)
    at extra.require (/usr/src/phabricator/webroot/rsrc/externals/javelin/core/init_node.js:48:16)
    at /usr/src/phabricator/support/aphlict/server/lib/AphlictClientServer.js:10:17
    at Script.(anonymous function) [as runInNewContext] (vm.js:41:22)
    at Object.JX.require (/usr/src/phabricator/webroot/rsrc/externals/javelin/core/init_node.js:58:6)
    at Object.<anonymous> (/usr/src/phabricator/support/aphlict/server/aphlict_server.js:102:4)
    at Module._compile (module.js:456:26)
>>> Server exited!
```

Test Plan: Now able to start the Aphlict server.

Reviewers: joshuaspence

Reviewed By: joshuaspence

Subscribers: Korvin, epriestley

Maniphest Tasks: T6987

Differential Revision: https://secure.phabricator.com/D11425
2015-01-19 11:46:14 -08:00
Joshua Spence
9a7ad972cd Refactoring of the Aphlict server
Summary: Tidy the Aphlict server by splitting the functionality into two main modules, `AphlictClientServer` and `AphlictAdminServer. There is still further tidying that could be done here, but I feel that this puts us in a much better place.

Test Plan: Sent notifications via `/notification/status/`.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11383
2015-01-19 07:49:50 +11:00