CouchDB
  1. CouchDB
  2. COUCHDB-1310

'/_restart' closes request socket before sending a response

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Won't Fix
    • Affects Version/s: 1.1
    • Fix Version/s: 1.1
    • Component/s: HTTP Interface
    • Labels:
      None
    • Environment:

      All platforms. Tested on Ubuntu 10.10.

    • Skill Level:
      New Contributors Level (Easy)

      Description

      In 'couchdb_misc_handlers.erl', `handle_restart_req(...)` calls `couch_server_sup:restart_core_server()` before sending the HTTP response. This causes the current request's socket to be closed without sending any response back to the client. From the perspective of the HTTP client, the connection is simply dropped - no response is ever received.

      In addition to the obvious aesthetic problems here, the HTTP1.1 spec suggests a specific (and non-desirable) client-side behavior for connections that drop like this. From RFC-2616 Section-8.2.4: "...if the client sees the connection close before receiving any status from the server, the client SHOULD retry the request..." (http://tools.ietf.org/html/rfc2616#section-8.2.4). Any HTTP client that actually obeys this direction, would end up restarting the server multiple times.

      I have a patch that fixes this issue. I will attach it to this report.

      This issue may be related to COUCHDB-946 (https://issues.apache.org/jira/browse/COUCHDB-946).

        Activity

        Hide
        J. Lee Coltrane added a comment - - edited

        Attached a patch that fixes this issue in 1.1.0.

        Show
        J. Lee Coltrane added a comment - - edited Attached a patch that fixes this issue in 1.1.0.
        Hide
        Paul Joseph Davis added a comment -

        I'd point out that _restart is really only meant for test code to get the server into a known state. This patch merely hides the issue and doesn't really address the underlying problem, namely, that init:restart is a hard core reset of the VM. If there are concurrent clients accessing the server they will be just as exposed to the original problem. About the only thing to do would be to figure out how to gracefully quit mochiweb before calling init:restart. Its not out of the question, but we moved to this approach when the graceful one didn't work as reliably.

        Show
        Paul Joseph Davis added a comment - I'd point out that _restart is really only meant for test code to get the server into a known state. This patch merely hides the issue and doesn't really address the underlying problem, namely, that init:restart is a hard core reset of the VM. If there are concurrent clients accessing the server they will be just as exposed to the original problem. About the only thing to do would be to figure out how to gracefully quit mochiweb before calling init:restart. Its not out of the question, but we moved to this approach when the graceful one didn't work as reliably.
        Hide
        J. Lee Coltrane added a comment -

        I agree: If '/_restart' were to become a truly "useful" part of the api, it would need to do a graceful shutdown. This might be a useful feature to add, but it's certainly not provided by the attached patch.

        However, as it stands today, the tests do depend on the '/_restart' interface, and the issue reported here appears to be the cause of a lot of the flakey test-suite behavior that I (and others) have observed in 1.1.0. This is especially true when accessing couchdb through a proxy, as the proxy's handling of the dropped connection varies depending on the proxy implementation. Unfortunately, this is also one of the cases where the test suite would be most useful – verifying the correctness of a custom reverse-proxy implementation for use in-front-of couchdb.

        There are various notes in the dev-list archives about the tests hanging (and other flakey behavior).
        With this patch in place, I'm no longer seeing tests hang like that. (I'm still seeing some scattered issues from the test suite – but no hanging)

        Show
        J. Lee Coltrane added a comment - I agree: If '/_restart' were to become a truly "useful" part of the api, it would need to do a graceful shutdown. This might be a useful feature to add, but it's certainly not provided by the attached patch. However, as it stands today, the tests do depend on the '/_restart' interface, and the issue reported here appears to be the cause of a lot of the flakey test-suite behavior that I (and others) have observed in 1.1.0. This is especially true when accessing couchdb through a proxy, as the proxy's handling of the dropped connection varies depending on the proxy implementation. Unfortunately, this is also one of the cases where the test suite would be most useful – verifying the correctness of a custom reverse-proxy implementation for use in-front-of couchdb. There are various notes in the dev-list archives about the tests hanging (and other flakey behavior). With this patch in place, I'm no longer seeing tests hang like that. (I'm still seeing some scattered issues from the test suite – but no hanging)
        Hide
        Paul Joseph Davis added a comment -

        quite possible that it makes the hangs better. The test that we currently have for detecting when the server came back is less than robust. I've tried to fix it a couple times but never managed to find something that was significantly less hacky.

        Show
        Paul Joseph Davis added a comment - quite possible that it makes the hangs better. The test that we currently have for detecting when the server came back is less than robust. I've tried to fix it a couple times but never managed to find something that was significantly less hacky.
        Hide
        Jan Lehnardt added a comment -

        I don't think the test suite is supposed to support a proxy setup.

        We added "verify installation" to Futon to cover the basics without running the full test suite for situations like yours.

        Show
        Jan Lehnardt added a comment - I don't think the test suite is supposed to support a proxy setup. We added "verify installation" to Futon to cover the basics without running the full test suite for situations like yours.
        Hide
        Randall Leeds added a comment -

        Impossible to send a response after the server has restarted... the socket has been closed as part of the restart.

        Show
        Randall Leeds added a comment - Impossible to send a response after the server has restarted... the socket has been closed as part of the restart.

          People

          • Assignee:
            Unassigned
            Reporter:
            J. Lee Coltrane
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development