Skip to content

Commit

Permalink
fixes spurious race condition in client close (#5318)
Browse files Browse the repository at this point in the history
When the close the accumulo client the following could happen.

 1. THREAD1 sets closed atomic boolean to true in client as part of
    close() call
 2. THREAD2 a background thread in transport pool tries to read property from
    client and gets exception
 3. THREAD1 shuts down the transport pool object as part of closing
    client

This change avoids throwing an exception in step 2 above.  This bug was
causing MetaFateOpsCommandsIT to sometimes fail because it would have
a random excpetion in the json the command printing.
  • Loading branch information
keith-turner authored Feb 10, 2025
1 parent ec77944 commit 170ea68
Showing 1 changed file with 17 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
import java.util.concurrent.ThreadPoolExecutor;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Function;
import java.util.function.LongSupplier;
import java.util.function.Supplier;
import java.util.stream.Collectors;

Expand Down Expand Up @@ -73,6 +74,7 @@
import org.apache.accumulo.core.client.security.tokens.AuthenticationToken;
import org.apache.accumulo.core.conf.AccumuloConfiguration;
import org.apache.accumulo.core.conf.ClientProperty;
import org.apache.accumulo.core.conf.ConfigurationTypeHelper;
import org.apache.accumulo.core.conf.Property;
import org.apache.accumulo.core.data.InstanceId;
import org.apache.accumulo.core.data.KeyValue;
Expand Down Expand Up @@ -1038,7 +1040,21 @@ protected long getTransportPoolMaxAgeMillis() {
public synchronized ThriftTransportPool getTransportPool() {
ensureOpen();
if (thriftTransportPool == null) {
thriftTransportPool = ThriftTransportPool.startNew(this::getTransportPoolMaxAgeMillis);
LongSupplier maxAgeSupplier = () -> {
try {
return getTransportPoolMaxAgeMillis();
} catch (IllegalStateException e) {
if (closed.get()) {
// The transport pool has a background thread that may call this supplier in the middle
// of closing. This is here to avoid spurious exceptions from race conditions that
// happen when closing a client.
return ConfigurationTypeHelper
.getTimeInMillis(ClientProperty.RPC_TRANSPORT_IDLE_TIMEOUT.getDefaultValue());
}
throw e;
}
};
thriftTransportPool = ThriftTransportPool.startNew(maxAgeSupplier);
}
return thriftTransportPool;
}
Expand Down

0 comments on commit 170ea68

Please sign in to comment.