Skip to content
This repository has been archived by the owner on Nov 14, 2024. It is now read-only.

Allow using more threads to refresh cassandra pools #5900

Merged
merged 4 commits into from
Feb 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -381,4 +381,13 @@ default void check() {
localHostWeighting() >= 0.0 && localHostWeighting() <= 1.0,
"'localHostWeighting' must be between 0 and 1 inclusive");
}

/**
* Number of threads to be used across ALL transaction managers to refresh the client pool. It is suggested to use
* one thread per approximately 10 transaction managers that are expected to be used.
*/
@Value.Default
default int numPoolRefreshingThreads() {
return 1;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@
import com.palantir.atlasdb.keyvalue.cassandra.pool.CassandraService;
import com.palantir.atlasdb.util.MetricsManager;
import com.palantir.common.base.FunctionCheckedException;
import com.palantir.common.concurrent.InitializeableScheduledExecutorServiceSupplier;
import com.palantir.common.concurrent.NamedThreadFactory;
import com.palantir.common.concurrent.PTExecutors;
import com.palantir.logsafe.SafeArg;
import com.palantir.logsafe.UnsafeArg;
import com.palantir.logsafe.exceptions.SafeIllegalStateException;
Expand Down Expand Up @@ -73,8 +73,9 @@
**/
@SuppressWarnings("checkstyle:FinalClass") // non-final for mocking
public class CassandraClientPoolImpl implements CassandraClientPool {
private static final ScheduledExecutorService sharedRefreshDaemon =
PTExecutors.newScheduledThreadPool(1, new NamedThreadFactory("CassandraClientPoolRefresh", true));
private static final InitializeableScheduledExecutorServiceSupplier SHARED_EXECUTOR_SUPPLIER =
new InitializeableScheduledExecutorServiceSupplier(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: static final should be all caps?

new NamedThreadFactory("CassandraClientPoolRefresh", true));

private class InitializingWrapper extends AsyncInitializer implements AutoDelegate_CassandraClientPool {
@Override
Expand Down Expand Up @@ -141,14 +142,14 @@ static CassandraClientPoolImpl createImplForTest(
MetricsManager metricsManager,
CassandraKeyValueServiceConfig config,
StartupChecks startupChecks,
ScheduledExecutorService refreshDaemon,
InitializeableScheduledExecutorServiceSupplier initializeableExecutorSupplier,
Blacklist blacklist,
CassandraService cassandra) {
CassandraRequestExceptionHandler exceptionHandler = testExceptionHandler(blacklist);
CassandraClientPoolImpl cassandraClientPool = new CassandraClientPoolImpl(
config,
startupChecks,
refreshDaemon,
initializeableExecutorSupplier,
exceptionHandler,
blacklist,
cassandra,
Expand Down Expand Up @@ -189,7 +190,7 @@ private CassandraClientPoolImpl(
this(
config,
startupChecks,
sharedRefreshDaemon,
SHARED_EXECUTOR_SUPPLIER,
exceptionHandler,
blacklist,
new CassandraService(metricsManager, config, blacklist, metrics),
Expand All @@ -199,14 +200,15 @@ private CassandraClientPoolImpl(
private CassandraClientPoolImpl(
CassandraKeyValueServiceConfig config,
StartupChecks startupChecks,
ScheduledExecutorService refreshDaemon,
InitializeableScheduledExecutorServiceSupplier initializeableExecutorSupplier,
CassandraRequestExceptionHandler exceptionHandler,
Blacklist blacklist,
CassandraService cassandra,
CassandraClientPoolMetrics metrics) {
this.config = config;
this.startupChecks = startupChecks;
this.refreshDaemon = refreshDaemon;
initializeableExecutorSupplier.initialize(config.numPoolRefreshingThreads());
this.refreshDaemon = initializeableExecutorSupplier.get();
this.blacklist = blacklist;
this.exceptionHandler = exceptionHandler;
this.cassandra = cassandra;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import com.palantir.atlasdb.keyvalue.cassandra.pool.CassandraService;
import com.palantir.atlasdb.util.MetricsManagers;
import com.palantir.common.base.FunctionCheckedException;
import com.palantir.common.concurrent.InitializeableScheduledExecutorServiceSupplier;
import com.palantir.common.exception.AtlasDbDependencyException;
import com.palantir.conjure.java.api.config.service.HumanReadableDuration;
import com.palantir.tritium.metrics.registry.DefaultTaggedMetricRegistry;
Expand Down Expand Up @@ -378,7 +379,7 @@ private CassandraClientPoolImpl createClientPool() {
MetricsManagers.createForTests(),
config,
CassandraClientPoolImpl.StartupChecks.DO_NOT_RUN,
deterministicExecutor,
InitializeableScheduledExecutorServiceSupplier.createForTests(deterministicExecutor),
blacklist,
cassandra);
}
Expand Down
6 changes: 6 additions & 0 deletions changelog/@unreleased/pr-5900.v2.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
type: improvement
improvement:
description: Cassandra users can now specify the number of threads to be used for
refreshing pools
links:
- https://github.com/palantir/atlasdb/pull/5900
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
/*
* (c) Copyright 2022 Palantir Technologies Inc. All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.palantir.common.concurrent;

import com.palantir.logsafe.Preconditions;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ThreadFactory;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Supplier;

/**
* A supplier of ScheduledExecutorService that can be initialized with the desired number of threads at runtime. Must
* be initialized before it is first used, and once initialized, further calls to the {@link #initialize(int)} method
* are ignored.
*/
public final class InitializeableScheduledExecutorServiceSupplier implements Supplier<ScheduledExecutorService> {
private final ThreadFactory threadFactory;
private final AtomicReference<ScheduledExecutorService> delegate = new AtomicReference<>();

public InitializeableScheduledExecutorServiceSupplier(ThreadFactory threadFactory) {
this.threadFactory = threadFactory;
}

private InitializeableScheduledExecutorServiceSupplier(ScheduledExecutorService executor, ThreadFactory factory) {
threadFactory = factory;
delegate.set(executor);
}

public static InitializeableScheduledExecutorServiceSupplier createForTests(ScheduledExecutorService executor) {
return new InitializeableScheduledExecutorServiceSupplier(executor, new NamedThreadFactory("fake", true));
}

public void initialize(int numThreads) {
delegate.compareAndSet(null, PTExecutors.newScheduledThreadPool(numThreads, threadFactory));
}

@Override
public ScheduledExecutorService get() {
Preconditions.checkState(delegate.get() != null, "Executor must be initialized");
return delegate.get();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
/*
* (c) Copyright 2022 Palantir Technologies Inc. All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.palantir.common.concurrent;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;

import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import org.junit.Test;

public class InitializeableScheduledExecutorServiceSupplierTest {
private final InitializeableScheduledExecutorServiceSupplier supplier =
new InitializeableScheduledExecutorServiceSupplier(new NamedThreadFactory("test", true));

@Test
public void uninitializedSupplierThrows() {
assertThatThrownBy(supplier::get).isInstanceOf(IllegalStateException.class);
}

@Test
public void canInitializeSupplier() throws ExecutionException, InterruptedException {
supplier.initialize(1);

assertThat(supplier.get().schedule(() -> 1L, 0, TimeUnit.SECONDS).get()).isEqualTo(1L);
supplier.get().shutdownNow();
}

@Test
public void originalNumberOfThreadsIsRespected() throws InterruptedException {
InitializeableScheduledExecutorServiceSupplier supplier =
new InitializeableScheduledExecutorServiceSupplier(new NamedThreadFactory("test", true));
supplier.initialize(1);
supplier.initialize(2);

CountDownLatch latch = new CountDownLatch(2);
Runnable task = () -> {
latch.countDown();
try {
latch.await();
} catch (InterruptedException e) {
// ok cool
}
};

supplier.get().schedule(task, 0, TimeUnit.SECONDS);
supplier.get().schedule(task, 0, TimeUnit.SECONDS);
assertThat(latch.await(1, TimeUnit.SECONDS)).isFalse();
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm the test makes sense.

}