[jira] [Commented] (CASSANDRA-14572) Expose all table metrics in virtual table
[
https://issues.apache.org/jira/browse/CASSANDRA-14572?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17800138#comment-17800138
]
Dinesh Joshi commented on CASSANDRA-14572:
--
hi [~mmuzaf] thanks for your patch. This is not a formal review but I wanted to
leave some brief observations -
1. I am concerned about the GC pressure this would generate if someone wrote a
script to constantly poll this virtual table. Maybe add some documentation that
discourages polling of this virtual table?
2. I am personally ok with adding the annotation processing. However, I would
suggest the use of some templating engine to generate the code. For example,
you could clean up {{SystemViewAnnotationProcessor::generate}} method with a
template engine. Right now it appears that it will be difficult to maintain it
the way it is written. I recognize this would mean we may need to pull in a
templating engine as a compile time dependency.
> Expose all table metrics in virtual table
> -
>
> Key: CASSANDRA-14572
> URL: https://issues.apache.org/jira/browse/CASSANDRA-14572
> Project: Cassandra
> Issue Type: New Feature
> Components: Legacy/Observability, Observability/Metrics
>Reporter: Chris Lohfink
>Assignee: Maxim Muzafarov
>Priority: Low
> Labels: virtual-tables
> Fix For: 5.x
>
> Attachments: keyspayces_group responses times.png, keyspayces_group
> summary.png, select keyspaces_group by string prefix.png, select
> keyspaces_group compare with wo.png, select keyspaces_group without
> value.png, systemv_views.metrics_dropped_message.png, thread_pools
> benchmark.png
>
> Time Spent: 20m
> Remaining Estimate: 0h
>
> While we want a number of virtual tables to display data in a way thats great
> and intuitive like in nodetool. There is also much for being able to expose
> the metrics we have for tooling via CQL instead of JMX. This is more for the
> tooling and adhoc advanced users who know exactly what they are looking for.
> *Schema:*
> Initial idea is to expose data via {{((keyspace, table), metric)}} with a
> column for each metric value. Could also use a Map or UDT instead of the
> column based that can be a bit more specific to each metric type. To that end
> there can be a {{metric_type}} column and then a UDT for each metric type
> filled in, or a single value with more of a Map style. I am
> purposing the column type though as with {{ALLOW FILTERING}} it does allow
> more extensive query capabilities.
> *Implementations:*
> * Use reflection to grab all the metrics from TableMetrics (see:
> CASSANDRA-7622 impl). This is easiest and least abrasive towards new metric
> implementors... but its reflection and a kinda a bad idea.
> * Add a hook in TableMetrics to register with this virtual table when
> registering
> * Pull from the CassandraMetrics registery (either reporter or iterate
> through metrics query on read of virtual table)
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] JAVA-3142: Ability to optionally specify remote dcs for deterministic failovers when remote dcs are used in query plan [cassandra-java-driver]
adutra commented on code in PR #1896:
URL:
https://github.com/apache/cassandra-java-driver/pull/1896#discussion_r1435650327
##
core/src/main/java/com/datastax/oss/driver/api/core/config/TypedDriverOption.java:
##
@@ -886,6 +886,16 @@ public String toString() {
DefaultDriverOption.LOAD_BALANCING_DC_FAILOVER_ALLOW_FOR_LOCAL_CONSISTENCY_LEVELS,
GenericType.BOOLEAN);
+ /**
+ * Ordered preference list of remote dc's optionally supplied for automatic
failover and included
+ * in query plan. This feature is enabled only when max-nodes-per-remote-dc
is greater than 0
Review Comment:
```suggestion
* in query plan. This feature is enabled only when
max-nodes-per-remote-dc is greater than 0.
```
##
core/src/main/java/com/datastax/oss/driver/internal/core/loadbalancing/BasicLoadBalancingPolicy.java:
##
@@ -131,6 +135,15 @@ public BasicLoadBalancingPolicy(@NonNull DriverContext
context, @NonNull String
this.context
.getConsistencyLevelRegistry()
.nameToLevel(profile.getString(DefaultDriverOption.REQUEST_CONSISTENCY));
+
+final List remoteDcs =
Review Comment:
For improved efficiency, I'd suggest that you store the preferred DCs in a
`String[]`:
```java
Set remoteDcs =
new LinkedHashSet<>(
Objects.requireNonNull(
profile.getStringList(
DefaultDriverOption.LOAD_BALANCING_DC_FAILOVER_PREFERRED_REMOTE_DCS,
new ArrayList<>(;
preferredRemoteDcs = remoteDcs.toArray(new String[0]);
```
##
core/src/main/java/com/datastax/oss/driver/internal/core/loadbalancing/BasicLoadBalancingPolicy.java:
##
@@ -117,6 +120,7 @@ public class BasicLoadBalancingPolicy implements
LoadBalancingPolicy {
private volatile NodeDistanceEvaluator nodeDistanceEvaluator;
private volatile String localDc;
private volatile NodeSet liveNodes;
+ private volatile Set preferredRemoteLocalDcs = new LinkedHashSet<>();
Review Comment:
Rename to `preferredRemoteDcs`. Also, the field should be final.
##
core/src/main/java/com/datastax/oss/driver/internal/core/loadbalancing/BasicLoadBalancingPolicy.java:
##
@@ -325,24 +338,63 @@ protected Queue maybeAddDcFailover(@Nullable
Request request, @NonNull Que
@Override
protected Object[] computeNodes() {
-Object[] remoteNodes =
-liveNodes.dcs().stream()
-.filter(Predicates.not(Predicates.equalTo(localDc)))
-.flatMap(dc ->
liveNodes.dc(dc).stream().limit(maxNodesPerRemoteDc))
-.toArray();
-
-int remoteNodesLength = remoteNodes.length;
-if (remoteNodesLength == 0) {
- return EMPTY_NODES;
+if (!preferredRemoteLocalDcs.isEmpty()) {
Review Comment:
Isn't this unnecessarily complex? I think the same effect could be achieved
with a sorted stream:
```java
Object[] remoteNodes =
liveNodes.dcs().stream()
.filter(Predicates.not(Predicates.equalTo(localDc)))
.sorted(
Comparator.comparingInt(
dc -> {
int i = Arrays.binarySearch(preferredRemoteDcs, dc);
return i < 0 ? Integer.MAX_VALUE : i;
}))
.flatMap(dc -> liveNodes.dc(dc).stream().limit(maxNodesPerRemoteDc))
.toArray();
```
And indeed, in presence of preferred dcs, it probably doesn't make sense to
shuffle the remote nodes:
```java
if (preferredRemoteDcs.length == 0) {
shuffleHead(remoteNodes, remoteNodesLength);
}
```
##
core/src/test/java/com/datastax/oss/driver/internal/core/loadbalancing/BasicLoadBalancingPolicyRemoteDcTest.java:
##
@@ -0,0 +1,189 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.datastax.oss.driver.internal.core.loadbalancing;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyInt;
+import static
[jira] [Commented] (CASSANDRA-19210) Bring Harry into OSS Cassandra tree
[ https://issues.apache.org/jira/browse/CASSANDRA-19210?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17800068#comment-17800068 ] Alex Petrov commented on CASSANDRA-19210: - Fresh test results uploaded. Apart from upgrade python dtests (which should not be affected by this change), everything else looks good. Committed to trunk with [439d1b122af334bf68c159b82ef4e4879c210bd5|https://github.com/apache/cassandra/commit/439d1b122af334bf68c159b82ef4e4879c210bd5]. Thank you [~maedhroz] and [~marcuse] for reviews! > Bring Harry into OSS Cassandra tree > --- > > Key: CASSANDRA-19210 > URL: https://issues.apache.org/jira/browse/CASSANDRA-19210 > Project: Cassandra > Issue Type: Improvement > Components: Test/fuzz >Reporter: Alex Petrov >Assignee: Alex Petrov >Priority: High > Fix For: 5.1-alpha1 > > Attachments: ci_summary-1.html, ci_summary.html, > result_details.tar-1.gz, result_details.tar.gz > > > This patch introduces Harry into tree. > _No_ modifications were made to Harry code, so technically our tree has > already relied on it. The intention to introduce Harry to tree to lower the > entry barrier and allow quicker developer turnarounds. > CI results attached; all tests are green. Python tests are skipped because > this patch does not make any changes to production code, it's all testonly. -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Updated] (CASSANDRA-19210) Bring Harry into OSS Cassandra tree
[ https://issues.apache.org/jira/browse/CASSANDRA-19210?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Alex Petrov updated CASSANDRA-19210: Attachment: ci_summary-1.html result_details.tar-1.gz > Bring Harry into OSS Cassandra tree > --- > > Key: CASSANDRA-19210 > URL: https://issues.apache.org/jira/browse/CASSANDRA-19210 > Project: Cassandra > Issue Type: Improvement > Components: Test/fuzz >Reporter: Alex Petrov >Assignee: Alex Petrov >Priority: High > Fix For: 5.1-alpha1 > > Attachments: ci_summary-1.html, ci_summary.html, > result_details.tar-1.gz, result_details.tar.gz > > > This patch introduces Harry into tree. > _No_ modifications were made to Harry code, so technically our tree has > already relied on it. The intention to introduce Harry to tree to lower the > entry barrier and allow quicker developer turnarounds. > CI results attached; all tests are green. Python tests are skipped because > this patch does not make any changes to production code, it's all testonly. -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Updated] (CASSANDRA-19210) Bring Harry into OSS Cassandra tree
[ https://issues.apache.org/jira/browse/CASSANDRA-19210?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Alex Petrov updated CASSANDRA-19210: Fix Version/s: 5.1-alpha1 Source Control Link: https://github.com/apache/cassandra/commit/439d1b122af334bf68c159b82ef4e4879c210bd5 Resolution: Fixed Status: Resolved (was: Ready to Commit) > Bring Harry into OSS Cassandra tree > --- > > Key: CASSANDRA-19210 > URL: https://issues.apache.org/jira/browse/CASSANDRA-19210 > Project: Cassandra > Issue Type: Improvement > Components: Test/fuzz >Reporter: Alex Petrov >Assignee: Alex Petrov >Priority: High > Fix For: 5.1-alpha1 > > Attachments: ci_summary.html, result_details.tar.gz > > > This patch introduces Harry into tree. > _No_ modifications were made to Harry code, so technically our tree has > already relied on it. The intention to introduce Harry to tree to lower the > entry barrier and allow quicker developer turnarounds. > CI results attached; all tests are green. Python tests are skipped because > this patch does not make any changes to production code, it's all testonly. -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Updated] (CASSANDRA-19210) Bring Harry into OSS Cassandra tree
[ https://issues.apache.org/jira/browse/CASSANDRA-19210?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Alex Petrov updated CASSANDRA-19210: Status: Ready to Commit (was: Review In Progress) > Bring Harry into OSS Cassandra tree > --- > > Key: CASSANDRA-19210 > URL: https://issues.apache.org/jira/browse/CASSANDRA-19210 > Project: Cassandra > Issue Type: Improvement > Components: Test/fuzz >Reporter: Alex Petrov >Assignee: Alex Petrov >Priority: High > Attachments: ci_summary.html, result_details.tar.gz > > > This patch introduces Harry into tree. > _No_ modifications were made to Harry code, so technically our tree has > already relied on it. The intention to introduce Harry to tree to lower the > entry barrier and allow quicker developer turnarounds. > CI results attached; all tests are green. Python tests are skipped because > this patch does not make any changes to production code, it's all testonly. -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
