You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by nickva <gi...@git.apache.org> on 2015/10/02 16:06:56 UTC
[GitHub] couchdb-couch-replicator pull request: Handle un-expected closing ...
GitHub user nickva opened a pull request:
https://github.com/apache/couchdb-couch-replicator/pull/19
Handle un-expected closing of pipelined connections better.
If during a pipelined connection, server closes its socket,
but http client has more requests to send, ibrowse will
detect that when it sends next request and throw
{error, connection_closing}.
Handle that error better, by closing the socket explicitly
and retrying the pipelined request that failed.
COUCHDB-2833
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/nickva/couchdb-couch-replicator 2833-handle-unexpected-connection-close
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/couchdb-couch-replicator/pull/19.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #19
----
commit 85d62d1e2b72250c329b4b10db4df1231e647933
Author: Nick Vatamaniuc <va...@gmail.com>
Date: 2015-10-02T13:55:42Z
Handle un-expected closing of pipelined connections better.
If during a pipelined connection, server closes its socket,
but http client has more requests to send, ibrowse will
detect that when it sends next request and throw
{error, connection_closing}.
Handle that error better, by closing the socket explicitly
and retrying the pipelined request that failed.
COUCHDB-2833
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch-replicator pull request: Handle un-expected closing ...
Posted by davisp <gi...@git.apache.org>.
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-couch-replicator/pull/19#issuecomment-145047426
Possible.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch-replicator pull request: Handle un-expected closing ...
Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on the pull request:
https://github.com/apache/couchdb-couch-replicator/pull/19#issuecomment-145484521
ok, here's how to make mochi send a Connection:close, you need to terminate request handling before we call json_body() (or equivalent). You can achieve that with `PUT /db/doc?open_revs=foo` as the JSON_DECODE of 'foo' will throw. The response will include a Connection:close header, which means any body bytes unread are discarded.
All this is telling us we need a higher level abstraction for our http code.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch-replicator pull request: Handle un-expected closing ...
Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on the pull request:
https://github.com/apache/couchdb-couch-replicator/pull/19#issuecomment-145483457
for a PUT /db/doc where the body has more bytes than Content-Length, couchdb just hangs, so it's not that.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch-replicator pull request: Handle un-expected closing ...
Posted by nickva <gi...@git.apache.org>.
Github user nickva commented on the pull request:
https://github.com/apache/couchdb-couch-replicator/pull/19#issuecomment-145039321
We know how to replicate the issue if we need to -- comment out this patch.
@rnewson I'll investigate the server next.
In general servers can choose to close the connection whenever they want, maybe due to concurrency throttling, or say even couchdb behind nginx proxy, it could have a setting to close long lived pipelined requests. It was kind of a nice bonus that changes in the server exposed this bug.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch-replicator pull request: Handle un-expected closing ...
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:
https://github.com/apache/couchdb-couch-replicator/pull/19#issuecomment-145032016
I like the fix, but I think it will hide our current issue now. Previously, we didn't suffer from this. Are there any other ways to prevent our own server to close connection when it not supposed to?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch-replicator pull request: Handle un-expected closing ...
Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on the pull request:
https://github.com/apache/couchdb-couch-replicator/pull/19#issuecomment-145045966
mochiweb is newly sending Connection:close when it determines should_close() is true. mochiweb is right, so I infer that we send some extra garbage in the PUT request (more bytes that Content-Length).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch-replicator pull request: Handle un-expected closing ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/couchdb-couch-replicator/pull/19
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch-replicator pull request: Handle un-expected closing ...
Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on the pull request:
https://github.com/apache/couchdb-couch-replicator/pull/19#issuecomment-145037688
+1, though we should find out why we get Connection:close for a successful PUT, it's important to reuse sockets when we can.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch-replicator pull request: Handle un-expected closing ...
Posted by davisp <gi...@git.apache.org>.
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-couch-replicator/pull/19#issuecomment-145045338
+1 to this change and +1 to finding why we're suddenly returning Connection: close for a succesful PUT in another bug. I'd just say that before we close this lets make sure we have a separate test case that reproduces the problem outside the replicator (which sounds easy based on my reading of 2833).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch-replicator pull request: Handle un-expected closing ...
Posted by nickva <gi...@git.apache.org>.
Github user nickva commented on the pull request:
https://github.com/apache/couchdb-couch-replicator/pull/19#issuecomment-145033850
This is to make our push replication side more robust. It's a good thing the server behaved in this new way, as it let us discover this case. In theory, replication protocol should be working with other (non-couchdb) servers and it needs to be robust. We can treat the two issues -- "http replication client robustness", and "server closing connection too quickly" as separate.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch-replicator pull request: Handle un-expected closing ...
Posted by nickva <gi...@git.apache.org>.
Github user nickva commented on the pull request:
https://github.com/apache/couchdb-couch-replicator/pull/19#issuecomment-145599470
@rnewson @davisp @kxepal
When processing the request we set mochiweb_request_recv = true in the process dictionary too late. That makes it return Connection: close. ibrowse used to ignore Connection: close and just kept the socket open. Then ibrowse was fixed and revealed the issue.
Here is a potential fix and the details: https://issues.apache.org/jira/browse/COUCHDB-2834
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch-replicator pull request: Handle un-expected closing ...
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:
https://github.com/apache/couchdb-couch-replicator/pull/19#issuecomment-145035378
Yes, I agree with that. My point is that applying this now will hide second issue behind all passed tests.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---