Skip to content
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

add an udp server example #502

Closed
wants to merge 1 commit into from
Closed

add an udp server example #502

wants to merge 1 commit into from

Conversation

schmurfy
Copy link

This might not be mergeable as is but so far this is the best I got for creating an udp server, what I was going for here is that if anything bad happens the server should restart and still be able to receive data.

The design started from a discussion in #144 with the modified code @pitr-ch and after playing around a bit I came to this, I am not entirely happy with this but it works, if you send "crash" to it it will crash and get restarted to continue process the inputs, the udp socket itself is never stopped.

If anyone has any idea to improve this you are welcome :)

@pitr-ch
Copy link
Member

pitr-ch commented Feb 11, 2016

Thank you for opening the PR!



class MyUDPServer < Concurrent::Actor::RestartingContext
@@thread = {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am curious why is a class variable used?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't figure out a way to properly exit the thread if the actor dies so I tried another road which was to keep the receiving thread linked to the socket alive and only restart the actor itself, the thread variable is used so that the thread is only spawned once.

As I said in my pr message I not that entirely happy with this solution but I tested it and at least it works, at least from what I remember when I tried :)

@pitr-ch pitr-ch added this to the 1.0.1 milestone Feb 15, 2016
@pitr-ch pitr-ch self-assigned this Feb 15, 2016
@pitr-ch
Copy link
Member

pitr-ch commented Feb 15, 2016

Thank you @schmurfy I have look later and I'll add it to our documentation.

@schmurfy
Copy link
Author

it should be refined before adding to the documentation but I wanted to start the conversation, if you have ideas on improvements I can give them a try, the main thing I was looking for was to have a reliable, fault resilient actor receiving the udp packets, it might miss a few but no matter what happens it should resume listening.

@pitr-ch pitr-ch modified the milestones: 1.0.2 Release, 1.0.1 Mar 12, 2016
@pitr-ch
Copy link
Member

pitr-ch commented Mar 12, 2016

@schmurfy could you have look if this https://gist.github.com/pitr-ch/119e48c75ab5ccbbdac9 works for you? If yes, please update the PR.

@pitr-ch pitr-ch mentioned this pull request Mar 12, 2016
@schmurfy
Copy link
Author

schmurfy commented Apr 1, 2016

Sure but after a quick look I am pretty sure it has the same issue I tried to dodge: if the server crash it will not be able to rebind on the same port because it will be already used (the old thread won't be dead), so if your actor crash for any reason you won't ever receive another message.

@jdantonio jdantonio modified the milestones: 1.1.0, 1.0.2 Apr 24, 2016
@pitr-ch
Copy link
Member

pitr-ch commented May 3, 2016

Isn't the tread terminated on actor crash? https://gist.github.com/pitr-ch/119e48c75ab5ccbbdac9#file-udp_server-rb-L38

@schmurfy
Copy link
Author

schmurfy commented May 4, 2016

I just did another test with your version, I just added a crash condition as I did in mine when receiving "crash" and here what I get hust after the crash:

[2016-05-04 11:43:28.363] DEBUG -- /udp1: event: [:terminated, nil] (public)
[2016-05-04 11:43:28.363] DEBUG -- /: was told [:terminated, nil] by #<Concurrent::Actor::Reference:0x7fe99516a9a0 /udp1 (MyUDPServer)>
[2016-05-04 11:43:28.363] DEBUG -- /: was told :remove_child by #<Concurrent::Actor::Reference:0x7fe99516a9a0 /udp1 (MyUDPServer)>
[2016-05-04 11:43:34.945] DEBUG -- /udp1: was told [:read, #<UDPSocket:fd 10>] by #<Thread:0x007fe995160270>
[2016-05-04 11:43:34.946] DEBUG -- /udp1: rejected [:read, #<UDPSocket:fd 10>] from #<Thread:0x007fe995160270>
[2016-05-04 11:43:34.946] DEBUG -- /default_dead_letter_handler: was told #<Concurrent::Actor::Envelope:0x007fe995141ff0 @message=[:read, #<UDPSocket:fd 10>], @future=nil, @sender=#<Thread:0x007fe995160270@examples/udp_server.rb:52 sleep>, @address=#<Concurrent::Actor::Reference:0x7fe99516a9a0 /udp1 (MyUDPServer)>> by #<Concurrent::Actor::Reference:0x7fe99516a9a0 /udp1 (MyUDPServer)>
[2016-05-04 11:43:34.947]  INFO -- /default_dead_letter_handler: got dead letter #<Concurrent::Actor::Envelope:0x007fe995141ff0 @message=[:read, #<UDPSocket:fd 10>], @future=nil, @sender=#<Thread:0x007fe995160270@examples/udp_server.rb:52 sleep>, @address=#<Concurrent::Actor::Reference:0x7fe99516a9a0 /udp1 (MyUDPServer)>>

The messages appear to be received so I imagined that the socket is either never closed or the reopening works but the read handler is never called after the crash

Edit: I use an external client (nc) to send the data to ensure nothing is interacting in either way with the server code, in the server I just create the actor at the end and call sleep

@pitr-ch pitr-ch removed the paused label Feb 23, 2017
@pitr-ch pitr-ch modified the milestones: 1.1.1, 1.1.0 Apr 2, 2017
@pitr-ch
Copy link
Member

pitr-ch commented Apr 2, 2017

Sorry for letting this PR without attention for so long. I'll get back to in next release.

@pitr-ch pitr-ch modified the milestones: 1.1.1, 1.1.0 Feb 21, 2018
@pitr-ch pitr-ch removed this from the 1.1.0 milestone Jul 6, 2018
@pitr-ch pitr-ch added this to Nope in Hackathon Aug 24, 2019
@schmurfy schmurfy closed this Apr 30, 2020
ruby-concurrency/concurrent-ruby automation moved this from Backlog to Done Apr 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants