ergo-services / ergo

An actor-based Framework with network transparency for creating event-driven architecture in Golang. Inspired by Erlang. Zero dependencies.
https://docs.ergo.services
MIT License
3.51k stars 138 forks source link

The message from the remote peer silently drops if the mailbox is full. #96

Closed halturin closed 2 years ago

halturin commented 2 years ago

There was an issue #95 I've moved to the discussion https://github.com/ergo-services/ergo/discussions/95 by mistake (misunderstood the real problem).

There should be a way to handle the situation of overflowing the mailbox during the message delivery from the remote peer. Current implementation just silently drops the message (master branch: https://github.com/ergo-services/ergo/blob/master/node/network.go#L346 and in the current development branch https://github.com/ergo-services/ergo/blob/v210/proto/dist/proto.go#L760)

Thanks to @jiait for pointing to this issue.

jiait commented 2 years ago

Very Good! Expect ergo to get better and better, Erlang Observer can't link Ergo, if it can also be fixed, it will be better for Erlang developers

jiait commented 2 years ago

Is there a good solution? This is a serious bug!!

halturin commented 2 years ago

I still don't have any solution for that. At the same time, I couldn't agree with the erlang approach - overflowing the process mailbox. This way leads to the killing of this node by OOM. It means any external process could flood the node, so it is a DOS attack.

I see a few ways to solve it

  1. drop the connection with the source of the flood (not sure if it's going to be a good solution)
  2. send a notification message to the sender if the mailbox is getting filled more than a half
  3. send this message back to the sender but wrapped as a notification message

I would appreciate it if you share better ideas.

PS: the next release will include a new feature - proxy. It means we shouldn't allow any outside actor to have the ability to flood the node.

jiait commented 2 years ago

Mailbox needs an appropriate number of concurrent requests, for example: The length of mailbox expands to 1000. The processes created by the Supervisor do not carry the Mailbox parameter and can follow the Supervisor process parameter by default When the Mailbox is full, it must notify the application layer so that the application layer can take other measures to resolve the problem

jiait commented 2 years ago

The process can apply for batch processing of Mailbox messages by itself. The consumption speed is higher than the production speed, so mailbox will not be full as much as possible. Of course, as you mentioned the external network open interface flood attack, it must control the concurrent speed, but the internal system is their own artificial control, will not produce malicious situation

halturin commented 2 years ago

I think it's a great idea to notify the parent (supervisor) or the group leader (application). But the problem is they don't have callback methods to handle this message.

Supervisor has Init method only https://github.com/ergo-services/ergo/blob/master/gen/supervisor.go#L12 Application has Load and Start methods only https://github.com/ergo-services/ergo/blob/master/gen/application.go#L35

I also can extend ApplicationChildSpec https://github.com/ergo-services/ergo/blob/master/gen/application.go#L54 and SupervisorChildSpec https://github.com/ergo-services/ergo/blob/master/gen/supervisor.go#L86 by adding field ProcessOptions to adjust the Mailbox length and other options

The process can apply for batch processing of Mailbox messages by itself.

not sure what does it mean in terms of actor where every message must be processed sequentially (due to internal atomic state).

halturin commented 2 years ago

There may be a solution by adding a new option like ProcessOptions.FallbackProcess where messages should be sent if the process can't handle coming messages?

jiait commented 2 years ago

When receiving mailbox messages in batches, the application layer can create message queues to store unprocessed messages and solve problems selectively, which is much more feasible than discarking messages in Mailbox

halturin commented 2 years ago

I believe, introducing FallbackProcess in ProcessOptions solves this issue. This process could start another child's process and send this message there or handle it by itself. There should be a good idea to send a notification message to the sender If the mailbox of FallbackProcess overflows with dropping message and printing warning to the stdout. If there wasn't FallbackProcess presented then send a notification, drop this message, print warning.

jiait commented 2 years ago

Looking forward to version update,and issue #92 #93

jiait commented 2 years ago

Looking forward to version update!!

halturin commented 2 years ago

Looking forward to version update!!

The next release is planned for March

halturin commented 2 years ago

fixed in 2.1.0