[
https://issues.apache.org/jira/browse/THRIFT-3737?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15200105#comment-15200105
]
ASF GitHub Bot commented on THRIFT-3737:
----------------------------------------
Github user cherrot commented on the pull request:
https://github.com/apache/thrift/pull/945#issuecomment-198018473
> happybase does not seem to bother with transport state at all; by isOpen
it only avoids "already opened" exceptions.
As what I described in [happybase
issue#112](https://github.com/wbolster/happybase/issues/112), happybase use
`isOpen` (after all there is not other method to do so) to check if it need to
reopen the connection in its connection pool implementation. (See
[pool.py](https://github.com/wbolster/happybase/blob/9cbd718c10a3089f234f1eac1236b631e1f8e7cd/happybase/pool.py#L135-L138)
and
[connection.py](https://github.com/wbolster/happybase/blob/9cbd718c10a3089f234f1eac1236b631e1f8e7cd/happybase/connection.py#L164-L170)).
However in this use case, `isOpen` always returns True.
Consider the following case: suppose Thrift server would drop a connection
which was idle for 2min, and the client side communicates with Thrift server
every 1 to 5min. With the current implementation the client side would
encounter extremely frequent network failures.
> Note that almighty "this connection is fine" guarantee is very costly:
even the current patch does not cover cases like invalidated file descriptor
(i.e. closed socket) and half closed non-writable sockets.
Compared with the former case, this patch is quite efficient (no underlying
network communication, no blocking).
And I don't think it breaks the KISS rule. At least you don't need to put
your failover code everywhere.
Of course we cannot cover all the circumstances of a connection's possible
state, but we can try our best to give a probably healthy connection for the
moment. For other circumstances, it depends on the user's decision (to let it
die or have a retry logic).
After all, what this patch do is providing a convenient way to test the
connection. It never forces developers to use this in their application or libs.
> Improve TSocket isOpen() implementation to give more accurate connection
> status.
> --------------------------------------------------------------------------------
>
> Key: THRIFT-3737
> URL: https://issues.apache.org/jira/browse/THRIFT-3737
> Project: Thrift
> Issue Type: Bug
> Components: Python - Library
> Reporter: Cherrot Luo
> Labels: patch
> Original Estimate: 0h
> Remaining Estimate: 0h
>
> h2. Description
> Typically isOpen() of TSocket.py in python lib would *ALWAYS* return true,
> even if the underlying TCP connection has been closed or reset by the peer,
> unless you manually invoke its close() method.
> This is because the isOpen method takes a simple but kind of "irresponsible"
> way to judge connection status:
> {code:title=TSocket.py}
> def isOpen(self):
> return self.handle is not None
> {code}
> This may affect the downstream lib/tools' implementation to offer a
> *reliable* transport instance.
> For example, in [happybase|https://github.com/wbolster/happybase] (a
> developer-friendly Python library to interact with Apache HBase), it use
> isOpen() to judge whether it neccessary to reopen the connection in its
> [connection|https://github.com/wbolster/happybase/blob/9cbd718c10a3089f234f1eac1236b631e1f8e7cd/happybase/connection.py#L164]
> and connection pool.
> h2. Fix
> I've sent a github [Pull Request|https://github.com/apache/thrift/pull/945]
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
