yifan-c commented on code in PR #74:
URL: https://github.com/apache/cassandra-sidecar/pull/74#discussion_r1414968092
##########
src/main/java/org/apache/cassandra/sidecar/cluster/SidecarLoadBalancingPolicy.java:
##########
@@ -193,25 +194,33 @@ private void addLocalHostsToSelected(List<Host>
sourceHosts)
private synchronized void recalculateSelectedHosts()
{
- // Copy the list to allow us to remove hosts as we build the list
- List<Host> sourceHosts = new ArrayList<>(this.allHosts);
- addLocalHostsToSelected(sourceHosts);
- Collections.shuffle(sourceHosts, this.secureRandom);
- Iterator<Host> hostIterator = sourceHosts.iterator();
- while (selectedHosts.size() < totalRequestedConnections &&
hostIterator.hasNext())
+ Map<Boolean, List<Host>> partitionedHosts = allHosts.stream()
+
.collect(Collectors.partitioningBy(
+ host ->
localHostAddresses.contains(
+
host.getEndPoint().resolve())));
+ List<Host> localHosts = partitionedHosts.get(true);
+ if (localHosts == null)
{
- Host host = hostIterator.next();
- if (!selectedHosts.contains(host) && host.isUp())
- {
- selectedHosts.add(host);
- hostIterator.remove();
- }
+ LOGGER.warn("Did not find any local hosts in the complete host
list!");
}
- if (selectedHosts.size() < totalRequestedConnections)
+ else
+ {
+ selectedHosts.addAll(localHosts);
+ }
+ int requiredNewHosts = this.totalRequestedConnections -
selectedHosts.size();
+ if (requiredNewHosts > 0)
{
- LOGGER.warn("Requested number of instances (local + additional)
({}) is greater " +
- "than the number of UP hosts in the cluster ({})",
- totalRequestedConnections, selectedHosts.size());
+ List<Host> nonLocalHosts = partitionedHosts.get(false);
+ if (nonLocalHosts == null)
+ {
+ LOGGER.warn("Did not find any local hosts in the complete host
list!");
Review Comment:
```suggestion
LOGGER.warn("Did not find any non-local hosts in the
complete host list!");
```
##########
src/main/java/org/apache/cassandra/sidecar/cluster/SidecarLoadBalancingPolicy.java:
##########
@@ -193,25 +194,33 @@ private void addLocalHostsToSelected(List<Host>
sourceHosts)
private synchronized void recalculateSelectedHosts()
{
- // Copy the list to allow us to remove hosts as we build the list
- List<Host> sourceHosts = new ArrayList<>(this.allHosts);
- addLocalHostsToSelected(sourceHosts);
- Collections.shuffle(sourceHosts, this.secureRandom);
- Iterator<Host> hostIterator = sourceHosts.iterator();
- while (selectedHosts.size() < totalRequestedConnections &&
hostIterator.hasNext())
+ Map<Boolean, List<Host>> partitionedHosts = allHosts.stream()
+
.collect(Collectors.partitioningBy(
+ host ->
localHostAddresses.contains(
+
host.getEndPoint().resolve())));
Review Comment:
I found declare a local variable reads better than the inlined lambda that
is formatted to a small block due to indentation.
```suggestion
Predicate<Host> hostPartitioner = host ->
localHostAddresses.contains(host.getEndPoint().resolve());
Map<Boolean, List<Host>> partitionedHosts = allHosts.stream()
.collect(Collectors.partitioningBy(hostPartitioner));
```
##########
src/main/java/org/apache/cassandra/sidecar/cluster/SidecarLoadBalancingPolicy.java:
##########
@@ -193,25 +194,33 @@ private void addLocalHostsToSelected(List<Host>
sourceHosts)
Review Comment:
Method `addLocalHostsToSelected(...)` (above this line) is not longer used
and can be deleted.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
