-
-
Notifications
You must be signed in to change notification settings - Fork 4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RFC(sharder): Scaling from the threads to the cores and beyond #8084
Comments
Personally I think yes.
We should most definitely support async too just in case I'd say...wouldn't make sense to be streams only
I think this is very bot specific, and if we allow bots to communicate with ease when sharding we shouldn't provide such things like commands by default (broadcastEval in d.js can be implemented at the library level)
Opt-in sounds the best (as not all broadcasts need to be replied to). We could support timeouts for acks I suppose. Cancelling doesn't make much sense since if you sent it to 5 shards out of 10 then cancel it, you'd need to instruct the 5 shards that received it already to not run what they most likely already ran.
We should probably just enqueue it with a timeout, at least that's what I'd do. Might be more problematic for messages that expect a reply...
Could you provide examples of where such a signal could be emitted? 👀
We should make this configurable by the users I'd say. But then we should also have a method that will be called when the connection fails multiple times, so users can setup alerts for it.
I guess this question is mostly for users that will use the proxy strategy... I'd say we leave this configurable by the users, but also have that aforementioned alert method that will be called. Alternatively we could just turn off the shards since the manager is gone, but users should be made aware somehow as to why it'd happen.
Tricky question...at that point there's no communication between the shards and their manager...I'd argue in that case we would probably support load-balancing multiple proxies. If all die well...refer to previous two questions.
Yep, alternatively exit early.
That would mean there is always a bi-directional channel of communication between proxy and manager, which wouldn't be fully efficient unless we spread messages across all proxies...unless we want to do it that way we can probably keep using the current proxy till it goes 💀 . Just as a reminder, these are my thoughts about these problems. These answers aren't meant to be taken as the rule! |
Raw data approach it is. And I believe we shouldn't make discord.js force either approach, as it limits the message format users can use for their applications. This is a major pain point I have with the current implementation, its wasteful and inefficient approach makes it not suitable for large-scale applications.
Not just timeouts for acks, but for all the calls. Cancelling a request early would basically signal the manager to remove the message from the queue before sending them, and there's a chance we could make shards construct an If a request has been aborted and a few shards have already finished processing it, we could either ignore the results, send the partial results (useful for collecting data even when there's an outage), or allow users to specify either (we could achieve this by sending an enum value next to the command).
Got it, do we also want to add a "hint" for how long shards take to signal ready? This has some use-cases. If the developer configures said hint to be 60 seconds, and shard B has been initializing for 45 seconds (15 seconds for estimated), and the message timeout is 5 seconds, then we could make it abort early since it's far from margin of error (configurable, let's say 10%, which would make 60s±6s). Said hint can also be used to determine when a shard takes way too long to signal ready and emit a warning for the developer. We could also determine this time by "benchmarking" how long shards take to hit ready, after all, the first shard will display a rough estimate of how long the other shards will take to start up, and they can fine-tune this value as more shards spawn.
Process exits with non-zero error code before signalling "ready". Also in worker mode, this would be when the worker cannot initialize.
Non-proxy strategies also would encounter this issue, specifically in fork and cluster modes. I also want to provide a sensible default for this, since it can get complicated real quick, but we can ship without any and then implement one once real-life implementations come out.
My idea was that each proxy would be able to setup maximum resource usage, which boils down to how many shards they can manage. If load-balancing would result on all proxies to max out, then the missing shards would be queued for spawning once the outage is resolved and proxies start reconnecting to the manager. This would also apply for next question as well.
Proxies and managers have bi-directional channel of communication, HTTP2, HTTP3/QUIC, gRPC (powered by HTTP2), and many other strategies support this kind of thing. HTTPS (1.1) also supports this with long-polling requests, and there's WS too, so yeah, it's definitely doable. Nevertheless, don't forget that proxies don't communicate with the manager unless they have to, since they will try to always work with their local group as much as possible. Communication with the manager should be kept to load-balancing and status management. At this point, developers should build a system that allows them to send messages between all processes by using a specialised message broker such as RabbitMQ. Spamming messages thru the network will only make load-balancing a lot slower. |
Maybe I'm missing something here, but what do you mean by
Don't die. If the manager dies, the children also die for Maybe I misunderstood but if a manager dies there will only be the other end of ShardManagerProxy that will have to wait for the manager to reconnect. |
This refers to Rust's way of doing error handling, see: https://doc.rust-lang.org/std/result/ Some folks here have made a JS port of this API: https://www.npmjs.com/package/@sapphire/result |
A bit of input from me:
|
The limited amount of time would be what the "hint" would do, it's basically a configurable timeout. If the messages are configured to take a reply within 5 seconds, and the shard takes 10 seconds and has just started, it won't enqueue unless it's been around 6 seconds (4 seconds remaining, gives 1 second before cancel).
I appreciate that link, I'll have to take a look into it and update the RFC to implement the new information.
Generally, anything that determines an outage: it's either not running or not accessible (the host cannot connect to the network, the port is not longer accessible, etc). I understand that distributed programming can be out of your (and even mine) comfort zone — I myself have never used more than 1 machine to host a service, so I don't really have experience with horizontal scaling1 and redundancy systems.
Seems I mixed up a few things while writing the paragraph, my apologies. WebSocket is just one of several strategies (the easiest but also the most limited and the least efficient). The paragraph was intended more for the raw approaches, doing HTTP calls (with support for HTTPS, HTTP2, and maybe QUIC in the future).
Noted, I was leaning towards telling users to use services such as Redis streams3 or RabbitMQ to distribute messages across shards more efficiently. This would also let us simplify the sharder by not reinventing the wheel. Generally speaking, while the sharder supports message broadcasting and sending, developers should know that they should keep their messages to a minimum, as it can lead to message jamming, which can lead to the sharder's operations running slower due to the jamming's added latency.
Related to the previous point in the issue's body, proxies would connect to several different managers, so if the manager that's managing them dies, another one takes over. It's related to redundancy. We can also make proxies connect to a single manager and tell users that for multi-manager setups, they should use a shared cache (e.g. Redis) so all managers share the state data. Footnotes |
Since you are planning on using core, threads etc, would it be plausible to add identification attributes to worker instances (PID, thread naming, ...) that could easily help end-users in observability of their clusters (not in the Node.js way). As for questions:
The command approach seems better. Structured data sent across processes is a standard practice to provide thread-safety (channel-like communication) and make development easier (typed structures). An event pattern (rather than command or query) is the solution to use to provide loose coupling of workers and independent control over them (which is most likely to be a requirement for downstream frameworks).
They should be by default, letting downstream frameworks deciding if the reply is mandatory. The coupling of workers is a complex topic that should be left to downstream users as operators.
At first it may be better to simply drop it and document the behaviour. In general, tight coupling requires both endpoints to be available and getting an inbox pattern (à la Erlang Virtual Machine's Actors) seems not to be a solution right here.
This question is also downstream specific (CAP theorem), however to be opinionated, if a Parent dies, Children should safely exit ASAP since they often can not be saved. However, it may be a better solution to also implements a strategy/policy pattern here and document the behaviour of each policies, since it is mostly a downstream problem.
IMHO, shards should immediately exit to avoid undefined behaviours since downstream may have rescheduled a proxy for clean shards.
This is complex.
Ideally yes, as shards should be loosely dependant on the proxy.
They should have multiple managers, for retry-policies.
Since proxies also serve as load-balancers for managers, I think it is better to let them scale down to zero yes. If all the managers failed, it is because there is a problem on downstream's cluster. |
It is not that over engineered, I am very happy to see improvement in the scalability of the module as it is one of the most used (if not the most) Discord wrapper in the open-source community. However the design seems to have some flaws, with each entity being too tied to others. Since you are searching for composability, I am not sure about the goal of the module, wouldn't it be better to go to larger scale and adopts decomposition similarly to projets such as Nova or Nostrum with the Erlang Virtual Machine and thus using the Discord.js library as an abstraction between the scaling infrastructure and the business logic written by downstream. I think the most confusing part of your proposal is the |
Preface: this RFC (Request For Comments) is meant to gather feedback and opinions for the feature set and some implementation details for
@discordjs/sharder
! While a lot of the things mentioned in this RFC will be opinionated from my side (like what I’d like to see implemented in the module), not everything might be added in the initial version or some extra things might be added as extras.This RFC is split into several parts, described below. As an important note, through this entire specification, "shard" is not used to describe a gateway shard, those are handled by
@discordjs/ws
(see #8083). "Shard" is used to describe the client that the manager instantiates. Those could be (but are not limited to): workers, processes, or servers.Before any work is done on this, I would like to ask them kindly to not take much if any inspiration from the current shard manager, and that this implementation needs to be as agnostic as possible so it can be used by bots made with discord.js's components (or even with older versions of discord.js).
Yes, I'm also aware this is over-engineered, but everything is designed with composability in mind, and has been thought with experience from implementing this in #7204. Truth be told, without some of those components, certain things would be significantly harder to implement.
ShardManager
Alternatively:
Sharder
,ProcessManager
,ClientManager
This is the entry point in using this module. Its job is to handle the creation of shards, and is the central place where shards can intercommunicate. This manager also sets several parameters via environment variables (if available) to configure the clients.
As a result, a
ShardManager
has access to one or moreShardClient
s, and has a channel for each one of them.API
Strategies
This class also instantiates shards in one of the following four ways:
worker_threads
, it has the benefit of faster IPC than process-based strategies.child_process.fork
, it's how the current shard manager creates the processes, and has the benefit that you can separate the manager from the worker.cluster.fork
, unlike the fork mode, ports are shared and load-balanced.ShardManagerProxy
. More information below.Custom strategies will also be supported (although if you see something useful, please let us know and we'll consider it!).
Default:
Fork
?Lifecycle Handlers
This class will emit events (not necessarily using
EventEmitter
) when:Life checks
All of the manager's requests must automatically timeout within maximum ping time, so if the shards are configured to ping every 45 seconds, and have a maximum time of 60 seconds, then the request timeout must be 60 seconds. This timeout can also be configured globally (defaults to maximum ping time) and per-request (defaults to global value).
Similarly, if a shard hasn't sent a ping within the maximum time, the lifecycle handler is called.
ShardClient
Alternatively:
ShardWorker
,Worker
,Client
This is a class that instantiates the channel with its respective
ShardManager
(orShardManagerProxy
) using the correct stream, and boils down to a wrapper of a duplex with extra management.API
Signals
MessageHandler
Alternatively:
MessageFormat
,MessageBroker
,MessageSerder
This is a class that defines how messages are serialized and deserialized.
API
Strategies
JSON.parse
and deserialized withJSON.stringify
.v8.serialize
and deserialized withv8.deserialize
.Custom strategies will also be supported (custom format, ETF, YAML, TOML, XML, you name it).
Default:
JSON
?MessageTransformer
Alternatively:
MessageMiddleware
,EdgeMiddleware
,ChannelMiddleware
This is a class that defines what to do when reading a message, and what to do when writing one:
API
Strategies
It is unknown if we will ship any built-in strategy for encryption or transformation, since they're very application-defined, but we will at least provide two from Node.js:
zlib.gzip
andzlib.gunzip
.zlib.brotliCompress
andzlib.brotliDecompress
.Custom strategies will also be supported (although if you see something useful, please let us know and we'll consider it!).
Default:
[]
ShardManagerProxy
This is a special class that operates as a shard for
ShardManager
and communicates to it via HTTPS, HTTP2, or HTTP3/QUIC, depending on the networking strategy used. Custom ones (such as gRPC) may be added. Encryption is recommended, so plain HTTP might not be available. There might also be a need to support SSH tunnels to bypass firewalls for greater security. AShardManagerProxy
is configured in a way similar toShardManager
, supporting all of its features, but also adds logic to communicate with the manager itself.The proxy may also use a strategy so if a shard needs to send a message to another, which is available within the proxy, no request is made to the parent, otherwise it will send to the parent, which will try to find the shard among the different proxies.
Questions to answer
Result<T, E>
classes to avoidtry
/catch
and enhance performance?async
inMessageTransformer
? Require streams only?eval
?Raw data approach, commands can be implemented on a bot-level. Similarly to how Discord.js just emits events, people are free to build frameworks on top of
@discordjs/sharder
, so there's no need for us to force a command format, which would pose several limitations on how users can structure their payloads.Opt-in, not all broadcasts or requests need to be replied to.
Yes, and tasks should be aborted and de-queued once the timeout is done.
I personally think this could be beneficial, but the implementation remains to be seen.
error
signal, or do we useexit
for it?process.exit()
without signalling the manager (or proxy), should it try to restart indefinitely? Have a limit that resets on "restart"?ShardManager
dies, what should the shards do?ShardManagerProxy
goes offline, what should its shards do? What should the manager do? Load-balance to other proxies?ShardManagerProxy
is offline at spawn, what should the manager do? Load-balance to other proxies?ShardManagerProxy
goes back online, should the manager load-balance its shards back to it?NEW
: Should the HTTP-based channel strategies be pooling or open on demand?The WebSocket strategy obviously needs to be open at all times, and keeping a connection open makes it easier for messages to be sent in both ways, as well as they reduce latency since they need to do fewer network hops as they don't require a handshake on every request, but it'll also require an advanced system to properly read messages, and will also make aborting requests harder.
NEW
: Should differentShardManagerProxy
s be able to communicate directly to each other, similar to P2P? Or should it be centralized, requiring theShardManager
?I personally think there isn't really much a need for this, a centralized cache database (such as Redis) could store the information, same for RabbitMQ or alternatives. Is there a need for each proxy to know each other? One pro of having this is that it makes proxies more resilient to outages from the manager, but one downside is that it would need to send a lot of data between proxies and the manager to keep all the information synchronized. I believe it's best to leave this job to Redis and let the user decide how it should be used. After all, proxies are very configurable.
NEW
: Should we allowShardManagerProxy
to have multiple managers, or stick to a single one for simplicity?This way, managers can be mirrored, so if the main one is down, the proxies connect to a secondary one. How the data is shared between managers is up to the user, we will try to give the hooks required for the storage to be easy to store and persist.
NEW
: What shouldShardManagerProxy
if all the managers are offline? Should they just stay up?Their capabilities may be limited, since they can't load-balance by themselves, and they may not be able to communicate to each other without P2P communication or a central message broker system, but most of the operations should, in theory, be able to stay up. I believe we can make the behaviour customizable by the developer, with the default of keeping the work and try to reconnect once the managers are up.
Anything else missing
If you think this RFC is missing any critical/crucial bits of information, let me know. I will add it to the main issue body with a bolded EDIT tag to ensure everyone sees it (if they don't want to check the edited history).
Changelog
ShardManagerProxy
.The text was updated successfully, but these errors were encountered: