Skip to content

Commit 087fe8c

Browse files
committed
3.x: fail fast on permanent keyspace setup failures
Keep keyspace setup failures on the borrow path from corrupting pool accounting, and surface validation failures from the internal USE query as request failures instead of treating them as host failures. Validation errors such as missing keyspaces or authorization problems are now returned as typed query errors without defuncting the connection or exhausting the query plan. Transient connection and busy-pool failures continue through the existing retry/next-host path. Tests cover failed keyspace future cleanup, pool counter restoration, non-defunct validation failures, and fail-fast request behavior.
1 parent b666e09 commit 087fe8c

4 files changed

Lines changed: 227 additions & 5 deletions

File tree

driver-core/src/main/java/com/datastax/driver/core/Connection.java

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import com.datastax.driver.core.exceptions.DriverInternalError;
3535
import com.datastax.driver.core.exceptions.FrameTooLongException;
3636
import com.datastax.driver.core.exceptions.OperationTimedOutException;
37+
import com.datastax.driver.core.exceptions.QueryValidationException;
3738
import com.datastax.driver.core.exceptions.TransportException;
3839
import com.datastax.driver.core.exceptions.UnsupportedProtocolVersionException;
3940
import com.datastax.driver.core.utils.MoreFutures;
@@ -900,7 +901,17 @@ ListenableFuture<Connection> setKeyspaceAsync(final String keyspace)
900901
logger.debug("{} Setting keyspace {}", this, keyspace);
901902
// Note: we quote the keyspace below, because the name is the one coming from Cassandra, so
902903
// it's in the right case already
903-
Future future = write(new Requests.Query("USE \"" + keyspace + '"'));
904+
Future future;
905+
try {
906+
future = write(new Requests.Query("USE \"" + keyspace + '"'));
907+
} catch (ConnectionException | BusyConnectionException e) {
908+
targetKeyspace.compareAndSet(attempt, defaultKeyspaceAttempt);
909+
ksFuture.setException(e);
910+
return ksFuture;
911+
} catch (RuntimeException e) {
912+
targetKeyspace.compareAndSet(attempt, defaultKeyspaceAttempt);
913+
throw e;
914+
}
904915
Futures.addCallback(
905916
future,
906917
new FutureCallback<Message.Response>() {
@@ -915,7 +926,12 @@ public void onSuccess(Message.Response response) {
915926
targetKeyspace.compareAndSet(attempt, defaultKeyspaceAttempt);
916927
if (response.type == ERROR) {
917928
Responses.Error error = (Responses.Error) response;
918-
ksFuture.setException(defunct(error.asException(endPoint)));
929+
DriverException exception = error.asException(endPoint);
930+
if (exception instanceof QueryValidationException) {
931+
ksFuture.setException(exception);
932+
} else {
933+
ksFuture.setException(defunct(exception));
934+
}
919935
} else {
920936
ksFuture.setException(
921937
defunct(

driver-core/src/main/java/com/datastax/driver/core/HostConnectionPool.java

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import static com.datastax.driver.core.Connection.State.TRASHED;
2828

2929
import com.datastax.driver.core.exceptions.AuthenticationException;
30+
import com.datastax.driver.core.exceptions.BusyConnectionException;
3031
import com.datastax.driver.core.exceptions.BusyPoolException;
3132
import com.datastax.driver.core.exceptions.ConnectionException;
3233
import com.datastax.driver.core.exceptions.UnsupportedProtocolVersionException;
@@ -611,7 +612,22 @@ ListenableFuture<Connection> borrowConnection(
611612
if (totalInFlightCount > currentCapacity) maybeSpawnNewConnection(shardId);
612613
}
613614

614-
return leastBusy.setKeyspaceAsync(manager.poolsState.keyspace);
615+
final Connection borrowedConnection = leastBusy;
616+
ListenableFuture<Connection> setKeyspaceFuture =
617+
borrowedConnection.setKeyspaceAsync(manager.poolsState.keyspace);
618+
Futures.addCallback(
619+
setKeyspaceFuture,
620+
new FutureCallback<Connection>() {
621+
@Override
622+
public void onSuccess(Connection connection) {}
623+
624+
@Override
625+
public void onFailure(Throwable t) {
626+
borrowedConnection.release(t instanceof BusyConnectionException);
627+
}
628+
},
629+
MoreExecutors.directExecutor());
630+
return setKeyspaceFuture;
615631
}
616632

617633
private ListenableFuture<Connection> enqueue(
@@ -698,8 +714,14 @@ private void dequeue(final Connection connection) {
698714
pendingBorrowCount.decrementAndGet();
699715
// Ensure that the keyspace set on the connection is the one set on the pool state, in the
700716
// general case it will be.
701-
ListenableFuture<Connection> setKeyspaceFuture =
702-
connection.setKeyspaceAsync(manager.poolsState.keyspace);
717+
ListenableFuture<Connection> setKeyspaceFuture;
718+
try {
719+
setKeyspaceFuture = connection.setKeyspaceAsync(manager.poolsState.keyspace);
720+
} catch (ConnectionException | BusyConnectionException e) {
721+
pendingBorrow.setException(e);
722+
connection.inFlight.decrementAndGet();
723+
continue;
724+
}
703725
// Slight optimization, if the keyspace was already correct the future will be complete, so
704726
// simply complete it here.
705727
if (setKeyspaceFuture.isDone()) {

driver-core/src/main/java/com/datastax/driver/core/RequestHandler.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import com.datastax.driver.core.exceptions.NoHostAvailableException;
3232
import com.datastax.driver.core.exceptions.OperationTimedOutException;
3333
import com.datastax.driver.core.exceptions.OverloadedException;
34+
import com.datastax.driver.core.exceptions.QueryValidationException;
3435
import com.datastax.driver.core.exceptions.ReadFailureException;
3536
import com.datastax.driver.core.exceptions.ReadTimeoutException;
3637
import com.datastax.driver.core.exceptions.ServerError;
@@ -468,6 +469,10 @@ public void onSuccess(Connection connection) {
468469

469470
@Override
470471
public void onFailure(Throwable t) {
472+
if (t instanceof QueryValidationException) {
473+
setFinalException(null, (QueryValidationException) t);
474+
return;
475+
}
471476
if (t instanceof BusyPoolException) {
472477
logError(host.getEndPoint(), t);
473478
} else {

driver-core/src/test/java/com/datastax/driver/core/HostConnectionPoolTest.java

Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@
5050
import com.datastax.driver.core.exceptions.BusyPoolException;
5151
import com.datastax.driver.core.exceptions.ConnectionException;
5252
import com.datastax.driver.core.exceptions.DriverException;
53+
import com.datastax.driver.core.exceptions.InvalidQueryException;
5354
import com.datastax.driver.core.exceptions.NoHostAvailableException;
5455
import com.datastax.driver.core.policies.ConstantReconnectionPolicy;
5556
import com.google.common.base.Function;
@@ -62,6 +63,9 @@
6263
import com.google.common.util.concurrent.ListeningExecutorService;
6364
import com.google.common.util.concurrent.MoreExecutors;
6465
import com.google.common.util.concurrent.Uninterruptibles;
66+
import io.netty.buffer.ByteBuf;
67+
import io.netty.buffer.Unpooled;
68+
import io.netty.util.CharsetUtil;
6569
import java.net.InetSocketAddress;
6670
import java.nio.ByteBuffer;
6771
import java.util.Collection;
@@ -134,6 +138,16 @@ private void assertBorrowedConnection(
134138
assertBorrowedConnections(requests, Collections.singletonList(expectedConnection));
135139
}
136140

141+
private static Responses.Error errorResponse(ExceptionCode code, String message) {
142+
ByteBuf body = Unpooled.buffer();
143+
body.writeInt(code.value);
144+
byte[] messageBytes = message.getBytes(CharsetUtil.UTF_8);
145+
body.writeShort(messageBytes.length);
146+
body.writeBytes(messageBytes);
147+
return Responses.Error.decoder.decode(
148+
body, ProtocolVersion.V4, CodecRegistry.DEFAULT_INSTANCE, ProtocolFeatureStore.EMPTY);
149+
}
150+
137151
/**
138152
* Ensures that if a fixed-sized pool has filled its core connections and reached its maximum
139153
* number of enqueued requests, then borrowConnection will fail instead of creating a new
@@ -393,6 +407,158 @@ public void should_adjust_connection_keyspace_on_dequeue_if_pool_state_is_differ
393407
}
394408
}
395409

410+
/**
411+
* Ensures that if keyspace setup fails while borrowing a connection, the borrow future fails and
412+
* pool accounting is restored.
413+
*
414+
* @test_category connection:connection_pool
415+
*/
416+
@Test(groups = "short")
417+
public void should_restore_pool_accounting_when_keyspace_setup_fails_on_borrow()
418+
throws Exception {
419+
Cluster cluster = createClusterBuilder().build();
420+
try {
421+
HostConnectionPool pool = createPool(cluster, 1, 1);
422+
Connection connection = spy(pool.connections[0].get(0));
423+
pool.connections[0].set(0, connection);
424+
425+
pool.manager.poolsState.setKeyspace("newkeyspace");
426+
ConnectionException failure =
427+
new ConnectionException(pool.host.getEndPoint(), "Write attempt on defunct connection");
428+
doReturn(Futures.<Connection>immediateFailedFuture(failure))
429+
.when(connection)
430+
.setKeyspaceAsync("newkeyspace");
431+
432+
MockRequest request = MockRequest.send(pool);
433+
434+
try {
435+
Uninterruptibles.getUninterruptibly(request.connectionFuture, 5, TimeUnit.SECONDS);
436+
fail("Should have failed to borrow connection");
437+
} catch (ExecutionException e) {
438+
assertThat(e.getCause()).isSameAs(failure);
439+
}
440+
assertThat(connection.inFlight.get()).isEqualTo(0);
441+
assertThat(pool.totalInFlight.get()).isEqualTo(0);
442+
} finally {
443+
cluster.close();
444+
}
445+
}
446+
447+
/**
448+
* Ensures that a synchronous write failure while setting the keyspace is reported through the
449+
* returned future and does not leave the failed keyspace attempt in-flight.
450+
*
451+
* @test_category connection:connection_pool
452+
*/
453+
@Test(groups = "short")
454+
public void should_fail_keyspace_future_and_clear_attempt_when_connection_is_defunct()
455+
throws Exception {
456+
Cluster cluster = createClusterBuilder().build();
457+
try {
458+
HostConnectionPool pool = createPool(cluster, 1, 1);
459+
Connection connection = pool.connections[0].get(0);
460+
461+
connection.defunct(
462+
new ConnectionException(pool.host.getEndPoint(), "Test defunct connection"));
463+
464+
ListenableFuture<Connection> firstAttempt = connection.setKeyspaceAsync("newkeyspace");
465+
ListenableFuture<Connection> secondAttempt = connection.setKeyspaceAsync("newkeyspace");
466+
467+
assertThat(firstAttempt).isNotSameAs(secondAttempt);
468+
try {
469+
Uninterruptibles.getUninterruptibly(firstAttempt, 5, TimeUnit.SECONDS);
470+
fail("Should have failed keyspace attempt");
471+
} catch (ExecutionException e) {
472+
assertThat(e.getCause())
473+
.isInstanceOf(ConnectionException.class)
474+
.hasMessageContaining("Write attempt on defunct connection");
475+
}
476+
try {
477+
Uninterruptibles.getUninterruptibly(secondAttempt, 5, TimeUnit.SECONDS);
478+
fail("Should have failed keyspace attempt");
479+
} catch (ExecutionException e) {
480+
assertThat(e.getCause())
481+
.isInstanceOf(ConnectionException.class)
482+
.hasMessageContaining("Write attempt on defunct connection");
483+
}
484+
} finally {
485+
cluster.close();
486+
}
487+
}
488+
489+
/**
490+
* Ensures that keyspace validation errors are surfaced as query errors and do not defunct the
491+
* connection.
492+
*
493+
* @test_category connection:connection_pool
494+
*/
495+
@Test(groups = "short")
496+
public void should_not_defunct_connection_when_keyspace_setup_fails_with_validation_error()
497+
throws Exception {
498+
Cluster cluster = createClusterBuilder().build();
499+
try {
500+
HostConnectionPool pool = createPool(cluster, 1, 1);
501+
Connection connection = spy(pool.connections[0].get(0));
502+
pool.connections[0].set(0, connection);
503+
SettableConnectionFuture useFuture =
504+
new SettableConnectionFuture(new Requests.Query("USE \"missingkeyspace\""));
505+
506+
doReturn(useFuture).when(connection).write(any(Message.Request.class));
507+
ListenableFuture<Connection> keyspaceFuture = connection.setKeyspaceAsync("missingkeyspace");
508+
useFuture.setResponse(
509+
errorResponse(ExceptionCode.INVALID, "Keyspace missingkeyspace does not exist"));
510+
511+
try {
512+
Uninterruptibles.getUninterruptibly(keyspaceFuture, 5, TimeUnit.SECONDS);
513+
fail("Should have failed keyspace attempt");
514+
} catch (ExecutionException e) {
515+
assertThat(e.getCause()).isInstanceOf(InvalidQueryException.class);
516+
}
517+
assertThat(connection.isDefunct()).isFalse();
518+
assertThat(pool.opened()).isEqualTo(1);
519+
} finally {
520+
cluster.close();
521+
}
522+
}
523+
524+
/**
525+
* Ensures that permanent keyspace setup failures fail the request directly instead of being
526+
* retried across hosts as connection failures.
527+
*
528+
* @test_category connection:connection_pool
529+
*/
530+
@Test(groups = "short")
531+
public void should_fail_fast_when_keyspace_setup_fails_with_validation_error() throws Exception {
532+
Cluster cluster = createClusterBuilder().build();
533+
try {
534+
SessionManager session = (SessionManager) cluster.connect();
535+
Host host = TestUtils.findHost(cluster, 1);
536+
HostConnectionPool pool = session.pools.get(host);
537+
Connection connection = spy(pool.connections[0].get(0));
538+
pool.connections[0].set(0, connection);
539+
InvalidQueryException failure =
540+
new InvalidQueryException(
541+
pool.host.getEndPoint(), "Keyspace missingkeyspace does not exist");
542+
session.poolsState.setKeyspace("missingkeyspace");
543+
doReturn(Futures.<Connection>immediateFailedFuture(failure))
544+
.when(connection)
545+
.setKeyspaceAsync("missingkeyspace");
546+
activityClient.clearAllRecordedActivity();
547+
548+
try {
549+
session.execute("select * from tbl");
550+
fail("Should have failed the request with the keyspace validation error");
551+
} catch (InvalidQueryException e) {
552+
// expected
553+
}
554+
assertThat(connection.isDefunct()).isFalse();
555+
assertThat(pool.opened()).isEqualTo(1);
556+
assertThat(activityClient.retrieveQueries()).extractingResultOf("getQuery").isEmpty();
557+
} finally {
558+
cluster.close();
559+
}
560+
}
561+
396562
/**
397563
* Ensures that on borrowConnection if a set keyspace attempt is in progress on that connection
398564
* for a different keyspace than the pool state that the borrowConnection future returned is
@@ -501,6 +667,9 @@ public void should_adjust_connection_keyspace_on_dequeue_if_pool_state_is_differ
501667
.contains(
502668
"Aborting attempt to set keyspace to 'newkeyspace' since there is already an in flight attempt to set keyspace to 'slowks'.");
503669
}
670+
assertThat(connection.inFlight.get()).isEqualTo(0);
671+
assertThat(pool.totalInFlight.get()).isEqualTo(0);
672+
assertThat(pool.pendingBorrowCount.get()).isEqualTo(0);
504673
} finally {
505674
MockRequest.completeAll(requests);
506675
cluster.close();
@@ -1743,4 +1912,14 @@ public void onFailure(Throwable t) {
17431912
return future;
17441913
}
17451914
}
1915+
1916+
private static class SettableConnectionFuture extends Connection.Future {
1917+
SettableConnectionFuture(Message.Request request) {
1918+
super(request);
1919+
}
1920+
1921+
void setResponse(Message.Response response) {
1922+
super.set(response);
1923+
}
1924+
}
17461925
}

0 commit comments

Comments
 (0)