Tornado client does not handle out of order responses correctly

[matthiasvegh]

Given a thrift file:

service TestService {
    i32 sleep (1: i32 ms)
}

thriftpy.tornado client:

#!/usr/bin/env python2

import thriftpy
import thriftpy.tornado
import tornado
import tornado.ioloop
import tornado.gen

thriftpy.install_import_hook()
import test_thrift

import functools

@tornado.gen.coroutine
def run(ioloop):
    client = yield thriftpy.tornado.make_client(
        test_thrift.TestService, '127.0.0.1', 9090, io_loop=ioloop)
    results = yield [client.sleep(50), client.sleep(100), client.sleep(10)]
    print results

def main():
    ioloop = tornado.ioloop.IOLoop.current()
    ioloop.run_sync(functools.partial(run, ioloop))

if __name__ == "__main__":
    main()

and a thrift server that reponds to sleep(n) by returning n after n ms has elapsed, the expected behavior is [50, 100, 10] in the order as called. Instead I get [10, 50, 100] which is the order of responses.

I have also tried using tornado's Future.add_done_callback, and tornado's WaitIterator, in the hopes that yielding lists in this manner aren't supposed to work, but both result in the same issue: the Future of the call is not matched to the response read.

Investigation into the code and some network-level debugging shows that the sequenceID of the thrift requests is not being incremented by thriftpy's tornado client. The first request should use 0, the second 1 and so on. When the response arrives, this sequenceID can be used to match the response to the future of the request. This omission is fine for synchronous clients, where only one request can be outstanding, in an asynchronous client, several requests may be posted at once. The problem would likely be even worse if unrelated functions (returning different types) are used in this manner.

I tried using a thriftpy.tornado based Server, but handler functions can not be invoked simultaneously, as the first response is waited for before reading the next request from the socket as shown here

Twisted thrift server that reorders requests: Build thrift file with thrift --gen py:twisted test.thrift

#!/usr/bin/env python2

import os
import sys

sys.path.insert(1, os.path.join(os.path.dirname(__file__), "gen-py.twisted"))

import test
import test.TestService
import twisted
import twisted.internet
import twisted.internet.task
import twisted.internet.defer
import twisted.internet.reactor
import twisted.python
import twisted.python.log

import thrift
import thrift.transport
import thrift.transport.TTwisted
import thrift.protocol
import thrift.protocol.TBinaryProtocol

import zope
import zope.interface

class TestHandler:
    zope.interface.implements(test.TestService.Iface)

    def __init__(self):
        self.reactor = twisted.internet.reactor

    def sleep(self, ms):
        print('got request: ' + str(ms))
        d = twisted.internet.defer.Deferred()
        twisted.internet.reactor.callLater(ms/1000.0, d.callback, ms)
        return d

def main():
    twisted.python.log.startLogging(sys.stdout)
    handler = TestHandler()
    processor = test.TestService.Processor(handler)
    factory = thrift.protocol.TBinaryProtocol.TBinaryProtocolFactory()

    server = twisted.internet.reactor.listenTCP(9090,
        thrift.transport.TTwisted.ThriftServerFactory(processor, factory))
    twisted.internet.reactor.run()

if __name__ == "__main__":
    main()

˙ Thanks for your help!

[Ferenc-]

It would be really nice to have a fix for this. Is there somebody out there who could help by at least outlining what needs to be done with this?