You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by GitBox <gi...@apache.org> on 2020/11/20 22:00:55 UTC
[GitHub] [cassandra] dcapwell opened a new pull request #831: CASSANDRA-16095 - Nodetool tablestats WriteLatency error
dcapwell opened a new pull request #831:
URL: https://github.com/apache/cassandra/pull/831
----------------------------------------------------------------
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.
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] dcapwell commented on a change in pull request #831: CASSANDRA-16095 - Nodetool tablestats WriteLatency error
Posted by GitBox <gi...@apache.org>.
dcapwell commented on a change in pull request #831:
URL: https://github.com/apache/cassandra/pull/831#discussion_r528024350
##########
File path: test/distributed/org/apache/cassandra/distributed/test/metric/TableMetricTest.java
##########
@@ -0,0 +1,210 @@
+/*
+ * 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 org.apache.cassandra.distributed.test.metric;
+
+import java.io.IOException;
+import java.util.Collection;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+import javax.management.InstanceAlreadyExistsException;
+import javax.management.InstanceNotFoundException;
+import javax.management.ObjectName;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+import org.junit.Assert;
+import org.junit.Test;
+
+import org.apache.cassandra.db.ColumnFamilyStore;
+import org.apache.cassandra.distributed.Cluster;
+import org.apache.cassandra.distributed.api.IInvokableInstance;
+import org.apache.cassandra.distributed.test.TestBaseImpl;
+import org.apache.cassandra.utils.MBeanWrapper;
+
+import static org.apache.cassandra.config.CassandraRelevantProperties.IS_DISABLED_MBEAN_REGISTRATION;
+import static org.apache.cassandra.config.CassandraRelevantProperties.MBEAN_REGISTRATION_CLASS;
+
+public class TableMetricTest extends TestBaseImpl
+{
+ static
+ {
+ MBEAN_REGISTRATION_CLASS.setString(MapMBeanWrapper.class.getName());
+ IS_DISABLED_MBEAN_REGISTRATION.setBoolean(false);
+ }
+
+ private static Map<String, Collection<String>> SYSTEM_TABLES = ImmutableMap.of(
+ "system", ImmutableSet.of("available_ranges", "available_ranges_v2", "batches", "built_views", "compaction_history", "IndexInfo", "local",
Review comment:
I don't see that in 3.0 or trunk...
```
$ grep -r ALL_TABLE_NAMES src/java/
$
```
----------------------------------------------------------------
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.
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] dcapwell commented on a change in pull request #831: CASSANDRA-16095 - Nodetool tablestats WriteLatency error
Posted by GitBox <gi...@apache.org>.
dcapwell commented on a change in pull request #831:
URL: https://github.com/apache/cassandra/pull/831#discussion_r527996075
##########
File path: test/distributed/org/apache/cassandra/distributed/impl/AbstractCluster.java
##########
@@ -313,6 +314,13 @@ public void uncaughtException(Thread thread, Throwable throwable)
else
logger.error("uncaught exception in thread {}", thread, throwable);
}
+
+ @Override
+ public String toString()
+ {
+ IInvokableInstance delegate = this.delegate;
Review comment:
did this in https://issues.apache.org/jira/browse/CASSANDRA-16213 as its useful for debugging.
----------------------------------------------------------------
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.
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] dcapwell commented on a change in pull request #831: CASSANDRA-16095 - Nodetool tablestats WriteLatency error
Posted by GitBox <gi...@apache.org>.
dcapwell commented on a change in pull request #831:
URL: https://github.com/apache/cassandra/pull/831#discussion_r527994984
##########
File path: src/java/org/apache/cassandra/metrics/TableMetrics.java
##########
@@ -62,6 +62,10 @@
*/
public class TableMetrics
{
+ /**
+ * stores metrics that will be rolled into a single global metric
+ */
+ private final static ConcurrentMap<String, Set<Metric>> ALL_TABLE_METRICS = Maps.newConcurrentMap();
Review comment:
renamed to all caps so its more clear this is global and not local to the table; seems that this naming convention caused `all` to be confused with local table names rather than global metrics, so switch to java naming convention to help to prevent this next time.
----------------------------------------------------------------
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.
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] dcapwell commented on a change in pull request #831: CASSANDRA-16095 - Nodetool tablestats WriteLatency error
Posted by GitBox <gi...@apache.org>.
dcapwell commented on a change in pull request #831:
URL: https://github.com/apache/cassandra/pull/831#discussion_r532788627
##########
File path: test/distributed/org/apache/cassandra/distributed/test/metric/TableMetricTest.java
##########
@@ -0,0 +1,210 @@
+/*
+ * 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 org.apache.cassandra.distributed.test.metric;
+
+import java.io.IOException;
+import java.util.Collection;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+import javax.management.InstanceAlreadyExistsException;
+import javax.management.InstanceNotFoundException;
+import javax.management.ObjectName;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+import org.junit.Assert;
+import org.junit.Test;
+
+import org.apache.cassandra.db.ColumnFamilyStore;
+import org.apache.cassandra.distributed.Cluster;
+import org.apache.cassandra.distributed.api.IInvokableInstance;
+import org.apache.cassandra.distributed.test.TestBaseImpl;
+import org.apache.cassandra.utils.MBeanWrapper;
+
+import static org.apache.cassandra.config.CassandraRelevantProperties.IS_DISABLED_MBEAN_REGISTRATION;
+import static org.apache.cassandra.config.CassandraRelevantProperties.MBEAN_REGISTRATION_CLASS;
+
+public class TableMetricTest extends TestBaseImpl
+{
+ static
+ {
+ MBEAN_REGISTRATION_CLASS.setString(MapMBeanWrapper.class.getName());
+ IS_DISABLED_MBEAN_REGISTRATION.setBoolean(false);
+ }
+
+ private static Map<String, Collection<String>> SYSTEM_TABLES = ImmutableMap.of(
+ "system", ImmutableSet.of("available_ranges", "available_ranges_v2", "batches", "built_views", "compaction_history", "IndexInfo", "local",
Review comment:
I switched to doing this, I do this by loading all tables in the first 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.
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] dcapwell commented on pull request #831: CASSANDRA-16095 - Nodetool tablestats WriteLatency error
Posted by GitBox <gi...@apache.org>.
dcapwell commented on pull request #831:
URL: https://github.com/apache/cassandra/pull/831#issuecomment-731459812
I updated to also check the keyspace metric, there isn't a keyspace mbean to validate, so only look for the keyspace version of the metrics list
----------------------------------------------------------------
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.
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] dcapwell commented on pull request #831: CASSANDRA-16095 - Nodetool tablestats WriteLatency error
Posted by GitBox <gi...@apache.org>.
dcapwell commented on pull request #831:
URL: https://github.com/apache/cassandra/pull/831#issuecomment-731456098
> LGTM.
>
> Is it worth also checking the KeyspaceMetrics are created and removed and that dropping a Keyspace removes all contained table metrics?
I thought about that but was lazy!
----------------------------------------------------------------
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.
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] yifan-c commented on a change in pull request #831: CASSANDRA-16095 - Nodetool tablestats WriteLatency error
Posted by GitBox <gi...@apache.org>.
yifan-c commented on a change in pull request #831:
URL: https://github.com/apache/cassandra/pull/831#discussion_r528031218
##########
File path: src/java/org/apache/cassandra/metrics/TableMetrics.java
##########
@@ -62,7 +62,17 @@
*/
public class TableMetrics
{
+ /**
+ * stores metrics that will be rolled into a single global metric
+ */
+ private static final ConcurrentMap<String, Set<Metric>> ALL_TABLE_METRICS = Maps.newConcurrentMap();
public static final long[] EMPTY = new long[0];
+ private static final MetricNameFactory GLOBAL_FACTORY = new AllTableMetricNameFactory("Table");
+ private static final MetricNameFactory GLOBAL_ALIAS_FACTORY = new AllTableMetricNameFactory("ColumnFamily");
+
+ public final static LatencyMetrics GLOBAL_READ_LATENCY = new LatencyMetrics(GLOBAL_FACTORY, GLOBAL_ALIAS_FACTORY, "Read");
+ public final static LatencyMetrics GLOBAL_WRITE_LATENCY = new LatencyMetrics(GLOBAL_FACTORY, GLOBAL_ALIAS_FACTORY, "Write");
+ public final static LatencyMetrics GLOBAL_RANGE_LATENCY = new LatencyMetrics(GLOBAL_FACTORY, GLOBAL_ALIAS_FACTORY, "Range");
Review comment:
great to group the static variable together.
##########
File path: src/java/org/apache/cassandra/metrics/TableMetrics.java
##########
@@ -609,7 +609,7 @@ public Long getValue()
public Long getValue()
{
long min = Long.MAX_VALUE;
- for (Metric cfGauge : allTableMetrics.get("MinPartitionSize"))
+ for (Metric cfGauge : ALL_TABLE_METRICS.get("MinPartitionSize"))
Review comment:
nit: maybe also create constants for the metrics names? since this patch is doing some code cleanup already.
(Maybe too much work. your call)
##########
File path: src/java/org/apache/cassandra/config/CassandraRelevantProperties.java
##########
@@ -196,6 +199,28 @@ public String getString()
return value == null ? defaultVal : STRING_CONVERTER.convert(value);
}
+ /**
Review comment:
The get with overrideDefaultValue methods are not used. Not a big problem if the other patch referenced is to be merged.
##########
File path: test/distributed/org/apache/cassandra/distributed/test/metric/TableMetricTest.java
##########
@@ -0,0 +1,210 @@
+/*
+ * 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 org.apache.cassandra.distributed.test.metric;
+
+import java.io.IOException;
+import java.util.Collection;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+import javax.management.InstanceAlreadyExistsException;
+import javax.management.InstanceNotFoundException;
+import javax.management.ObjectName;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+import org.junit.Assert;
+import org.junit.Test;
+
+import org.apache.cassandra.db.ColumnFamilyStore;
+import org.apache.cassandra.distributed.Cluster;
+import org.apache.cassandra.distributed.api.IInvokableInstance;
+import org.apache.cassandra.distributed.test.TestBaseImpl;
+import org.apache.cassandra.utils.MBeanWrapper;
+
+import static org.apache.cassandra.config.CassandraRelevantProperties.IS_DISABLED_MBEAN_REGISTRATION;
+import static org.apache.cassandra.config.CassandraRelevantProperties.MBEAN_REGISTRATION_CLASS;
+
+public class TableMetricTest extends TestBaseImpl
+{
+ static
+ {
+ MBEAN_REGISTRATION_CLASS.setString(MapMBeanWrapper.class.getName());
+ IS_DISABLED_MBEAN_REGISTRATION.setBoolean(false);
+ }
+
+ private static Map<String, Collection<String>> SYSTEM_TABLES = ImmutableMap.of(
+ "system", ImmutableSet.of("available_ranges", "available_ranges_v2", "batches", "built_views", "compaction_history", "IndexInfo", "local",
Review comment:
I think it is better to load the tables programmatically. So compiler prompts when code changes. It can be done via
```java
Map<String, Collection<String>> systemTables = new HashMap<>();
Consumer<KeyspaceMetadata> loader = meta ->
{
Set<String> tables = meta.tables.stream().map(t -> t.name).collect(Collectors.toSet());
systemTables.put(meta.name, tables);
};
Arrays.asList(SystemKeyspace.metadata(), AuthKeyspace.metadata(), SystemDistributedKeyspace.metadata(),
SchemaKeyspace.metadata(), TraceKeyspace.metadata())
.forEach(loader);
```
----------------------------------------------------------------
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.
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] dcapwell commented on a change in pull request #831: CASSANDRA-16095 - Nodetool tablestats WriteLatency error
Posted by GitBox <gi...@apache.org>.
dcapwell commented on a change in pull request #831:
URL: https://github.com/apache/cassandra/pull/831#discussion_r527996485
##########
File path: test/distributed/org/apache/cassandra/distributed/impl/Instance.java
##########
@@ -485,11 +485,10 @@ public void startup(ICluster cluster)
GossipHelper.unsafeStatusToNormal(this, (IInstance) peer);
});
+ StorageService.instance.setUpDistributedSystemKeyspaces();
Review comment:
this was a regressions... when not using gossip we didn't register the distributed tables, and we tried to force trace regardless...
right before we gossip tokens around we call this method, so adding it here makes sure the distributed tables work with and without gossip.
----------------------------------------------------------------
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.
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] dcapwell commented on a change in pull request #831: CASSANDRA-16095 - Nodetool tablestats WriteLatency error
Posted by GitBox <gi...@apache.org>.
dcapwell commented on a change in pull request #831:
URL: https://github.com/apache/cassandra/pull/831#discussion_r528024350
##########
File path: test/distributed/org/apache/cassandra/distributed/test/metric/TableMetricTest.java
##########
@@ -0,0 +1,210 @@
+/*
+ * 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 org.apache.cassandra.distributed.test.metric;
+
+import java.io.IOException;
+import java.util.Collection;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+import javax.management.InstanceAlreadyExistsException;
+import javax.management.InstanceNotFoundException;
+import javax.management.ObjectName;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+import org.junit.Assert;
+import org.junit.Test;
+
+import org.apache.cassandra.db.ColumnFamilyStore;
+import org.apache.cassandra.distributed.Cluster;
+import org.apache.cassandra.distributed.api.IInvokableInstance;
+import org.apache.cassandra.distributed.test.TestBaseImpl;
+import org.apache.cassandra.utils.MBeanWrapper;
+
+import static org.apache.cassandra.config.CassandraRelevantProperties.IS_DISABLED_MBEAN_REGISTRATION;
+import static org.apache.cassandra.config.CassandraRelevantProperties.MBEAN_REGISTRATION_CLASS;
+
+public class TableMetricTest extends TestBaseImpl
+{
+ static
+ {
+ MBEAN_REGISTRATION_CLASS.setString(MapMBeanWrapper.class.getName());
+ IS_DISABLED_MBEAN_REGISTRATION.setBoolean(false);
+ }
+
+ private static Map<String, Collection<String>> SYSTEM_TABLES = ImmutableMap.of(
+ "system", ImmutableSet.of("available_ranges", "available_ranges_v2", "batches", "built_views", "compaction_history", "IndexInfo", "local",
Review comment:
I don't see that in 3.0 or trunk...
----------------------------------------------------------------
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.
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] dcapwell commented on a change in pull request #831: CASSANDRA-16095 - Nodetool tablestats WriteLatency error
Posted by GitBox <gi...@apache.org>.
dcapwell commented on a change in pull request #831:
URL: https://github.com/apache/cassandra/pull/831#discussion_r528051579
##########
File path: test/distributed/org/apache/cassandra/distributed/test/metric/TableMetricTest.java
##########
@@ -0,0 +1,210 @@
+/*
+ * 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 org.apache.cassandra.distributed.test.metric;
+
+import java.io.IOException;
+import java.util.Collection;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+import javax.management.InstanceAlreadyExistsException;
+import javax.management.InstanceNotFoundException;
+import javax.management.ObjectName;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+import org.junit.Assert;
+import org.junit.Test;
+
+import org.apache.cassandra.db.ColumnFamilyStore;
+import org.apache.cassandra.distributed.Cluster;
+import org.apache.cassandra.distributed.api.IInvokableInstance;
+import org.apache.cassandra.distributed.test.TestBaseImpl;
+import org.apache.cassandra.utils.MBeanWrapper;
+
+import static org.apache.cassandra.config.CassandraRelevantProperties.IS_DISABLED_MBEAN_REGISTRATION;
+import static org.apache.cassandra.config.CassandraRelevantProperties.MBEAN_REGISTRATION_CLASS;
+
+public class TableMetricTest extends TestBaseImpl
+{
+ static
+ {
+ MBEAN_REGISTRATION_CLASS.setString(MapMBeanWrapper.class.getName());
+ IS_DISABLED_MBEAN_REGISTRATION.setBoolean(false);
+ }
+
+ private static Map<String, Collection<String>> SYSTEM_TABLES = ImmutableMap.of(
+ "system", ImmutableSet.of("available_ranges", "available_ranges_v2", "batches", "built_views", "compaction_history", "IndexInfo", "local",
Review comment:
that has the same issue, we load the whole DB into the app space, so was trying to avoid that.
----------------------------------------------------------------
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.
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] dcapwell commented on a change in pull request #831: CASSANDRA-16095 - Nodetool tablestats WriteLatency error
Posted by GitBox <gi...@apache.org>.
dcapwell commented on a change in pull request #831:
URL: https://github.com/apache/cassandra/pull/831#discussion_r527994342
##########
File path: src/java/org/apache/cassandra/config/CassandraRelevantProperties.java
##########
@@ -196,6 +199,28 @@ public String getString()
return value == null ? defaultVal : STRING_CONVERTER.convert(value);
}
+ /**
Review comment:
Pulled from https://issues.apache.org/jira/browse/CASSANDRA-16213, was going to add them there but need them in this patch as well, so copy/pasted from there (but added docs for the ones missing)
----------------------------------------------------------------
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.
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] jonmeredith commented on a change in pull request #831: CASSANDRA-16095 - Nodetool tablestats WriteLatency error
Posted by GitBox <gi...@apache.org>.
jonmeredith commented on a change in pull request #831:
URL: https://github.com/apache/cassandra/pull/831#discussion_r528027285
##########
File path: test/distributed/org/apache/cassandra/distributed/test/metric/TableMetricTest.java
##########
@@ -0,0 +1,210 @@
+/*
+ * 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 org.apache.cassandra.distributed.test.metric;
+
+import java.io.IOException;
+import java.util.Collection;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+import javax.management.InstanceAlreadyExistsException;
+import javax.management.InstanceNotFoundException;
+import javax.management.ObjectName;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+import org.junit.Assert;
+import org.junit.Test;
+
+import org.apache.cassandra.db.ColumnFamilyStore;
+import org.apache.cassandra.distributed.Cluster;
+import org.apache.cassandra.distributed.api.IInvokableInstance;
+import org.apache.cassandra.distributed.test.TestBaseImpl;
+import org.apache.cassandra.utils.MBeanWrapper;
+
+import static org.apache.cassandra.config.CassandraRelevantProperties.IS_DISABLED_MBEAN_REGISTRATION;
+import static org.apache.cassandra.config.CassandraRelevantProperties.MBEAN_REGISTRATION_CLASS;
+
+public class TableMetricTest extends TestBaseImpl
+{
+ static
+ {
+ MBEAN_REGISTRATION_CLASS.setString(MapMBeanWrapper.class.getName());
+ IS_DISABLED_MBEAN_REGISTRATION.setBoolean(false);
+ }
+
+ private static Map<String, Collection<String>> SYSTEM_TABLES = ImmutableMap.of(
+ "system", ImmutableSet.of("available_ranges", "available_ranges_v2", "batches", "built_views", "compaction_history", "IndexInfo", "local",
Review comment:
My mistake :)
----------------------------------------------------------------
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.
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] dcapwell commented on a change in pull request #831: CASSANDRA-16095 - Nodetool tablestats WriteLatency error
Posted by GitBox <gi...@apache.org>.
dcapwell commented on a change in pull request #831:
URL: https://github.com/apache/cassandra/pull/831#discussion_r528020034
##########
File path: test/distributed/org/apache/cassandra/distributed/test/metric/TableMetricTest.java
##########
@@ -0,0 +1,210 @@
+/*
+ * 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 org.apache.cassandra.distributed.test.metric;
+
+import java.io.IOException;
+import java.util.Collection;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+import javax.management.InstanceAlreadyExistsException;
+import javax.management.InstanceNotFoundException;
+import javax.management.ObjectName;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+import org.junit.Assert;
+import org.junit.Test;
+
+import org.apache.cassandra.db.ColumnFamilyStore;
+import org.apache.cassandra.distributed.Cluster;
+import org.apache.cassandra.distributed.api.IInvokableInstance;
+import org.apache.cassandra.distributed.test.TestBaseImpl;
+import org.apache.cassandra.utils.MBeanWrapper;
+
+import static org.apache.cassandra.config.CassandraRelevantProperties.IS_DISABLED_MBEAN_REGISTRATION;
+import static org.apache.cassandra.config.CassandraRelevantProperties.MBEAN_REGISTRATION_CLASS;
+
+public class TableMetricTest extends TestBaseImpl
+{
+ static
+ {
+ MBEAN_REGISTRATION_CLASS.setString(MapMBeanWrapper.class.getName());
+ IS_DISABLED_MBEAN_REGISTRATION.setBoolean(false);
+ }
+
+ private static Map<String, Collection<String>> SYSTEM_TABLES = ImmutableMap.of(
+ "system", ImmutableSet.of("available_ranges", "available_ranges_v2", "batches", "built_views", "compaction_history", "IndexInfo", "local",
Review comment:
I didn't want to as that would load the whole DB, I could refactor to avoid that but felt simplest to just fork.
----------------------------------------------------------------
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.
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] smiklosovic closed pull request #831: CASSANDRA-16095 - Nodetool tablestats WriteLatency error
Posted by GitBox <gi...@apache.org>.
smiklosovic closed pull request #831:
URL: https://github.com/apache/cassandra/pull/831
--
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] jonmeredith commented on a change in pull request #831: CASSANDRA-16095 - Nodetool tablestats WriteLatency error
Posted by GitBox <gi...@apache.org>.
jonmeredith commented on a change in pull request #831:
URL: https://github.com/apache/cassandra/pull/831#discussion_r528015547
##########
File path: test/distributed/org/apache/cassandra/distributed/test/metric/TableMetricTest.java
##########
@@ -0,0 +1,210 @@
+/*
+ * 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 org.apache.cassandra.distributed.test.metric;
+
+import java.io.IOException;
+import java.util.Collection;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+import javax.management.InstanceAlreadyExistsException;
+import javax.management.InstanceNotFoundException;
+import javax.management.ObjectName;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+import org.junit.Assert;
+import org.junit.Test;
+
+import org.apache.cassandra.db.ColumnFamilyStore;
+import org.apache.cassandra.distributed.Cluster;
+import org.apache.cassandra.distributed.api.IInvokableInstance;
+import org.apache.cassandra.distributed.test.TestBaseImpl;
+import org.apache.cassandra.utils.MBeanWrapper;
+
+import static org.apache.cassandra.config.CassandraRelevantProperties.IS_DISABLED_MBEAN_REGISTRATION;
+import static org.apache.cassandra.config.CassandraRelevantProperties.MBEAN_REGISTRATION_CLASS;
+
+public class TableMetricTest extends TestBaseImpl
+{
+ static
+ {
+ MBEAN_REGISTRATION_CLASS.setString(MapMBeanWrapper.class.getName());
+ IS_DISABLED_MBEAN_REGISTRATION.setBoolean(false);
+ }
+
+ private static Map<String, Collection<String>> SYSTEM_TABLES = ImmutableMap.of(
+ "system", ImmutableSet.of("available_ranges", "available_ranges_v2", "batches", "built_views", "compaction_history", "IndexInfo", "local",
Review comment:
Could you use `org.apache.cassandra.db.SystemKeyspace#ALL_TABLE_NAMES` here? and similarly create ALL_TABLE_NAMES for the other keyspaces where they're missing?
----------------------------------------------------------------
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.
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