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

Retry on hosts in different datacenters #5902

Merged
merged 15 commits into from
Feb 10, 2022
Merged

Conversation

jeremyk-91
Copy link
Contributor

Goals (and why):

==COMMIT_MSG==
==COMMIT_MSG==

Implementation Description (bullets):

Testing (What was existing testing like? What have you done to improve it?):

Concerns (what feedback would you like?):

Where should we start reviewing?:

Priority (whenever / two weeks / yesterday):

@changelog-app
Copy link

changelog-app bot commented Feb 10, 2022

Generate changelog in changelog/@unreleased

Type

  • Feature
  • Improvement
  • Fix
  • Break
  • Deprecation
  • Manual task
  • Migration

Description

In a multi-DC configuration, we will try at least one host in each datacenter matching the predicate before trying other hosts. This is necessary for internal deployments.

Check the box to generate changelog(s)

  • Generate changelog entry

Copy link
Contributor

@Jolyon-S Jolyon-S left a comment

Choose a reason for hiding this comment

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

Looks good, pending tests

Comment on lines 172 to 182
config.servers().accept(new CassandraServersConfigs.ThriftHostsExtractingVisitor());
config.servers().accept(new ThriftHostsExtractingVisitor());
Copy link
Contributor

Choose a reason for hiding this comment

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

same here

.mapKeys(EndpointDetails::getHost)
.mapKeys(this::getAddressForHostThrowUnchecked)
.map(EndpointDetails::getDatacenter)
.collectToMap();
Copy link
Contributor

Choose a reason for hiding this comment

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

please trace log this so we can more easily reason about it if needed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

discussed offline: moved to once per iteration as opposed to once per interval, but added!

@Jolyon-S Jolyon-S changed the title dc jump Retry on hosts in different datacenters Feb 10, 2022
Comment on lines 169 to 189
public void selectsAnyHostIfAllDatacentersAlreadyTried() {
ImmutableSet<InetSocketAddress> allHosts = ImmutableSet.of(HOST_1, HOST_2);
CassandraService cassandra = clientPoolWithServers(allHosts);
cassandra.overrideHostToDatacenterMapping(ImmutableMap.of(HOST_1, DC_1, HOST_2, DC_2));
Set<InetSocketAddress> suggestedHosts = IntStream.range(0, 1_000)
.mapToObj(attempt -> cassandra.getRandomGoodHostForPredicate(address -> true, allHosts))
.flatMap(Optional::stream)
.map(CassandraClientPoolingContainer::getHost)
.collect(Collectors.toSet());
assertThat(suggestedHosts).containsExactlyInAnyOrderElementsOf(allHosts);
}

@Test
public void selectsAnyHostIfNoDatacentersAlreadyTried() {
ImmutableSet<InetSocketAddress> allHosts = ImmutableSet.of(HOST_1, HOST_2);
CassandraService cassandra = clientPoolWithServers(allHosts);
cassandra.overrideHostToDatacenterMapping(ImmutableMap.of(HOST_1, DC_1, HOST_2, DC_2));
Set<InetSocketAddress> suggestedHosts = IntStream.range(0, 1_000)
.mapToObj(attempt -> cassandra.getRandomGoodHostForPredicate(address -> true, ImmutableSet.of()))
.flatMap(Optional::stream)
.map(CassandraClientPoolingContainer::getHost)
Copy link
Contributor

Choose a reason for hiding this comment

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

it'd be great if we could extract a common method here


@Test
public void selectsHostMatchingPredicateEvenIfRelatedHostsAlreadyTried() {
ImmutableSet<InetSocketAddress> allHosts = ImmutableSet.of(HOST_1, HOST_2, HOST_3);
Copy link
Contributor

Choose a reason for hiding this comment

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

we don't need this variable when we only use it once


@Test
public void selectsHostsWithUnknownDatacenterMappingIfAllKnownDatacentersTried() {
ImmutableSet<InetSocketAddress> allHosts = ImmutableSet.of(HOST_1, HOST_2, HOST_3);
Copy link
Contributor

Choose a reason for hiding this comment

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

same here

Copy link
Contributor

@Jolyon-S Jolyon-S left a comment

Choose a reason for hiding this comment

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

Looks good

@jeremyk-91
Copy link
Contributor Author

👍

@bulldozer-bot bulldozer-bot bot merged commit 8f8de6b into develop Feb 10, 2022
@bulldozer-bot bulldozer-bot bot deleted the jkong/cheap-soln branch February 10, 2022 21:19
@svc-autorelease
Copy link
Collaborator

Released 0.540.0

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants