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

Excavator: Upgrades Baseline to the latest version #6214

Merged
Sam-Kramer merged 8 commits into from
Sep 28, 2022
Merged

Excavator: Upgrades Baseline to the latest version #6214

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
8f15a40
Excavator: Upgrades Baseline to the latest version
svc-excavator-bot Sep 21, 2022
effb9ee
Fix tests to not use direct invocations on mocks
Sep 23, 2022
93aba54
rename method for more clarity
Sep 23, 2022
5d455f0
Lambdas cannot be mocked, therefore create empty runnable class
Sep 26, 2022
5bbf929
spotless
Sep 26, 2022
ff78a8c
spotless
Sep 26, 2022
29bc3af
remove unneeded test
Sep 28, 2022
c0f32a7
spotless
Sep 28, 2022
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
4 changes: 3 additions & 1 deletion .baseline/checkstyle/checkstyle.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,9 @@
<property name="tagOrder" value="@param, @return, @throws, @deprecated"/>
<property name="target" value="CLASS_DEF, INTERFACE_DEF, ENUM_DEF, METHOD_DEF, CTOR_DEF, VARIABLE_DEF"/>
</module>
<module name="CyclomaticComplexity"/> <!-- Java Coding Guidelines: Reduce Cyclomatic Complexity -->
<module name="CyclomaticComplexity"> <!-- Java Coding Guidelines: Reduce Cyclomatic Complexity -->
<property name="switchBlockAsSingleDecisionPoint" value="true"/>
</module>
<module name="DesignForExtension"> <!-- Java Coding Guidelines: Design for extension -->
<property name="ignoredAnnotations" value="ParameterizedTest, Test, Before, BeforeEach, After, AfterEach, BeforeClass, BeforeAll, AfterClass, AfterAll"/>
</module>
Expand Down
10 changes: 9 additions & 1 deletion atlasdb-api/src/test/java/com/palantir/atlasdb/metrics/MetricPublicationArbiterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ public void metricsWithOneFilterAreFiltered() {
}

@Test
@SuppressWarnings("DistinctVarargsChecker")
Copy link
Contributor

@Sam-Kramer Sam-Kramer Sep 26, 2022

Choose a reason for hiding this comment

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

DistinctVarargsChecker throws even for methods, despite that they may return a unique object

public void metricsWithMultipleFiltersAreAcceptedOnlyIfAllFiltersPermit() {
MetricPublicationArbiter arbiter = createArbiter(ImmutableMap.of(
METRIC_NAME_1, ImmutableSet.of(trueFilter(), falseFilter(), trueFilter()),
Expand Down Expand Up @@ -100,7 +101,14 @@ public void deduplicatesFilters() {
}

private static MetricPublicationFilter trueFilter() {
return () -> true;
// Depending on the JDK, identical lambdas may share the same hashcode
// As a result, we cannot use this method to create a set of filters
return new MetricPublicationFilter() {
@Override
public boolean shouldPublish() {
return true;
}
};
}

private static MetricPublicationFilter falseFilter() {
Expand Down
14 changes: 5 additions & 9 deletions atlasdb-cassandra/src/test/java/com/palantir/atlasdb/cassandra/backup/BackupTestUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Range;
import com.palantir.atlasdb.cassandra.CassandraKeyValueServiceRuntimeConfig;
import com.palantir.atlasdb.cassandra.CassandraServersConfigs;
import com.palantir.atlasdb.cassandra.ImmutableCqlCapableConfig;
import com.palantir.atlasdb.keyvalue.cassandra.LightweightOppToken;
Expand Down Expand Up @@ -56,6 +55,11 @@ public final class BackupTestUtils {
static final Range<LightweightOppToken> RANGE_2_TO_3 = Range.openClosed(TOKEN_2, TOKEN_3);
static final Range<LightweightOppToken> RANGE_GREATER_THAN_3 = Range.greaterThan(TOKEN_3);

static final CassandraServersConfigs.CqlCapableConfig CASSANDRA_SERVERS_CONFIG = ImmutableCqlCapableConfig.builder()
.addAllCqlHosts(HOSTS)
.addAllThriftHosts(HOSTS)
.build();

private BackupTestUtils() {
// utility
}
Expand All @@ -79,14 +83,6 @@ static void mockTokenRanges(CqlSession cqlSession, CqlMetadata cqlMetadata) {
when(cqlSession.getMetadata()).thenReturn(cqlMetadata);
}

static void mockConfig(CassandraKeyValueServiceRuntimeConfig runtimeConfig) {
CassandraServersConfigs.CqlCapableConfig cqlCapableConfig = ImmutableCqlCapableConfig.builder()
.addAllCqlHosts(HOSTS)
.addAllThriftHosts(HOSTS)
.build();
when(runtimeConfig.servers()).thenReturn(cqlCapableConfig);
}

static List<TableMetadata> mockTableMetadatas(KeyspaceMetadata keyspaceMetadata, String... tableNames) {
return Arrays.stream(tableNames)
.map(tableName -> mockTableMetadata(keyspaceMetadata, tableName))
Expand Down
8 changes: 2 additions & 6 deletions ...cassandra/src/test/java/com/palantir/atlasdb/cassandra/backup/RepairRangeFetcherTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Range;
import com.google.common.collect.RangeSet;
import com.palantir.atlasdb.cassandra.CassandraKeyValueServiceRuntimeConfig;
import com.palantir.atlasdb.cassandra.backup.transaction.Transactions1TableInteraction;
import com.palantir.atlasdb.cassandra.backup.transaction.Transactions2TableInteraction;
import com.palantir.atlasdb.cassandra.backup.transaction.Transactions3TableInteraction;
Expand Down Expand Up @@ -60,9 +59,6 @@ public class RepairRangeFetcherTest {
@Mock
private CqlMetadata cqlMetadata;

@Mock
private CassandraKeyValueServiceRuntimeConfig runtimeConfig;

private RepairRangeFetcher repairRangeFetcher;

@Before
Expand All @@ -72,14 +68,14 @@ public void setUp() {
when(keyspaceMetadata.getTables()).thenReturn(tableMetadatas);

BackupTestUtils.mockTokenRanges(cqlSession, cqlMetadata);
BackupTestUtils.mockConfig(runtimeConfig);

when(cqlSession.retrieveRowKeysAtConsistencyAll(anyList()))
.thenReturn(ImmutableSet.of(BackupTestUtils.TOKEN_1, OTHER_TOKEN));
when(cqlMetadata.getReplicas(eq(BackupTestUtils.NAMESPACE), any()))
.thenReturn(ImmutableSet.copyOf(BackupTestUtils.HOSTS));

repairRangeFetcher = new RepairRangeFetcher(cqlSession, BackupTestUtils.NAMESPACE, runtimeConfig.servers());
repairRangeFetcher =
new RepairRangeFetcher(cqlSession, BackupTestUtils.NAMESPACE, BackupTestUtils.CASSANDRA_SERVERS_CONFIG);
}

@Test
Expand Down
8 changes: 2 additions & 6 deletions ...-cassandra/src/test/java/com/palantir/atlasdb/cassandra/backup/TokenRangeFetcherTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
import com.google.common.collect.Iterables;
import com.google.common.collect.Range;
import com.google.common.collect.RangeSet;
import com.palantir.atlasdb.cassandra.CassandraKeyValueServiceRuntimeConfig;
import com.palantir.atlasdb.keyvalue.cassandra.LightweightOppToken;
import java.net.InetSocketAddress;
import java.util.List;
Expand All @@ -51,9 +50,6 @@ public class TokenRangeFetcherTest {
@Mock
private CqlMetadata cqlMetadata;

@Mock
private CassandraKeyValueServiceRuntimeConfig runtimeConfig;

private TokenRangeFetcher tokenRangeFetcher;

@Before
Expand All @@ -62,13 +58,13 @@ public void setUp() {
List<TableMetadata> tableMetadatas = BackupTestUtils.mockTableMetadatas(keyspaceMetadata, TABLE_NAME);
when(keyspaceMetadata.getTable(TABLE_NAME)).thenReturn(Iterables.getOnlyElement(tableMetadatas));

BackupTestUtils.mockConfig(runtimeConfig);
BackupTestUtils.mockTokenRanges(cqlSession, cqlMetadata);

when(cqlSession.retrieveRowKeysAtConsistencyAll(anyList()))
.thenReturn(ImmutableSet.of(BackupTestUtils.TOKEN_1, BackupTestUtils.TOKEN_2, BackupTestUtils.TOKEN_3));

tokenRangeFetcher = new TokenRangeFetcher(cqlSession, BackupTestUtils.NAMESPACE, runtimeConfig.servers());
tokenRangeFetcher =
new TokenRangeFetcher(cqlSession, BackupTestUtils.NAMESPACE, BackupTestUtils.CASSANDRA_SERVERS_CONFIG);
}

@Test
Expand Down
3 changes: 0 additions & 3 deletions .../com/palantir/atlasdb/cassandra/backup/transaction/Transactions3TableInteractionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import com.datastax.driver.core.Row;
import com.datastax.driver.core.Statement;
import com.datastax.driver.core.TableMetadata;
import com.datastax.driver.core.policies.RetryPolicy;
import com.google.common.collect.Range;
import com.google.common.primitives.Longs;
import com.palantir.atlasdb.atomic.AtomicValue;
Expand All @@ -53,8 +52,6 @@ public class Transactions3TableInteractionTest {
new TwoPhaseEncodingStrategy(BaseProgressEncodingStrategy.INSTANCE);

private static final String KEYSPACE = "keyspace";

private final RetryPolicy mockPolicy = mock(RetryPolicy.class);
private final TransactionsTableInteraction interaction = new Transactions3TableInteraction(RANGE);
private final TableMetadata tableMetadata = mock(TableMetadata.class, RETURNS_DEEP_STUBS);

Expand Down
71 changes: 27 additions & 44 deletions ...sandra/src/test/java/com/palantir/atlasdb/keyvalue/cassandra/CassandraClientPoolTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
import com.palantir.atlasdb.cassandra.CassandraCredentialsConfig;
import com.palantir.atlasdb.cassandra.CassandraKeyValueServiceConfig;
import com.palantir.atlasdb.cassandra.CassandraKeyValueServiceRuntimeConfig;
import com.palantir.atlasdb.cassandra.CassandraServersConfigs.ThriftHostsExtractingVisitor;
import com.palantir.atlasdb.cassandra.CassandraServersConfigs;
import com.palantir.atlasdb.cassandra.ImmutableDefaultConfig;
import com.palantir.atlasdb.keyvalue.cassandra.pool.CassandraServer;
import com.palantir.atlasdb.keyvalue.cassandra.pool.CassandraService;
Expand Down Expand Up @@ -108,13 +108,8 @@ public void setup() {
when(runtimeConfig.unresponsiveHostBackoffTimeSeconds()).thenReturn(UNRESPONSIVE_HOST_BACKOFF_SECONDS);
when(config.credentials()).thenReturn(mock(CassandraCredentialsConfig.class));
when(config.getKeyspaceOrThrow()).thenReturn("ks");
blacklist = new Blacklist(config, Refreshable.only(runtimeConfig.unresponsiveHostBackoffTimeSeconds()));
blacklist = new Blacklist(config, Refreshable.only(UNRESPONSIVE_HOST_BACKOFF_SECONDS));

doAnswer(invocation -> runtimeConfig.servers().accept(ThriftHostsExtractingVisitor.INSTANCE).stream()
.map(CassandraServer::of)
.collect(ImmutableSet.toImmutableSet()))
.when(cassandra)
.getCurrentServerListFromConfig();
doAnswer(invocation -> poolServers.add(getInvocationAddress(invocation)))
.when(cassandra)
.addPool(any());
Expand Down Expand Up @@ -277,8 +272,7 @@ public void resilientToRollingRestarts() {

@Test
public void attemptsShouldBeCountedPerHost() {
when(runtimeConfig.servers())
.thenReturn(ImmutableDefaultConfig.builder().addThriftHosts().build());
setThriftServers(ImmutableSet.of());
CassandraClientPoolImpl cassandraClientPool = CassandraClientPoolImpl.createImplForTest(
MetricsManagers.of(metricRegistry, taggedMetricRegistry),
config,
Expand All @@ -299,10 +293,7 @@ public void attemptsShouldBeCountedPerHost() {

@Test
public void hostIsAutomaticallyRemovedOnStartup() {
when(runtimeConfig.servers())
.thenReturn(ImmutableDefaultConfig.builder()
.addThriftHosts(CASS_SERVER_1.proxy(), CASS_SERVER_2.proxy(), CASS_SERVER_3.proxy())
.build());
setThriftServers(ImmutableSet.of(CASS_SERVER_1.proxy(), CASS_SERVER_2.proxy(), CASS_SERVER_3.proxy()));
when(config.autoRefreshNodes()).thenReturn(true);

setCassandraServersTo(CASS_SERVER_1);
Expand All @@ -313,10 +304,7 @@ public void hostIsAutomaticallyRemovedOnStartup() {

@Test
public void hostIsAutomaticallyRemovedOnRefresh() {
when(runtimeConfig.servers())
.thenReturn(ImmutableDefaultConfig.builder()
.addThriftHosts(CASS_SERVER_1.proxy(), CASS_SERVER_2.proxy(), CASS_SERVER_3.proxy())
.build());
setThriftServers(ImmutableSet.of(CASS_SERVER_1.proxy(), CASS_SERVER_2.proxy(), CASS_SERVER_3.proxy()));
when(config.autoRefreshNodes()).thenReturn(true);

setCassandraServersTo(CASS_SERVER_1, CASS_SERVER_2, CASS_SERVER_3);
Expand All @@ -325,16 +313,13 @@ public void hostIsAutomaticallyRemovedOnRefresh() {
assertThat(poolServers).containsExactlyInAnyOrder(CASS_SERVER_1, CASS_SERVER_2, CASS_SERVER_3);

setCassandraServersTo(CASS_SERVER_1, CASS_SERVER_2);
deterministicExecutor.tick(config.poolRefreshIntervalSeconds(), TimeUnit.SECONDS);
deterministicExecutor.tick(POOL_REFRESH_INTERVAL_SECONDS, TimeUnit.SECONDS);
assertThat(poolServers).containsExactlyInAnyOrder(CASS_SERVER_1, CASS_SERVER_2);
}

@Test
public void hostIsAutomaticallyAddedOnStartup() {
when(runtimeConfig.servers())
.thenReturn(ImmutableDefaultConfig.builder()
.addThriftHosts(CASS_SERVER_1.proxy())
.build());
setThriftServers(ImmutableSet.of(CASS_SERVER_1.proxy()));
when(config.autoRefreshNodes()).thenReturn(true);

setCassandraServersTo(CASS_SERVER_1, CASS_SERVER_2);
Expand All @@ -345,10 +330,7 @@ public void hostIsAutomaticallyAddedOnStartup() {

@Test
public void hostIsAutomaticallyAddedOnRefresh() {
when(runtimeConfig.servers())
.thenReturn(ImmutableDefaultConfig.builder()
.addThriftHosts(CASS_SERVER_1.proxy(), CASS_SERVER_2.proxy())
.build());
setThriftServers(ImmutableSet.of(CASS_SERVER_1.proxy(), CASS_SERVER_2.proxy()));
when(config.autoRefreshNodes()).thenReturn(true);

setCassandraServersTo(CASS_SERVER_1, CASS_SERVER_2);
Expand All @@ -357,50 +339,44 @@ public void hostIsAutomaticallyAddedOnRefresh() {
assertThat(poolServers).containsExactlyInAnyOrder(CASS_SERVER_1, CASS_SERVER_2);

setCassandraServersTo(CASS_SERVER_1, CASS_SERVER_2, CASS_SERVER_3);
deterministicExecutor.tick(config.poolRefreshIntervalSeconds(), TimeUnit.SECONDS);
deterministicExecutor.tick(POOL_REFRESH_INTERVAL_SECONDS, TimeUnit.SECONDS);
assertThat(poolServers).containsExactlyInAnyOrder(CASS_SERVER_1, CASS_SERVER_2, CASS_SERVER_3);
}

@Test
public void hostsAreNotRemovedOrAddedWhenRefreshIsDisabled() {
when(runtimeConfig.servers())
.thenReturn(ImmutableDefaultConfig.builder()
.addThriftHosts(CASS_SERVER_1.proxy(), CASS_SERVER_2.proxy())
.build());
setThriftServers(ImmutableSet.of(CASS_SERVER_1.proxy(), CASS_SERVER_2.proxy()));
when(config.autoRefreshNodes()).thenReturn(false);

setCassandraServersTo(CASS_SERVER_1);
createClientPool();
assertThat(poolServers).containsExactlyInAnyOrder(CASS_SERVER_1, CASS_SERVER_2);

setCassandraServersTo(CASS_SERVER_1, CASS_SERVER_2, CASS_SERVER_3);
deterministicExecutor.tick(config.poolRefreshIntervalSeconds(), TimeUnit.SECONDS);
deterministicExecutor.tick(POOL_REFRESH_INTERVAL_SECONDS, TimeUnit.SECONDS);
assertThat(poolServers).containsExactlyInAnyOrder(CASS_SERVER_1, CASS_SERVER_2);
}

@Test
public void hostsAreResetToConfigOnRefreshWhenRefreshIsDisabled() {
when(runtimeConfig.servers())
.thenReturn(ImmutableDefaultConfig.builder()
.addThriftHosts(CASS_SERVER_1.proxy(), CASS_SERVER_2.proxy())
.build());
setThriftServers(ImmutableSet.of(CASS_SERVER_1.proxy(), CASS_SERVER_2.proxy()));
when(config.autoRefreshNodes()).thenReturn(false);

setCassandraServersTo(CASS_SERVER_1);
createClientPool();
assertThat(poolServers).containsExactlyInAnyOrder(CASS_SERVER_1, CASS_SERVER_2);

cassandra.addPool(CASS_SERVER_3);
poolServers.add(CASS_SERVER_3);
assertThat(poolServers).containsExactlyInAnyOrder(CASS_SERVER_1, CASS_SERVER_2, CASS_SERVER_3);

deterministicExecutor.tick(config.poolRefreshIntervalSeconds(), TimeUnit.SECONDS);
deterministicExecutor.tick(POOL_REFRESH_INTERVAL_SECONDS, TimeUnit.SECONDS);
assertThat(poolServers).containsExactlyInAnyOrder(CASS_SERVER_1, CASS_SERVER_2);

setCassandraServersTo(CASS_SERVER_2, CASS_SERVER_3);
cassandra.removePool(CASS_SERVER_1);
poolServers.remove(CASS_SERVER_1);
assertThat(poolServers).containsExactlyInAnyOrder(CASS_SERVER_2);

deterministicExecutor.tick(config.poolRefreshIntervalSeconds(), TimeUnit.SECONDS);
deterministicExecutor.tick(POOL_REFRESH_INTERVAL_SECONDS, TimeUnit.SECONDS);
assertThat(poolServers).containsExactlyInAnyOrder(CASS_SERVER_1, CASS_SERVER_2);
}

Expand Down Expand Up @@ -501,10 +477,7 @@ private CassandraClientPoolImpl throwingClientPoolWithServersInCurrentPool(
@SuppressWarnings("OptionalUsedAsFieldOrParameterType") // Unpacking it seems less readable
private CassandraClientPoolImpl clientPoolWith(
Set<InetSocketAddress> servers, Set<CassandraServer> serversInPool, Optional<Exception> failureMode) {
when(runtimeConfig.servers())
.thenReturn(ImmutableDefaultConfig.builder()
.addAllThriftHosts(servers)
.build());
setThriftServers(servers);
when(config.timeoutOnConnectionClose()).thenReturn(Duration.ofSeconds(10));
when(config.timeoutOnConnectionBorrow()).thenReturn(HumanReadableDuration.minutes(10));
when(config.consecutiveAbsencesBeforePoolRemoval()).thenReturn(1);
Expand Down Expand Up @@ -596,4 +569,14 @@ private Object getAggregateMetricValueForMetricName(String metricName) {
String fullyQualifiedMetricName = MetricRegistry.name(CassandraClientPool.class, metricName);
return metricRegistry.getGauges().get(fullyQualifiedMetricName).getValue();
}

private void setThriftServers(Set<InetSocketAddress> servers) {
CassandraServersConfigs.CassandraServersConfig config =
ImmutableDefaultConfig.builder().addAllThriftHosts(servers).build();
when(runtimeConfig.servers()).thenReturn(config);
when(cassandra.getCurrentServerListFromConfig())
.thenReturn(config.accept(CassandraServersConfigs.ThriftHostsExtractingVisitor.INSTANCE).stream()
.map(CassandraServer::of)
.collect(ImmutableSet.toImmutableSet()));
}
}
9 changes: 6 additions & 3 deletions ...a/src/test/java/com/palantir/atlasdb/keyvalue/cassandra/ProfilingCassandraClientTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import org.apache.cassandra.thrift.CqlResultType;
import org.apache.cassandra.thrift.CqlRow;
import org.apache.cassandra.thrift.SlicePredicate;
import org.apache.cassandra.thrift.SliceRange;
import org.apache.thrift.TException;
import org.junit.After;
import org.junit.Test;
Expand Down Expand Up @@ -89,14 +90,16 @@ public void handlesNullMultigetSliceResponseFromDelegate() throws TException {
List<ColumnOrSuperColumn> columns =
ImmutableList.of(new ColumnOrSuperColumn().setColumn(new Column(byteBuffer)));
ImmutableMap<ByteBuffer, List<ColumnOrSuperColumn>> resultMap = ImmutableMap.of(byteBuffer, columns);
SlicePredicate slicePredicate = new SlicePredicate();
slicePredicate.setSlice_range(new SliceRange());

when(delegate.multiget_slice(any(), any(), any(), any(), any())).thenReturn(resultMap);

assertThat(delegate.multiget_slice(
assertThat(profilingClient.multiget_slice(
"getRows",
TableReference.createFromFullyQualifiedName("a.b"),
ImmutableList.of(byteBuffer),
new SlicePredicate(),
slicePredicate,
ConsistencyLevel.QUORUM))
.containsExactlyInAnyOrderEntriesOf(resultMap);

Expand All @@ -105,7 +108,7 @@ public void handlesNullMultigetSliceResponseFromDelegate() throws TException {
"getRows",
TableReference.createFromFullyQualifiedName("a.b"),
ImmutableList.of(byteBuffer),
new SlicePredicate(),
slicePredicate,
ConsistencyLevel.QUORUM);
}

Expand Down
2 changes: 2 additions & 0 deletions ...-commons/src/test/java/com/palantir/common/proxy/ReplaceIfExceptionMatchingProxyTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
import org.mockito.Mock;
import org.mockito.junit.MockitoJUnitRunner;

// Mock is used as a convenient supplier, alternatives are rather verbose
@SuppressWarnings("DirectInvocationOnMock")
@RunWith(MockitoJUnitRunner.class)
public class ReplaceIfExceptionMatchingProxyTest {
@Mock
Expand Down
Loading