Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

C11 exception base class changes vs. pool retry #164

Merged
merged 3 commits into from

2 participants

Hanno Schlichting Tyler Hobbs
Hanno Schlichting
Owner

I noticed one unintended side-effect of the thrift API updates. The custom exceptions in ttypes.py now inherit from TException instead of the Python's Exception base class. I think that's a sensible change.

But the retry logic in the connection pool, amongst others retried all exceptions inheriting from TException. This would also retry things like InvalidRequestException or AuthenticationException. I changed the logic to retry TTransportException instead, which restores the old behavior (letting those raise directly and not being covered up in a MaximumRetryException).

Compared to pycassa 1.6.0 TProtocolException is no longer retried. Looking at the places where thrift raises those, they look like general configuration/installation problems and not intermittent problems likely to be fixed by a retry.

hannosch added some commits
Hanno Schlichting hannosch Let the pool retry TTransportException instead of the general TException
With the c11 thrift API update, all exceptions in ttypes inherit from
TException instead of Exception. This change restores the old behavior and
does not retry any of: NotFoundException, InvalidRequestException,
AuthenticationException, AuthorizationException, SchemaDisagreementException

Compared to pycassa 1.6.0 TProtocolException is no longer retried either.
9591f71
Hanno Schlichting hannosch Add explicit test for NotFoundException 83c6da6
Hanno Schlichting hannosch Apply TException to TTransportException change to pool create connection 00cd392
Tyler Hobbs
Owner

Good catch, and thanks for the test!

Tyler Hobbs thobbs merged commit ca12fca into from
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jul 18, 2012
  1. Hanno Schlichting

    Let the pool retry TTransportException instead of the general TException

    hannosch authored
    With the c11 thrift API update, all exceptions in ttypes inherit from
    TException instead of Exception. This change restores the old behavior and
    does not retry any of: NotFoundException, InvalidRequestException,
    AuthenticationException, AuthorizationException, SchemaDisagreementException
    
    Compared to pycassa 1.6.0 TProtocolException is no longer retried either.
  2. Hanno Schlichting
  3. Hanno Schlichting
This page is out of date. Refresh to see the latest.
Showing with 22 additions and 2 deletions.
  1. +4 −2 pycassa/pool.py
  2. +18 −0 tests/test_connection_pooling.py
6 pycassa/pool.py
View
@@ -12,6 +12,7 @@
import Queue
from thrift import Thrift
+from thrift.transport.TTransport import TTransportException
from connection import Connection
from logging.pool_logger import PoolLogger
from util import as_interface
@@ -128,7 +129,8 @@ def new_f(self, *args, **kwargs):
self._pool._decrement_overflow()
self._pool._clear_current()
raise app_exc
- except (TimedOutException, UnavailableException, Thrift.TException,
+ except (TimedOutException, UnavailableException,
+ TTransportException,
socket.error, IOError, EOFError), exc:
self._pool._notify_on_failure(exc, server=self.server, connection=self)
@@ -400,7 +402,7 @@ def _create_connection(self):
server = self._get_next_server()
wrapper = self._get_new_wrapper(server)
return wrapper
- except (Thrift.TException, socket.error, IOError, EOFError), exc:
+ except (TTransportException, socket.error, IOError, EOFError), exc:
self._notify_on_failure(exc, server)
failure_count += 1
raise AllServersUnavailable('An attempt was made to connect to each of the servers ' +
18 tests/test_connection_pooling.py
View
@@ -6,6 +6,9 @@
from pycassa import ColumnFamily, ConnectionPool, PoolListener, InvalidRequestError,\
NoConnectionAvailable, MaximumRetryException, AllServersUnavailable
from pycassa.cassandra.ttypes import ColumnPath
+from pycassa.cassandra.ttypes import InvalidRequestException
+from pycassa.cassandra.ttypes import NotFoundException
+
_credentials = {'username':'jsmith', 'password':'havebadpass'}
@@ -486,6 +489,21 @@ def test_failure_connection_info(self):
assert_equal(request['args'], ('greunt', ColumnPath('Counter1', None, 'col'), 1))
assert_equal(request['kwargs'], {})
+ def test_pool_invalid_request(self):
+ listener = _TestListener()
+ pool = ConnectionPool(pool_size=1, max_overflow=0, recycle=10000,
+ prefill=True, max_retries=3,
+ keyspace='PycassaTestKeyspace',
+ credentials=_credentials,
+ listeners=[listener], use_threadlocal=False,
+ server_list=['localhost:9160'])
+ cf = ColumnFamily(pool, 'Standard1')
+ # Make sure the pool doesn't hide and retries invalid requests
+ assert_raises(InvalidRequestException, cf.add, 'key', 'col')
+ assert_raises(NotFoundException, cf.get, 'none')
+ pool.dispose()
+
+
class _TestListener(PoolListener):
def __init__(self):
Something went wrong with that request. Please try again.