You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by "arjunashok (via GitHub)" <gi...@apache.org> on 2023/09/18 23:59:12 UTC
[GitHub] [cassandra-sidecar] arjunashok opened a new pull request, #69: CASSANDRASC-73 Updates token-ranges endpoint to return additional ins…
arjunashok opened a new pull request, #69:
URL: https://github.com/apache/cassandra-sidecar/pull/69
…tance metadata
### Changes
The sidecar `token-range-replicas` endpoint response is updated to include a `ReplicaMetadata` section which includes the following attrbutes. This replaces the `replicaState` attribute which included a mapping of the replica IP address/port to the `State` of the instance.
ReplicaMetadata attributes:
- State - State of the instance. eg. `Normal`, `Joining`, `Leaving`, `Replacing`
- Status - Status of the instance. eg. `Up`, `Down`
- Name - Hostname/FQDN of the instance resolved from the IP address in the below field
- Address - IP address of the instance
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRASC-73 Updates token-ranges endpoint to return additional ins… [cassandra-sidecar]
Posted by "yifan-c (via GitHub)" <gi...@apache.org>.
yifan-c closed pull request #69: CASSANDRASC-73 Updates token-ranges endpoint to return additional ins…
URL: https://github.com/apache/cassandra-sidecar/pull/69
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra-sidecar] frankgh commented on a diff in pull request #69: CASSANDRASC-73 Updates token-ranges endpoint to return additional ins…
Posted by "frankgh (via GitHub)" <gi...@apache.org>.
frankgh commented on code in PR #69:
URL: https://github.com/apache/cassandra-sidecar/pull/69#discussion_r1339251708
##########
adapters/base/src/main/java/org/apache/cassandra/sidecar/adapters/base/CassandraStorageOperations.java:
##########
@@ -49,20 +49,16 @@ public class CassandraStorageOperations implements StorageOperations
protected final TokenRangeReplicaProvider tokenRangeReplicaProvider;
/**
- * Creates a new instance with the provided {@link JmxClient}
+ * Creates a new instance with the provided {@link JmxClient} and {@link DnsResolver}
*
* @param jmxClient the JMX client used to communicate with the Cassandra instance
+ * @param dnsResolver the DNS resolver used to lookup replicas
*/
public CassandraStorageOperations(JmxClient jmxClient, DnsResolver dnsResolver)
- {
- this(jmxClient, new RingProvider(jmxClient, dnsResolver));
- }
-
- public CassandraStorageOperations(JmxClient jmxClient, RingProvider ringProvider)
{
this.jmxClient = jmxClient;
- this.ringProvider = ringProvider;
- this.tokenRangeReplicaProvider = new TokenRangeReplicaProvider(jmxClient);
+ this.ringProvider = new RingProvider(jmxClient, dnsResolver);
+ this.tokenRangeReplicaProvider = new TokenRangeReplicaProvider(jmxClient, dnsResolver);
Review Comment:
minor cosmetic issues, can you reformat the files touched in this PR?
```suggestion
this.tokenRangeReplicaProvider = new TokenRangeReplicaProvider(jmxClient, dnsResolver);
```
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra-sidecar] frankgh commented on a diff in pull request #69: CASSANDRASC-73 Updates token-ranges endpoint to return additional ins…
Posted by "frankgh (via GitHub)" <gi...@apache.org>.
frankgh commented on code in PR #69:
URL: https://github.com/apache/cassandra-sidecar/pull/69#discussion_r1331961884
##########
adapters/base/src/main/java/org/apache/cassandra/sidecar/adapters/base/TokenRangeReplicaProvider.java:
##########
@@ -115,22 +121,51 @@ private List<TokenRangeReplicas> transformRangeMappings(Map<List<String>, List<S
.collect(toList());
}
- private Map<String, String> replicaToStateMap(List<TokenRangeReplicas> replicaSet, StorageJmxOperations storage)
+ private List<ReplicaMetadata> getReplicaMetadata(List<TokenRangeReplicas> replicaSet,
+ StorageJmxOperations storage,
+ Map<String, String> hostToDatacenter)
{
List<String> joiningNodes = storage.getJoiningNodesWithPort();
List<String> leavingNodes = storage.getLeavingNodesWithPort();
List<String> movingNodes = storage.getMovingNodesWithPort();
+ List<String> liveNodes = storage.getLiveNodesWithPort();
+ List<String> deadNodes = storage.getUnreachableNodesWithPort();
+
+
String rawGossipInfo = getRawGossipInfo();
GossipInfoResponse gossipInfo = GossipInfoParser.parse(rawGossipInfo);
StateWithReplacement state = new StateWithReplacement(joiningNodes, leavingNodes, movingNodes, gossipInfo);
+ RingProvider.Status status = new RingProvider.Status(liveNodes, deadNodes);
return replicaSet.stream()
.map(TokenRangeReplicas::replicaSet)
.flatMap(Collection::stream)
.distinct()
- .collect(Collectors.toMap(Function.identity(), state::of));
+ .map(r -> new ReplicaMetadata(state.of(r),
Review Comment:
reading this is confusing, can we give `r` a more descriptive name?
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra-sidecar] arjunashok commented on a diff in pull request #69: CASSANDRASC-73 Updates token-ranges endpoint to return additional ins…
Posted by "arjunashok (via GitHub)" <gi...@apache.org>.
arjunashok commented on code in PR #69:
URL: https://github.com/apache/cassandra-sidecar/pull/69#discussion_r1334817382
##########
adapters/base/src/main/java/org/apache/cassandra/sidecar/adapters/base/TokenRangeReplicaProvider.java:
##########
@@ -115,22 +121,51 @@ private List<TokenRangeReplicas> transformRangeMappings(Map<List<String>, List<S
.collect(toList());
}
- private Map<String, String> replicaToStateMap(List<TokenRangeReplicas> replicaSet, StorageJmxOperations storage)
+ private List<ReplicaMetadata> getReplicaMetadata(List<TokenRangeReplicas> replicaSet,
+ StorageJmxOperations storage,
+ Map<String, String> hostToDatacenter)
{
List<String> joiningNodes = storage.getJoiningNodesWithPort();
List<String> leavingNodes = storage.getLeavingNodesWithPort();
List<String> movingNodes = storage.getMovingNodesWithPort();
+ List<String> liveNodes = storage.getLiveNodesWithPort();
+ List<String> deadNodes = storage.getUnreachableNodesWithPort();
+
+
String rawGossipInfo = getRawGossipInfo();
GossipInfoResponse gossipInfo = GossipInfoParser.parse(rawGossipInfo);
StateWithReplacement state = new StateWithReplacement(joiningNodes, leavingNodes, movingNodes, gossipInfo);
+ RingProvider.Status status = new RingProvider.Status(liveNodes, deadNodes);
return replicaSet.stream()
.map(TokenRangeReplicas::replicaSet)
.flatMap(Collection::stream)
.distinct()
- .collect(Collectors.toMap(Function.identity(), state::of));
+ .map(r -> new ReplicaMetadata(state.of(r),
Review Comment:
Addressed
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra-sidecar] yifan-c commented on pull request #69: CASSANDRASC-73 Updates token-ranges endpoint to return additional ins…
Posted by "yifan-c (via GitHub)" <gi...@apache.org>.
yifan-c commented on PR #69:
URL: https://github.com/apache/cassandra-sidecar/pull/69#issuecomment-1736015116
The patch looks good to me! Waiting for the new CI run to finish.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra-sidecar] arjunashok commented on a diff in pull request #69: CASSANDRASC-73 Updates token-ranges endpoint to return additional ins…
Posted by "arjunashok (via GitHub)" <gi...@apache.org>.
arjunashok commented on code in PR #69:
URL: https://github.com/apache/cassandra-sidecar/pull/69#discussion_r1330769227
##########
common/src/main/java/org/apache/cassandra/sidecar/common/data/TokenRangeReplicasResponse.java:
##########
@@ -154,4 +155,74 @@ public String toString()
'}';
}
}
+
+ /**
+ * Class representing metadata associated with replica instances
+ */
+ public static class ReplicaMetadata
+ {
+ private final String state;
+ private final String status;
+ private final String name;
+ private final String address;
+ public ReplicaMetadata(@JsonProperty("state") String state,
+ @JsonProperty("status") String status,
+ @JsonProperty("name") String name,
+ @JsonProperty("address") String address)
+ {
+ this.state = state;
+ this.status = status;
+ this.name = name;
+ this.address = address;
+ }
+
+ /**
+ * @return the node state. eg. NORMAL, JOINING, LEAVING, etc.
+ */
+ @JsonProperty("state")
+ public String state()
+ {
+ return state;
+ }
+
+ /**
+ * @return the node status. eg. UP, DOWN
+ */
+ @JsonProperty("status")
+ public String status()
+ {
+ return status;
+ }
+
+ /**
+ * @return FQDN of the node
+ */
+ @JsonProperty("name")
+ public String name()
Review Comment:
Addressed
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra-sidecar] frankgh commented on a diff in pull request #69: CASSANDRASC-73 Updates token-ranges endpoint to return additional ins…
Posted by "frankgh (via GitHub)" <gi...@apache.org>.
frankgh commented on code in PR #69:
URL: https://github.com/apache/cassandra-sidecar/pull/69#discussion_r1331967226
##########
adapters/base/src/main/java/org/apache/cassandra/sidecar/adapters/base/TokenRangeReplicaProvider.java:
##########
@@ -115,22 +121,51 @@ private List<TokenRangeReplicas> transformRangeMappings(Map<List<String>, List<S
.collect(toList());
}
- private Map<String, String> replicaToStateMap(List<TokenRangeReplicas> replicaSet, StorageJmxOperations storage)
+ private List<ReplicaMetadata> getReplicaMetadata(List<TokenRangeReplicas> replicaSet,
+ StorageJmxOperations storage,
+ Map<String, String> hostToDatacenter)
{
List<String> joiningNodes = storage.getJoiningNodesWithPort();
List<String> leavingNodes = storage.getLeavingNodesWithPort();
List<String> movingNodes = storage.getMovingNodesWithPort();
+ List<String> liveNodes = storage.getLiveNodesWithPort();
+ List<String> deadNodes = storage.getUnreachableNodesWithPort();
+
+
String rawGossipInfo = getRawGossipInfo();
GossipInfoResponse gossipInfo = GossipInfoParser.parse(rawGossipInfo);
StateWithReplacement state = new StateWithReplacement(joiningNodes, leavingNodes, movingNodes, gossipInfo);
+ RingProvider.Status status = new RingProvider.Status(liveNodes, deadNodes);
return replicaSet.stream()
.map(TokenRangeReplicas::replicaSet)
.flatMap(Collection::stream)
.distinct()
- .collect(Collectors.toMap(Function.identity(), state::of));
+ .map(r -> new ReplicaMetadata(state.of(r),
+ status.of(r),
+ resolveReplica(r),
+ r,
+ hostToDatacenter.get(r)))
+ .collect(Collectors.toList());
+
+ }
+
+ private String resolveReplica(String r)
+ {
+ String hostName;
+ try
+ {
+ hostName = dnsResolver.reverseResolve(r.split(":")[0]);
Review Comment:
We have some class that will get the host and port for use, just use the class. Splitting and getting an index is prone to errors. I believe the library is HostAndPort
```suggestion
hostName = dnsResolver.reverseResolve(r.split(":")[0]);
```
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra-sidecar] frankgh commented on a diff in pull request #69: CASSANDRASC-73 Updates token-ranges endpoint to return additional ins…
Posted by "frankgh (via GitHub)" <gi...@apache.org>.
frankgh commented on code in PR #69:
URL: https://github.com/apache/cassandra-sidecar/pull/69#discussion_r1331968368
##########
adapters/base/src/main/java/org/apache/cassandra/sidecar/adapters/base/TokenRangeReplicaProvider.java:
##########
@@ -115,22 +121,51 @@ private List<TokenRangeReplicas> transformRangeMappings(Map<List<String>, List<S
.collect(toList());
}
- private Map<String, String> replicaToStateMap(List<TokenRangeReplicas> replicaSet, StorageJmxOperations storage)
+ private List<ReplicaMetadata> getReplicaMetadata(List<TokenRangeReplicas> replicaSet,
+ StorageJmxOperations storage,
+ Map<String, String> hostToDatacenter)
{
List<String> joiningNodes = storage.getJoiningNodesWithPort();
List<String> leavingNodes = storage.getLeavingNodesWithPort();
List<String> movingNodes = storage.getMovingNodesWithPort();
+ List<String> liveNodes = storage.getLiveNodesWithPort();
+ List<String> deadNodes = storage.getUnreachableNodesWithPort();
+
+
String rawGossipInfo = getRawGossipInfo();
GossipInfoResponse gossipInfo = GossipInfoParser.parse(rawGossipInfo);
StateWithReplacement state = new StateWithReplacement(joiningNodes, leavingNodes, movingNodes, gossipInfo);
+ RingProvider.Status status = new RingProvider.Status(liveNodes, deadNodes);
return replicaSet.stream()
.map(TokenRangeReplicas::replicaSet)
.flatMap(Collection::stream)
.distinct()
- .collect(Collectors.toMap(Function.identity(), state::of));
+ .map(r -> new ReplicaMetadata(state.of(r),
+ status.of(r),
+ resolveReplica(r),
+ r,
+ hostToDatacenter.get(r)))
+ .collect(Collectors.toList());
+
+ }
+
+ private String resolveReplica(String r)
+ {
+ String hostName;
+ try
+ {
+ hostName = dnsResolver.reverseResolve(r.split(":")[0]);
+ }
+ catch (Exception e)
+ {
+ // Swallow exception to not fail-fast on DNS resolution failure
+ LOGGER.warn("Failed to resolve hostname for instance {}: {}", r, e);
+ hostName = "";
Review Comment:
why empty? maybe use the unresolved IP?
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra-sidecar] arjunashok commented on a diff in pull request #69: CASSANDRASC-73 Updates token-ranges endpoint to return additional ins…
Posted by "arjunashok (via GitHub)" <gi...@apache.org>.
arjunashok commented on code in PR #69:
URL: https://github.com/apache/cassandra-sidecar/pull/69#discussion_r1332054697
##########
adapters/base/src/main/java/org/apache/cassandra/sidecar/adapters/base/TokenRangeReplicaProvider.java:
##########
@@ -115,22 +121,51 @@ private List<TokenRangeReplicas> transformRangeMappings(Map<List<String>, List<S
.collect(toList());
}
- private Map<String, String> replicaToStateMap(List<TokenRangeReplicas> replicaSet, StorageJmxOperations storage)
+ private List<ReplicaMetadata> getReplicaMetadata(List<TokenRangeReplicas> replicaSet,
+ StorageJmxOperations storage,
+ Map<String, String> hostToDatacenter)
{
List<String> joiningNodes = storage.getJoiningNodesWithPort();
List<String> leavingNodes = storage.getLeavingNodesWithPort();
List<String> movingNodes = storage.getMovingNodesWithPort();
+ List<String> liveNodes = storage.getLiveNodesWithPort();
+ List<String> deadNodes = storage.getUnreachableNodesWithPort();
+
+
String rawGossipInfo = getRawGossipInfo();
GossipInfoResponse gossipInfo = GossipInfoParser.parse(rawGossipInfo);
StateWithReplacement state = new StateWithReplacement(joiningNodes, leavingNodes, movingNodes, gossipInfo);
+ RingProvider.Status status = new RingProvider.Status(liveNodes, deadNodes);
return replicaSet.stream()
.map(TokenRangeReplicas::replicaSet)
.flatMap(Collection::stream)
.distinct()
- .collect(Collectors.toMap(Function.identity(), state::of));
+ .map(r -> new ReplicaMetadata(state.of(r),
+ status.of(r),
+ resolveReplica(r),
+ r,
+ hostToDatacenter.get(r)))
+ .collect(Collectors.toList());
+
+ }
+
+ private String resolveReplica(String r)
+ {
+ String hostName;
+ try
+ {
+ hostName = dnsResolver.reverseResolve(r.split(":")[0]);
+ }
+ catch (Exception e)
+ {
+ // Swallow exception to not fail-fast on DNS resolution failure
+ LOGGER.warn("Failed to resolve hostname for instance {}: {}", r, e);
+ hostName = "";
Review Comment:
I explicitly left this empty to denote the resolution failure, so the client can choose to handle it differently in this event. For instance, it can retry instead of making a sidecar request with the IP, which would fail to verify the client identity with TLS.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra-sidecar] frankgh commented on a diff in pull request #69: CASSANDRASC-73 Updates token-ranges endpoint to return additional ins…
Posted by "frankgh (via GitHub)" <gi...@apache.org>.
frankgh commented on code in PR #69:
URL: https://github.com/apache/cassandra-sidecar/pull/69#discussion_r1331967753
##########
adapters/base/src/main/java/org/apache/cassandra/sidecar/adapters/base/TokenRangeReplicaProvider.java:
##########
@@ -115,22 +121,51 @@ private List<TokenRangeReplicas> transformRangeMappings(Map<List<String>, List<S
.collect(toList());
}
- private Map<String, String> replicaToStateMap(List<TokenRangeReplicas> replicaSet, StorageJmxOperations storage)
+ private List<ReplicaMetadata> getReplicaMetadata(List<TokenRangeReplicas> replicaSet,
+ StorageJmxOperations storage,
+ Map<String, String> hostToDatacenter)
{
List<String> joiningNodes = storage.getJoiningNodesWithPort();
List<String> leavingNodes = storage.getLeavingNodesWithPort();
List<String> movingNodes = storage.getMovingNodesWithPort();
+ List<String> liveNodes = storage.getLiveNodesWithPort();
+ List<String> deadNodes = storage.getUnreachableNodesWithPort();
+
+
String rawGossipInfo = getRawGossipInfo();
GossipInfoResponse gossipInfo = GossipInfoParser.parse(rawGossipInfo);
StateWithReplacement state = new StateWithReplacement(joiningNodes, leavingNodes, movingNodes, gossipInfo);
+ RingProvider.Status status = new RingProvider.Status(liveNodes, deadNodes);
return replicaSet.stream()
.map(TokenRangeReplicas::replicaSet)
.flatMap(Collection::stream)
.distinct()
- .collect(Collectors.toMap(Function.identity(), state::of));
+ .map(r -> new ReplicaMetadata(state.of(r),
+ status.of(r),
+ resolveReplica(r),
+ r,
+ hostToDatacenter.get(r)))
+ .collect(Collectors.toList());
+
+ }
+
+ private String resolveReplica(String r)
+ {
+ String hostName;
+ try
+ {
+ hostName = dnsResolver.reverseResolve(r.split(":")[0]);
+ }
+ catch (Exception e)
+ {
+ // Swallow exception to not fail-fast on DNS resolution failure
+ LOGGER.warn("Failed to resolve hostname for instance {}: {}", r, e);
Review Comment:
I think we want to print the exception with the stacktrace here
```suggestion
LOGGER.warn("Failed to resolve hostname for instance={}", r, e);
```
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra-sidecar] arjunashok commented on a diff in pull request #69: CASSANDRASC-73 Updates token-ranges endpoint to return additional ins…
Posted by "arjunashok (via GitHub)" <gi...@apache.org>.
arjunashok commented on code in PR #69:
URL: https://github.com/apache/cassandra-sidecar/pull/69#discussion_r1334823626
##########
adapters/base/src/main/java/org/apache/cassandra/sidecar/adapters/base/TokenRangeReplicaProvider.java:
##########
@@ -115,22 +121,51 @@ private List<TokenRangeReplicas> transformRangeMappings(Map<List<String>, List<S
.collect(toList());
}
- private Map<String, String> replicaToStateMap(List<TokenRangeReplicas> replicaSet, StorageJmxOperations storage)
+ private List<ReplicaMetadata> getReplicaMetadata(List<TokenRangeReplicas> replicaSet,
+ StorageJmxOperations storage,
+ Map<String, String> hostToDatacenter)
{
List<String> joiningNodes = storage.getJoiningNodesWithPort();
List<String> leavingNodes = storage.getLeavingNodesWithPort();
List<String> movingNodes = storage.getMovingNodesWithPort();
+ List<String> liveNodes = storage.getLiveNodesWithPort();
+ List<String> deadNodes = storage.getUnreachableNodesWithPort();
+
+
String rawGossipInfo = getRawGossipInfo();
GossipInfoResponse gossipInfo = GossipInfoParser.parse(rawGossipInfo);
StateWithReplacement state = new StateWithReplacement(joiningNodes, leavingNodes, movingNodes, gossipInfo);
+ RingProvider.Status status = new RingProvider.Status(liveNodes, deadNodes);
return replicaSet.stream()
.map(TokenRangeReplicas::replicaSet)
.flatMap(Collection::stream)
.distinct()
- .collect(Collectors.toMap(Function.identity(), state::of));
+ .map(r -> new ReplicaMetadata(state.of(r),
+ status.of(r),
+ resolveReplica(r),
+ r,
+ hostToDatacenter.get(r)))
+ .collect(Collectors.toList());
+
+ }
+
+ private String resolveReplica(String r)
+ {
+ String hostName;
+ try
+ {
+ hostName = dnsResolver.reverseResolve(r.split(":")[0]);
+ }
+ catch (Exception e)
+ {
+ // Swallow exception to not fail-fast on DNS resolution failure
+ LOGGER.warn("Failed to resolve hostname for instance {}: {}", r, e);
+ hostName = "";
Review Comment:
From offline discussion, the resolver will not fail on resolution failures. Decided to allow exception to be thrown on failures from the resolver. Addressed
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRASC-73 Updates token-ranges endpoint to return additional ins… [cassandra-sidecar]
Posted by "yifan-c (via GitHub)" <gi...@apache.org>.
yifan-c commented on PR #69:
URL: https://github.com/apache/cassandra-sidecar/pull/69#issuecomment-1785677750
Closing the PR as it is merged already.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra-sidecar] frankgh commented on a diff in pull request #69: CASSANDRASC-73 Updates token-ranges endpoint to return additional ins…
Posted by "frankgh (via GitHub)" <gi...@apache.org>.
frankgh commented on code in PR #69:
URL: https://github.com/apache/cassandra-sidecar/pull/69#discussion_r1333640986
##########
adapters/base/src/main/java/org/apache/cassandra/sidecar/adapters/base/TokenRangeReplicaProvider.java:
##########
@@ -115,22 +121,51 @@ private List<TokenRangeReplicas> transformRangeMappings(Map<List<String>, List<S
.collect(toList());
}
- private Map<String, String> replicaToStateMap(List<TokenRangeReplicas> replicaSet, StorageJmxOperations storage)
+ private List<ReplicaMetadata> getReplicaMetadata(List<TokenRangeReplicas> replicaSet,
+ StorageJmxOperations storage,
+ Map<String, String> hostToDatacenter)
{
List<String> joiningNodes = storage.getJoiningNodesWithPort();
List<String> leavingNodes = storage.getLeavingNodesWithPort();
List<String> movingNodes = storage.getMovingNodesWithPort();
+ List<String> liveNodes = storage.getLiveNodesWithPort();
+ List<String> deadNodes = storage.getUnreachableNodesWithPort();
+
+
String rawGossipInfo = getRawGossipInfo();
GossipInfoResponse gossipInfo = GossipInfoParser.parse(rawGossipInfo);
StateWithReplacement state = new StateWithReplacement(joiningNodes, leavingNodes, movingNodes, gossipInfo);
+ RingProvider.Status status = new RingProvider.Status(liveNodes, deadNodes);
return replicaSet.stream()
.map(TokenRangeReplicas::replicaSet)
.flatMap(Collection::stream)
.distinct()
- .collect(Collectors.toMap(Function.identity(), state::of));
+ .map(r -> new ReplicaMetadata(state.of(r),
+ status.of(r),
+ resolveReplica(r),
+ r,
+ hostToDatacenter.get(r)))
+ .collect(Collectors.toList());
+
+ }
+
+ private String resolveReplica(String r)
+ {
+ String hostName;
+ try
+ {
+ hostName = dnsResolver.reverseResolve(r.split(":")[0]);
+ }
+ catch (Exception e)
+ {
+ // Swallow exception to not fail-fast on DNS resolution failure
+ LOGGER.warn("Failed to resolve hostname for instance {}: {}", r, e);
+ hostName = "";
Review Comment:
that sounds like a bad idea. Why don't we mimic what `RingProvider` is doing instead. If there are host resolution errors, we should fail the request, client will see a 500, clients should leverage retries in that case.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra-sidecar] frankgh commented on a diff in pull request #69: CASSANDRASC-73 Updates token-ranges endpoint to return additional ins…
Posted by "frankgh (via GitHub)" <gi...@apache.org>.
frankgh commented on code in PR #69:
URL: https://github.com/apache/cassandra-sidecar/pull/69#discussion_r1329441803
##########
common/src/main/java/org/apache/cassandra/sidecar/common/data/TokenRangeReplicasResponse.java:
##########
@@ -154,4 +155,74 @@ public String toString()
'}';
}
}
+
+ /**
+ * Class representing metadata associated with replica instances
+ */
+ public static class ReplicaMetadata
+ {
+ private final String state;
+ private final String status;
+ private final String name;
+ private final String address;
+ public ReplicaMetadata(@JsonProperty("state") String state,
+ @JsonProperty("status") String status,
+ @JsonProperty("name") String name,
+ @JsonProperty("address") String address)
+ {
+ this.state = state;
+ this.status = status;
+ this.name = name;
+ this.address = address;
+ }
+
+ /**
+ * @return the node state. eg. NORMAL, JOINING, LEAVING, etc.
+ */
+ @JsonProperty("state")
+ public String state()
+ {
+ return state;
+ }
+
+ /**
+ * @return the node status. eg. UP, DOWN
+ */
+ @JsonProperty("status")
+ public String status()
+ {
+ return status;
+ }
+
+ /**
+ * @return FQDN of the node
+ */
+ @JsonProperty("name")
+ public String name()
Review Comment:
this should just be called `fqdn`, see https://github.com/apache/cassandra-sidecar/blob/trunk/common/src/main/java/org/apache/cassandra/sidecar/common/data/RingEntry.java#L171
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra-sidecar] frankgh commented on a diff in pull request #69: CASSANDRASC-73 Updates token-ranges endpoint to return additional ins…
Posted by "frankgh (via GitHub)" <gi...@apache.org>.
frankgh commented on code in PR #69:
URL: https://github.com/apache/cassandra-sidecar/pull/69#discussion_r1331966040
##########
adapters/base/src/main/java/org/apache/cassandra/sidecar/adapters/base/TokenRangeReplicaProvider.java:
##########
@@ -115,22 +121,51 @@ private List<TokenRangeReplicas> transformRangeMappings(Map<List<String>, List<S
.collect(toList());
}
- private Map<String, String> replicaToStateMap(List<TokenRangeReplicas> replicaSet, StorageJmxOperations storage)
+ private List<ReplicaMetadata> getReplicaMetadata(List<TokenRangeReplicas> replicaSet,
+ StorageJmxOperations storage,
+ Map<String, String> hostToDatacenter)
{
List<String> joiningNodes = storage.getJoiningNodesWithPort();
List<String> leavingNodes = storage.getLeavingNodesWithPort();
List<String> movingNodes = storage.getMovingNodesWithPort();
+ List<String> liveNodes = storage.getLiveNodesWithPort();
+ List<String> deadNodes = storage.getUnreachableNodesWithPort();
+
+
String rawGossipInfo = getRawGossipInfo();
GossipInfoResponse gossipInfo = GossipInfoParser.parse(rawGossipInfo);
StateWithReplacement state = new StateWithReplacement(joiningNodes, leavingNodes, movingNodes, gossipInfo);
+ RingProvider.Status status = new RingProvider.Status(liveNodes, deadNodes);
return replicaSet.stream()
.map(TokenRangeReplicas::replicaSet)
.flatMap(Collection::stream)
.distinct()
- .collect(Collectors.toMap(Function.identity(), state::of));
+ .map(r -> new ReplicaMetadata(state.of(r),
+ status.of(r),
+ resolveReplica(r),
+ r,
+ hostToDatacenter.get(r)))
+ .collect(Collectors.toList());
+
+ }
+
+ private String resolveReplica(String r)
Review Comment:
use replica here?
```suggestion
private String resolveReplica(String replica)
```
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra-sidecar] arjunashok commented on a diff in pull request #69: CASSANDRASC-73 Updates token-ranges endpoint to return additional ins…
Posted by "arjunashok (via GitHub)" <gi...@apache.org>.
arjunashok commented on code in PR #69:
URL: https://github.com/apache/cassandra-sidecar/pull/69#discussion_r1334821910
##########
adapters/base/src/main/java/org/apache/cassandra/sidecar/adapters/base/TokenRangeReplicaProvider.java:
##########
@@ -115,22 +121,51 @@ private List<TokenRangeReplicas> transformRangeMappings(Map<List<String>, List<S
.collect(toList());
}
- private Map<String, String> replicaToStateMap(List<TokenRangeReplicas> replicaSet, StorageJmxOperations storage)
+ private List<ReplicaMetadata> getReplicaMetadata(List<TokenRangeReplicas> replicaSet,
+ StorageJmxOperations storage,
+ Map<String, String> hostToDatacenter)
{
List<String> joiningNodes = storage.getJoiningNodesWithPort();
List<String> leavingNodes = storage.getLeavingNodesWithPort();
List<String> movingNodes = storage.getMovingNodesWithPort();
+ List<String> liveNodes = storage.getLiveNodesWithPort();
+ List<String> deadNodes = storage.getUnreachableNodesWithPort();
+
+
String rawGossipInfo = getRawGossipInfo();
GossipInfoResponse gossipInfo = GossipInfoParser.parse(rawGossipInfo);
StateWithReplacement state = new StateWithReplacement(joiningNodes, leavingNodes, movingNodes, gossipInfo);
+ RingProvider.Status status = new RingProvider.Status(liveNodes, deadNodes);
return replicaSet.stream()
.map(TokenRangeReplicas::replicaSet)
.flatMap(Collection::stream)
.distinct()
- .collect(Collectors.toMap(Function.identity(), state::of));
+ .map(r -> new ReplicaMetadata(state.of(r),
+ status.of(r),
+ resolveReplica(r),
+ r,
+ hostToDatacenter.get(r)))
+ .collect(Collectors.toList());
+
+ }
+
+ private String resolveReplica(String r)
+ {
+ String hostName;
+ try
+ {
+ hostName = dnsResolver.reverseResolve(r.split(":")[0]);
Review Comment:
Addressed
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra-sidecar] frankgh commented on a diff in pull request #69: CASSANDRASC-73 Updates token-ranges endpoint to return additional ins…
Posted by "frankgh (via GitHub)" <gi...@apache.org>.
frankgh commented on code in PR #69:
URL: https://github.com/apache/cassandra-sidecar/pull/69#discussion_r1338970416
##########
adapters/base/src/main/java/org/apache/cassandra/sidecar/adapters/base/CassandraStorageOperations.java:
##########
@@ -55,14 +55,18 @@ public class CassandraStorageOperations implements StorageOperations
*/
public CassandraStorageOperations(JmxClient jmxClient, DnsResolver dnsResolver)
{
- this(jmxClient, new RingProvider(jmxClient, dnsResolver));
+ this(jmxClient,
+ new RingProvider(jmxClient, dnsResolver),
+ new TokenRangeReplicaProvider(jmxClient, dnsResolver));
}
- public CassandraStorageOperations(JmxClient jmxClient, RingProvider ringProvider)
+ public CassandraStorageOperations(JmxClient jmxClient,
Review Comment:
I wonder why we have this constructor. It only seems to be used from the call in line 58. Should we remove it?
##########
adapters/base/src/main/java/org/apache/cassandra/sidecar/adapters/base/TokenRangeReplicaProvider.java:
##########
@@ -115,22 +122,45 @@ private List<TokenRangeReplicas> transformRangeMappings(Map<List<String>, List<S
.collect(toList());
}
- private Map<String, String> replicaToStateMap(List<TokenRangeReplicas> replicaSet, StorageJmxOperations storage)
+ private List<ReplicaMetadata> getReplicaMetadata(List<TokenRangeReplicas> replicaSet,
+ StorageJmxOperations storage,
+ Map<String, String> hostToDatacenter)
{
List<String> joiningNodes = storage.getJoiningNodesWithPort();
List<String> leavingNodes = storage.getLeavingNodesWithPort();
List<String> movingNodes = storage.getMovingNodesWithPort();
+ List<String> liveNodes = storage.getLiveNodesWithPort();
+ List<String> deadNodes = storage.getUnreachableNodesWithPort();
+
+
String rawGossipInfo = getRawGossipInfo();
GossipInfoResponse gossipInfo = GossipInfoParser.parse(rawGossipInfo);
StateWithReplacement state = new StateWithReplacement(joiningNodes, leavingNodes, movingNodes, gossipInfo);
+ RingProvider.Status status = new RingProvider.Status(liveNodes, deadNodes);
return replicaSet.stream()
.map(TokenRangeReplicas::replicaSet)
.flatMap(Collection::stream)
.distinct()
- .collect(Collectors.toMap(Function.identity(), state::of));
+ .map(replica -> {
+ try
+ {
+ HostAndPort hap = HostAndPort.fromString(replica);
+ return new ReplicaMetadata(state.of(replica),
+ status.of(replica),
+ dnsResolver.reverseResolve(hap.getHost()),
+ replica,
Review Comment:
since we already have it split, let's add an address field and a port field.
```suggestion
hap.getHost(),
hap.getPort(),
```
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org