Improved handling of spawned functions in socket server #1524

Closed
MFlyer opened this Issue Nov 15, 2016 · 3 comments

Projects

None yet

2 participants

@MFlyer
Member
MFlyer commented Nov 15, 2016

Hi @schakrava ,
this is a memo/reference for upcoming (next days) PR related to #1522 (comment) and #1510

Testing together on new python-socketio for #1510 is good and now I'll check gevent.spawn + that glitch

Important: on every F5 (page refresh) we get for each gevent.spawn 2 active threads instead of 1 as expected, that should be quite safe (not a real cpu eater), but better to avoid it and close this before 3.8.16 release

Mirko

@MFlyer
Member
MFlyer commented Nov 22, 2016

From now back on this after Samba cycle

@MFlyer
Member
MFlyer commented Nov 23, 2016 edited

Hi @schakrava and @phillxnet ,
today got back to this with first good results :)

First of all, school time!:
being a curious guy had a look to old gevent-socketio code and my reaction was a : "wow!"
Actually I can say it's a shame that code has been abandoned, check this:
http://gevent-socketio.readthedocs.io/en/latest/_modules/socketio/virtsocket.html
Being closely related to gevent, gevent.socketio had a great greenlets handling (spawning, killing, etc).
Unfortunately python-socketio is more generic (works with eventlet, gevent, etc) so it's missing threads handling and we have to code it: let the fun begin!

Roadmap: perform tests with one emitter, find best solutions, extend it to other emitters/classes.

First test

class ServicesNamespace(socketio.Namespace):

    start = False
    threads = [] #1

    def on_connect(self, sid, environ):

        self.emit('connected', {
            'key': 'services:connected', 'data': 'connected'
        })
        self.start = True
        print('Connection opened - sid: %s' % sid) #Debug to supervisord
        sys.stdout.flush()
        self.threads.append(gevent.spawn(self.send_service_statuses, sid)) #2

    def on_disconnect(self, sid):
        print('Greenlets : %s' % self.threads) #Debug to supervisord
        print('Connection closed - sid: %s' % sid)
        sys.stdout.flush()
        gevent.killall(self.threads) #3
        self.threads = [] #3
        self.start = False

    def send_service_statuses(self, sid):

        while self.start:
            print('Emitting to sid: %s' % sid)
            sys.stdout.flush()

To grant an easier debugging had some print to supervisord_data-collector_stdout.log
example:

Emitting to sid: 7b384aa4ef4b4347a3f2055dae808947
Greenlets : [<Greenlet at 0x2bc5050: <bound method ServicesNamespace.send_service_statuses of <smart_manager.data_collector.ServicesNamespace object at 0x28e0f10>>('7b384aa4ef4b4347a3f2055dae808947')>]
Connection closed - sid: 4adcc98a7d6f4daa92116a3fcb26b6ca
Greenlets : [<Greenlet at 0x2bc5050: <bound method ServicesNamespace.send_service_statuses of <smart_manager.data_collector.ServicesNamespace object at 0x28e0f10>>('7b384aa4ef4b4347a3f2055dae808947')>]
Connection closed - sid: 7b384aa4ef4b4347a3f2055dae808947
Greenlets : []
Connection closed - sid: 4adcc98a7d6f4daa92116a3fcb26b6ca
Connection opened - sid: f3a2c084924b4314ae246c4769104c57

Reading code:

  • 1 - we have a new empty array
  • 2 - every spawned action is queued to our array
  • 3 - when a client disconnects we enforce killing all pending greenlets and flush our array

Does this solve zomby threads? Yep!
Is this the best solution? No!
Had an "expected behavior" browsing on Rockstor WebUI (no F5) and it was fine, had a page refresh with F5 and previous thread got killed, but wtih a "obsessive compulsive browsing" (multiple F5/page refreshes) that gevent.killall sometime kills current thread too: nothing serious, changing to another view solves it, but handling it another way (connections dictionary) we can avoid this too.

Probably going to PR before tomorrow

@MFlyer MFlyer added a commit to MFlyer/rockstor-core that referenced this issue Nov 24, 2016
@MFlyer MFlyer Fixes #1524 - Removed client side logging
Signed-off-by: Mirko Arena <mirko.arena@gmail.com>
211c3e2
@MFlyer
Member
MFlyer commented Nov 24, 2016

Hi all,
on #1544 there's a PR solving this, already told @schakrava it's ready for review/merge, but further tests @phillxnet @tomtom13 always appreciated :)

M

@schakrava schakrava added this to the Pinnacles milestone Nov 24, 2016
@MFlyer MFlyer was assigned by schakrava Nov 24, 2016
@MFlyer MFlyer added a commit to MFlyer/rockstor-core that referenced this issue Nov 24, 2016
@MFlyer MFlyer #1524 Fixed removing unrequired keys()
Signed-off-by: Mirko Arena <mirko.arena@gmail.com>
463cc3b
@schakrava schakrava closed this in #1544 Nov 25, 2016
@schakrava schakrava changed the title from [Feature enhancement] New python-socketio not handling spawned functions to Improved handling of spawned functions in socket server Dec 13, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment