You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@couchdb.apache.org by mikewallace1979 <gi...@git.apache.org> on 2015/01/14 20:20:21 UTC
[GitHub] couchdb-couch pull request: Add a configurable timeout for get_pro...
GitHub user mikewallace1979 opened a pull request:
https://github.com/apache/couchdb-couch/pull/31
Add a configurable timeout for get_proc calls
Previously the gen_server calls to couch_proc_manager/get_proc
used a timeout of infinity. There are multiple places in the
couch_proc_manager code path where that process can die without
replying. With an infinity timeout the couch_query_server process
would then hang around forever.
This commit makes the gen_server call to get_proc use a configurable
timeout, set by the couchdb/get_proc_timeout config variable. It
defaults to 60000ms.
Closes:
COUCHDB-2425
COUCHDB-2426
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/mikewallace1979/couchdb-couch 2425-fix-dangling-couch_query_server-procs
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/couchdb-couch/pull/31.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 #31
----
commit c6f3aafd32fe9b3ddfa93634d0a27fb51017db93
Author: Mike Wallace <mi...@apache.org>
Date: 2015-01-14T18:39:44Z
Add a configurable timeout for get_proc calls
Previously the gen_server calls to couch_proc_manager/get_proc
used a timeout of infinity. There are multiple places in the
couch_proc_manager code path where that process can die without
replying. With an infinity timeout the couch_query_server process
would then hang around forever.
This commit makes the gen_server call to get_proc use a configurable
timeout, set by the couchdb/get_proc_timeout config variable. It
defaults to 60000ms.
Closes:
COUCHDB-2425
COUCHDB-2426
----
---
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 pull request: Add a configurable timeout for get_pro...
Posted by mikewallace1979 <gi...@git.apache.org>.
Github user mikewallace1979 commented on the pull request:
https://github.com/apache/couchdb-couch/pull/31#issuecomment-70082220
@kxepal Maybe re-using couchdb/os_process_timeout is actually the best way forward - solves the naming problem and means we're using one timeout for all os process related things, which is simpler from a config point of view. What do you think?
---
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 pull request: Add a configurable timeout for get_pro...
Posted by mikewallace1979 <gi...@git.apache.org>.
Github user mikewallace1979 commented on the pull request:
https://github.com/apache/couchdb-couch/pull/31#issuecomment-70110699
@kxepal The cause of the timeout error will be logged internally by rexi_server so the information is available to the operators. I was reasoning that as the client is external it can't really do anything with the details of what went wrong internally though I guess there's no reason why we shouldn't provide that information.
---
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 pull request: Add a configurable timeout for get_pro...
Posted by mikewallace1979 <gi...@git.apache.org>.
Github user mikewallace1979 commented on the pull request:
https://github.com/apache/couchdb-couch/pull/31#issuecomment-70008468
> get_proc_timeout is good name for internals, but for configuration it should be more user friendly and explain what exactly it controls. Exposing knowledge of gen_server calls to user level configs doesn't simplifies all the things
I agree but I couldn't figure out a better name given that it is pretty much only an internal gen_server concern. Will ponder some more. Maybe it's a sign that exposing it externally is a Bad Idea.
---
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 pull request: Add a configurable timeout for get_pro...
Posted by mikewallace1979 <gi...@git.apache.org>.
Github user mikewallace1979 commented on the pull request:
https://github.com/apache/couchdb-couch/pull/31#issuecomment-70104114
Done.
Since the resulting timeout will be thrown back up to chttpd I think we'll also want https://github.com/apache/couchdb-chttpd/pull/18 so that instead of returning:
```
{"error":"timeout","reason":"{gen_server,call,[couch_proc_manager,{get_proc,<<\"javascript\">>},5000]}"}
```
to the client we instead return our normal timeout message of:
```
{"error":"timeout","reason":"The request could not be processed in a reasonable amount of time."}
```
---
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 pull request: Add a configurable timeout for get_pro...
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:
https://github.com/apache/couchdb-couch/pull/31#issuecomment-70111522
@mikewallace1979 Oh, in this case cool and agreed. All good for me, thanks a lot! Happy to merge, but let's wait for a while for others opinion.
---
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 pull request: Add a configurable timeout for get_pro...
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:
https://github.com/apache/couchdb-couch/pull/31#issuecomment-70086818
@mikewallace1979 I think it's good idea!
---
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 pull request: Add a configurable timeout for get_pro...
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:
https://github.com/apache/couchdb-couch/pull/31#issuecomment-70409254
@mikewallace1979 could you please squash your commits so I merge them?
---
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 pull request: Add a configurable timeout for get_pro...
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:
https://github.com/apache/couchdb-couch/pull/31#issuecomment-69976435
`get_proc_timeout` is good name for internals, but for configuration it should be more user friendly and explain what exactly it controls. Exposing knowledge of gen_server calls to user level configs doesn't simplifies all the things (:
---
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 pull request: Add a configurable timeout for get_pro...
Posted by mikewallace1979 <gi...@git.apache.org>.
Github user mikewallace1979 commented on the pull request:
https://github.com/apache/couchdb-couch/pull/31#issuecomment-70009332
> How about to reuse couch_util:get_value(<<"timeout">>, QueryConfig) for get_proc call?
I'd sort of assumed that we wouldn't want to use the same timeout which the QueryConfig picks up (which will be `couchdb/os_process_timeout`) since this was previously an infinity timeout. Also we don't get the QueryConfig record proplist until the get_proc has received a response.
---
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 pull request: Add a configurable timeout for get_pro...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/couchdb-couch/pull/31
---
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 pull request: Add a configurable timeout for get_pro...
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:
https://github.com/apache/couchdb-couch/pull/31#issuecomment-70903183
Landed, thanks! (:
---
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 pull request: Add a configurable timeout for get_pro...
Posted by mikewallace1979 <gi...@git.apache.org>.
Github user mikewallace1979 commented on the pull request:
https://github.com/apache/couchdb-couch/pull/31#issuecomment-70901683
@kxepal All squashed - sorry for taking my time about it.
@janl +1 on friendliness, though I admit I find it's sometimes a hard thing to achieve in practice :) In this case I think returning the same message we return for all other timeouts is sufficiently friendly - in which case I'll re-open the chttpd [PR](https://github.com/apache/couchdb-chttpd/pull/18).
---
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 pull request: Add a configurable timeout for get_pro...
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:
https://github.com/apache/couchdb-couch/pull/31#issuecomment-69979880
How about to reuse `couch_util:get_value(<<"timeout">>, QueryConfig)` for `get_proc` call?
---
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 pull request: Add a configurable timeout for get_pro...
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:
https://github.com/apache/couchdb-couch/pull/31#issuecomment-70106596
@mikewallace1979 nice idea, but the message hides original issue: what exactly caused timeout error. May be just reformat reason in more human-friendly way?
---
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 pull request: Add a configurable timeout for get_pro...
Posted by janl <gi...@git.apache.org>.
Github user janl commented on the pull request:
https://github.com/apache/couchdb-couch/pull/31#issuecomment-70517554
just a note that “the operators“, at least in the beginning, are end-users as well, so we want to make this as friendly as 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.
---