You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by "Wil Tan (JIRA)" <ji...@apache.org> on 2009/05/08 17:17:45 UTC

[jira] Created: (THRIFT-492) Add Timeout Argument to thrift_client:call()

Add Timeout Argument to thrift_client:call()
--------------------------------------------

                 Key: THRIFT-492
                 URL: https://issues.apache.org/jira/browse/THRIFT-492
             Project: Thrift
          Issue Type: Improvement
          Components: Library (Erlang)
         Environment: N/A
            Reporter: Wil Tan
            Priority: Minor
         Attachments: thrift_client.diff

thrift_client:call() does not use a timeout option when calling gen_server:call(), so the default timeout of 5 seconds is used. It would be good to offer that to the user of the module.

The attached trivial patch does exports a new function call/4 which does this, and makes the original call/3 function pass in the default 5000 ms timeout value.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (THRIFT-492) Add Timeout Argument to thrift_client:call()

Posted by "Wil Tan (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/THRIFT-492?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12891651#action_12891651 ] 

Wil Tan commented on THRIFT-492:
--------------------------------

Please ignore the attached patch, I've updated it to work with trunk. It's on my github fork of David's repos here: http://github.com/wil/thrift/commit/103cee455456b5728e981f7dd4088edb366dfa40

Could someone please advise how I could help to get this merged into the official code base?

IMO it's quite an important issue as the default 5 seconds timeout is too restrictive for many use cases.

Thanks!

> Add Timeout Argument to thrift_client:call()
> --------------------------------------------
>
>                 Key: THRIFT-492
>                 URL: https://issues.apache.org/jira/browse/THRIFT-492
>             Project: Thrift
>          Issue Type: Improvement
>          Components: Library (Erlang)
>         Environment: N/A
>            Reporter: Wil Tan
>            Priority: Minor
>         Attachments: thrift_client-v2.diff
>
>
> thrift_client:call() does not use a timeout option when calling gen_server:call(), so the default timeout of 5 seconds is used. It would be good to offer that to the user of the module.
> The attached trivial patch does exports a new function call/4 which does this, and makes the original call/3 function pass in the default 5000 ms timeout value.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Resolved: (THRIFT-492) Add Timeout Argument to thrift_client:call()

Posted by "Anthony Molinaro (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/THRIFT-492?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Anthony Molinaro resolved THRIFT-492.
-------------------------------------

    Resolution: Invalid

Refactoring made this obsolete.

> Add Timeout Argument to thrift_client:call()
> --------------------------------------------
>
>                 Key: THRIFT-492
>                 URL: https://issues.apache.org/jira/browse/THRIFT-492
>             Project: Thrift
>          Issue Type: Improvement
>          Components: Erlang - Library
>         Environment: N/A
>            Reporter: Wil Tan
>            Priority: Minor
>         Attachments: thrift_client-v2.diff
>
>
> thrift_client:call() does not use a timeout option when calling gen_server:call(), so the default timeout of 5 seconds is used. It would be good to offer that to the user of the module.
> The attached trivial patch does exports a new function call/4 which does this, and makes the original call/3 function pass in the default 5000 ms timeout value.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (THRIFT-492) Add Timeout Argument to thrift_client:call()

Posted by "Wil Tan (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/THRIFT-492?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Wil Tan updated THRIFT-492:
---------------------------

    Attachment: thrift_client.diff

Said patch.

> Add Timeout Argument to thrift_client:call()
> --------------------------------------------
>
>                 Key: THRIFT-492
>                 URL: https://issues.apache.org/jira/browse/THRIFT-492
>             Project: Thrift
>          Issue Type: Improvement
>          Components: Library (Erlang)
>         Environment: N/A
>            Reporter: Wil Tan
>            Priority: Minor
>         Attachments: thrift_client.diff
>
>
> thrift_client:call() does not use a timeout option when calling gen_server:call(), so the default timeout of 5 seconds is used. It would be good to offer that to the user of the module.
> The attached trivial patch does exports a new function call/4 which does this, and makes the original call/3 function pass in the default 5000 ms timeout value.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Closed: (THRIFT-492) Add Timeout Argument to thrift_client:call()

Posted by "Anthony Molinaro (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/THRIFT-492?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Anthony Molinaro closed THRIFT-492.
-----------------------------------


> Add Timeout Argument to thrift_client:call()
> --------------------------------------------
>
>                 Key: THRIFT-492
>                 URL: https://issues.apache.org/jira/browse/THRIFT-492
>             Project: Thrift
>          Issue Type: Improvement
>          Components: Erlang - Library
>         Environment: N/A
>            Reporter: Wil Tan
>            Priority: Minor
>         Attachments: thrift_client-v2.diff
>
>
> thrift_client:call() does not use a timeout option when calling gen_server:call(), so the default timeout of 5 seconds is used. It would be good to offer that to the user of the module.
> The attached trivial patch does exports a new function call/4 which does this, and makes the original call/3 function pass in the default 5000 ms timeout value.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (THRIFT-492) Add Timeout Argument to thrift_client:call()

Posted by "Wil Tan (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/THRIFT-492?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12707385#action_12707385 ] 

Wil Tan commented on THRIFT-492:
--------------------------------

Sorry, I was too eager to submit that patch. Just realized that it doesn't quite solve the issue the issue completely. Some other calls to gen_server:call() are not still not using timeout. I'm digging deeper... will report back. Thanks.

> Add Timeout Argument to thrift_client:call()
> --------------------------------------------
>
>                 Key: THRIFT-492
>                 URL: https://issues.apache.org/jira/browse/THRIFT-492
>             Project: Thrift
>          Issue Type: Improvement
>          Components: Library (Erlang)
>         Environment: N/A
>            Reporter: Wil Tan
>            Priority: Minor
>         Attachments: thrift_client.diff
>
>
> thrift_client:call() does not use a timeout option when calling gen_server:call(), so the default timeout of 5 seconds is used. It would be good to offer that to the user of the module.
> The attached trivial patch does exports a new function call/4 which does this, and makes the original call/3 function pass in the default 5000 ms timeout value.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (THRIFT-492) Add Timeout Argument to thrift_client:call()

Posted by "Todd Lipcon (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/THRIFT-492?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12707374#action_12707374 ] 

Todd Lipcon commented on THRIFT-492:
------------------------------------

Looks good to me

> Add Timeout Argument to thrift_client:call()
> --------------------------------------------
>
>                 Key: THRIFT-492
>                 URL: https://issues.apache.org/jira/browse/THRIFT-492
>             Project: Thrift
>          Issue Type: Improvement
>          Components: Library (Erlang)
>         Environment: N/A
>            Reporter: Wil Tan
>            Priority: Minor
>         Attachments: thrift_client.diff
>
>
> thrift_client:call() does not use a timeout option when calling gen_server:call(), so the default timeout of 5 seconds is used. It would be good to offer that to the user of the module.
> The attached trivial patch does exports a new function call/4 which does this, and makes the original call/3 function pass in the default 5000 ms timeout value.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (THRIFT-492) Add Timeout Argument to thrift_client:call()

Posted by "Wil Tan (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/THRIFT-492?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Wil Tan updated THRIFT-492:
---------------------------

    Attachment: thrift_client-v2.diff

I'm using framed transport, and found that making the gen_server:call() timeout infinity for reads fixes the issue for me, i.e. the timeout is determined by the initial thrift_client:call(), so we can allow the underlying transport to take as long as it wants.

If you think that this is patch is the way to go, then maybe other transports would need similar fixes as well, but I don't use them so I'm not so comfortable patching them.

> Add Timeout Argument to thrift_client:call()
> --------------------------------------------
>
>                 Key: THRIFT-492
>                 URL: https://issues.apache.org/jira/browse/THRIFT-492
>             Project: Thrift
>          Issue Type: Improvement
>          Components: Library (Erlang)
>         Environment: N/A
>            Reporter: Wil Tan
>            Priority: Minor
>         Attachments: thrift_client-v2.diff, thrift_client.diff
>
>
> thrift_client:call() does not use a timeout option when calling gen_server:call(), so the default timeout of 5 seconds is used. It would be good to offer that to the user of the module.
> The attached trivial patch does exports a new function call/4 which does this, and makes the original call/3 function pass in the default 5000 ms timeout value.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (THRIFT-492) Add Timeout Argument to thrift_client:call()

Posted by "Wil Tan (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/THRIFT-492?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Wil Tan updated THRIFT-492:
---------------------------

    Attachment:     (was: thrift_client.diff)

> Add Timeout Argument to thrift_client:call()
> --------------------------------------------
>
>                 Key: THRIFT-492
>                 URL: https://issues.apache.org/jira/browse/THRIFT-492
>             Project: Thrift
>          Issue Type: Improvement
>          Components: Library (Erlang)
>         Environment: N/A
>            Reporter: Wil Tan
>            Priority: Minor
>         Attachments: thrift_client-v2.diff
>
>
> thrift_client:call() does not use a timeout option when calling gen_server:call(), so the default timeout of 5 seconds is used. It would be good to offer that to the user of the module.
> The attached trivial patch does exports a new function call/4 which does this, and makes the original call/3 function pass in the default 5000 ms timeout value.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (THRIFT-492) Add Timeout Argument to thrift_client:call()

Posted by "Anthony Molinaro (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/THRIFT-492?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12900780#action_12900780 ] 

Anthony Molinaro commented on THRIFT-492:
-----------------------------------------

I'm pretty sure the refactoring https://issues.apache.org/jira/browse/THRIFT-599 means this ticket can be closed.   thrift_client is no longer a gen_server that is left up to the application developer.  The timeout controls are provided by the underlying erlang connection.

> Add Timeout Argument to thrift_client:call()
> --------------------------------------------
>
>                 Key: THRIFT-492
>                 URL: https://issues.apache.org/jira/browse/THRIFT-492
>             Project: Thrift
>          Issue Type: Improvement
>          Components: Erlang - Library
>         Environment: N/A
>            Reporter: Wil Tan
>            Priority: Minor
>         Attachments: thrift_client-v2.diff
>
>
> thrift_client:call() does not use a timeout option when calling gen_server:call(), so the default timeout of 5 seconds is used. It would be good to offer that to the user of the module.
> The attached trivial patch does exports a new function call/4 which does this, and makes the original call/3 function pass in the default 5000 ms timeout value.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.