Uploaded image for project: 'Couchbase Python Client Library'
  1. Couchbase Python Client Library
  2. PYCBC-463

TXIoEvent errors out application upon connection loss

    XMLWordPrintable

Details

    • Bug
    • Status: Closed
    • Major
    • Resolution: Fixed
    • 2.3.3
    • 2.3.4
    • library
    • None

    Description

      When a connection is lost, tornado will callback to the twisted object to tell it about the failure.

      The problem is it differentiates read failures and write failures using the readConnectionLost and writeConnectionLost methods.

      Our TxIOEvent class actually only has a generic connectionLost method.
      To make the API compatible without breaking anything or changing behaviour for anything else, we should add readConnectionLost and writeConnectionLost methods which in turn just call the existing connectionLost method.

      Example of the error which will crash the coroutine:

      2018-01-12T07:20:11.445581000Z ERROR|tornado.application: Exception in callback None
      2018-01-12T07:20:11.446154000Z Traceback (most recent call last):
      2018-01-12T07:20:11.446654000Z   File "/usr/local/lib/pypy2.7/dist-packages/tornado/ioloop.py", line 888, in start
      2018-01-12T07:20:11.447054000Z     handler_func(fd_obj, events)
      2018-01-12T07:20:11.447484000Z   File "/usr/local/lib/pypy2.7/dist-packages/tornado/stack_context.py", line 277, in null_wrapper
      2018-01-12T07:20:11.448008000Z     return fn(*args, **kwargs)
      2018-01-12T07:20:11.448443000Z   File "/usr/local/lib/pypy2.7/dist-packages/tornado/platform/twisted.py", line 196, in _invoke_callback
      2018-01-12T07:20:11.448846000Z     reader.readConnectionLost(failure.Failure(err))
      2018-01-12T07:20:11.449260000Z AttributeError: 'TxIOEvent' object has no attribute 'readConnectionLost'
      

      Attachments

        For Gerrit Dashboard: PYCBC-463
        # Subject Branch Project Status CR V

        Activity

          matt.carabine Matt Carabine created issue -
          matt.carabine Matt Carabine made changes -
          Field Original Value New Value
          Description When a connection is lost, tornado will callback to the twisted object to tell it about the failure.

          The problem is it differentiates read failures and write failures using the {{readConnectionLost}} and {{writeConnectionLost}} methods.

          Our TxIOEvent class actually only has a generic {{connectionLost}} method.
          To make the API compatible without breaking anything or changing behaviour for anything else, we should add {{readConnectionLost}} and {{writeConnectionLost}} methods which in turn just call the existing {{connectionLost}} method.
          When a connection is lost, tornado will callback to the twisted object to tell it about the failure.

          The problem is it differentiates read failures and write failures using the {{readConnectionLost}} and {{writeConnectionLost}} methods.

          Our TxIOEvent class actually only has a generic {{connectionLost}} method.
          To make the API compatible without breaking anything or changing behaviour for anything else, we should add {{readConnectionLost}} and {{writeConnectionLost}} methods which in turn just call the existing {{connectionLost}} method.

          Example of the error which will crash the coroutine:

          {noformat}
          2018-01-12T07:20:11.445581000Z ERROR|tornado.application: Exception in callback None
          2018-01-12T07:20:11.446154000Z Traceback (most recent call last):
          2018-01-12T07:20:11.446654000Z File "/usr/local/lib/pypy2.7/dist-packages/tornado/ioloop.py", line 888, in start
          2018-01-12T07:20:11.447054000Z handler_func(fd_obj, events)
          2018-01-12T07:20:11.447484000Z File "/usr/local/lib/pypy2.7/dist-packages/tornado/stack_context.py", line 277, in null_wrapper
          2018-01-12T07:20:11.448008000Z return fn(*args, **kwargs)
          2018-01-12T07:20:11.448443000Z File "/usr/local/lib/pypy2.7/dist-packages/tornado/platform/twisted.py", line 196, in _invoke_callback
          2018-01-12T07:20:11.448846000Z reader.readConnectionLost(failure.Failure(err))
          2018-01-12T07:20:11.449260000Z AttributeError: 'TxIOEvent' object has no attribute 'readConnectionLost'
          {noformat}
          Ellis.Breen Ellis Breen made changes -
          Status New [ 10003 ] Open [ 1 ]
          Ellis.Breen Ellis Breen made changes -
          Fix Version/s 2.3.4 [ 15051 ]

          Technically I think the API is supposed to mirror the one listed here, but can't find a definitive enough reference for that:

          http://twistedmatrix.com/documents/current/api/twisted.internet.tcp.Connection.html

          I have just implemented the bare-bones to avoid the crash we have been seeing under Tornado.

          matt.carabine Matt Carabine added a comment - Technically I think the API is supposed to mirror the one listed here, but can't find a definitive enough reference for that: http://twistedmatrix.com/documents/current/api/twisted.internet.tcp.Connection.html I have just implemented the bare-bones to avoid the crash we have been seeing under Tornado.
          Ellis.Breen Ellis Breen added a comment - - edited

          Hi Matt, I have approved your changes as they appear to be fairly benign/small (just calling the superclass member). I'll try to do some extra testing with Twisted but looks fine to me. Let me know if you hit any more problems with this specific issue.

          Ellis.Breen Ellis Breen added a comment - - edited Hi Matt, I have approved your changes as they appear to be fairly benign/small (just calling the superclass member). I'll try to do some extra testing with Twisted but looks fine to me. Let me know if you hit any more problems with this specific issue.
          Ellis.Breen Ellis Breen added a comment -

          Hi Matt, have you seen any more issues related to this? Would you be happy to close this if not?

          Thanks,

          Ellis

          Ellis.Breen Ellis Breen added a comment - Hi Matt, have you seen any more issues related to this? Would you be happy to close this if not? Thanks, Ellis

          Apologies, so far so good. I've created myself a custom build with this on top of 2.0.7 (for use with PyPy) and haven't seen any issues since.

          Will close this.

          matt.carabine Matt Carabine added a comment - Apologies, so far so good. I've created myself a custom build with this on top of 2.0.7 (for use with PyPy) and haven't seen any issues since. Will close this.
          matt.carabine Matt Carabine made changes -
          Resolution Fixed [ 1 ]
          Status Open [ 1 ] Closed [ 6 ]
          Ellis.Breen Ellis Breen added a comment -

          Ah, thanks. Really hope I can address PyPy soon. I gather cpyext is better than it used to be, but evidently not perfect.

          Ellis.Breen Ellis Breen added a comment - Ah, thanks. Really hope I can address PyPy soon. I gather cpyext is better than it used to be, but evidently not perfect.

          People

            matt.carabine Matt Carabine
            matt.carabine Matt Carabine
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Gerrit Reviews

                There are no open Gerrit changes

                PagerDuty