[ https://issues.apache.org/jira/browse/CASSANDRA-14523?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16530085#comment-16530085 ]
Benjamin Lerer commented on CASSANDRA-14523: -------------------------------------------- Thanks for the patch. I think we should take advantages of the virtual table patches to clean a bit the Metric classes. I pushed a commit [here|https://github.com/apache/cassandra/commit/7e8ec2f22c8d58351e72f550f811108fdc71a0a7] to remove the duplicated code in {{ThreadPoolMetrics}} and {{SEPMetrics}}. One common problem to expose the metrics through virtual tables when the {{DataSet}} is build on demand is to be able to retrieve the set of metrics. Due to that the current patch has to fetch the metric values through {{JMX}} and change the visibility of some thread pools. To avoid that problem, we could keep a concurrent {{DataSet}} in memory and force the metric classes to add the column values to the tables. I have pushed a patch to add a concurrent {{DataSet}} [here|https://github.com/apache/cassandra/commit/98ff447ec580bf42cc7141a4a484f79ef7f664a9] and one to use it in {{ThreadPoolMetrics}} [here|https://github.com/apache/cassandra/commit/960563fdc865c9c06b66c4f29b161ffbb3d7213d]. I had too change a bit the {{DataSet}} API to be able to add the concurrent {{DataSet}} but the new API for building the {{DataSet}} can be used for both type of {{DataSets}}. Adding the metrics for the {{ScheduledThreadPool}} is a good idea. I just wonder if we should not simply add some real metrics for them. We could do that by replacing the {{ScheduledThreadPoolExecutor}} from {{SSTableReader}} by a {{DebuggableScheduledThreadPoolExecutor}} and by having the {{DebuggableScheduledThreadPoolExecutor}} class creating a metric for itself. The advantage of that approach is that it will also be possible to monitor those thread pools through {{nodetool}} and {{JMX}}. Ideally, I think it should be done in another ticket. I noticed that the patch reformat the name of the pools for exposing them in the virtual table. We should probably keep the original name as it is a bit confusing if other tools and logs display a different name. It is not directly related to this patch but more a general remark about Virtual Tables: I wonder if we should not use a {{LocalPartitioner}} for them. Keeping the partition ordered by their partition key natural order is in my opinion an advantage for the virtual tables. ([~iamaleksey] do you see any problem with that?) I pushed some commits [here|https://github.com/apache/cassandra/compare/trunk...blerer:14523-trunk-review] to cover all the things I mentioned. What do you think [~cnlwsu]? > Thread pool stats virtual table > ------------------------------- > > Key: CASSANDRA-14523 > URL: https://issues.apache.org/jira/browse/CASSANDRA-14523 > Project: Cassandra > Issue Type: New Feature > Reporter: Chris Lohfink > Assignee: Chris Lohfink > Priority: Minor > Labels: virtual-tables > > Expose the thread pools like in status logger/tpstats. Additionally be nice > to include the scheduled executor pools that are currently unmonitored. > {code:java} > cqlsh> select * from system_views.thread_pools; > thread_pool | active | active_max | completed | pending > | tasks_blocked | total_blocked > ----------------------------------+--------+------------+-----------+---------+---------------+--------------- > anti_entropy_stage | 0 | 1 | 0 | 0 > | 0 | 0 > cache_cleanup_executor | 0 | 1 | 0 | 0 > | 0 | 0 > compaction_executor | 0 | 4 | 41 | 0 > | 0 | 0 > counter_mutation_stage | 0 | 32 | 0 | 0 > | 0 | 0 > gossip_stage | 0 | 1 | 0 | 0 > | 0 | 0 > hints_dispatcher | 0 | 2 | 0 | 0 > | 0 | 0 > internal_response_stage | 0 | 8 | 0 | 0 > | 0 | 0 > memtable_flush_writer | 0 | 2 | 5 | 0 > | 0 | 0 > memtable_post_flush | 0 | 1 | 20 | 0 > | 0 | 0 > memtable_reclaim_memory | 0 | 1 | 5 | 0 > | 0 | 0 > migration_stage | 0 | 1 | 0 | 0 > | 0 | 0 > misc_stage | 0 | 1 | 0 | 0 > | 0 | 0 > mutation_stage | 0 | 32 | 247 | 0 > | 0 | 0 > native_transport_requests | 1 | 128 | 28 | 0 > | 0 | 0 > pending_range_calculator | 0 | 1 | 2 | 0 > | 0 | 0 > per_disk_memtable_flush_writer_0 | 0 | 2 | 5 | 0 > | 0 | 0 > read_repair_stage | 0 | 8 | 0 | 0 > | 0 | 0 > read_stage | 0 | 32 | 13 | 0 > | 0 | 0 > repair_task | 0 | 2147483647 | 0 | 0 > | 0 | 0 > request_response_stage | 0 | 8 | 0 | 0 > | 0 | 0 > sampler | 0 | 1 | 0 | 0 > | 0 | 0 > scheduled_fast_tasks | 0 | 2147483647 | 1398 | 1 > | 0 | 0 > scheduled_heartbeat | 0 | 2147483647 | 14 | 1 > | 0 | 0 > scheduled_hotness_tracker | 0 | 2147483647 | 0 | 1 > | 0 | 0 > scheduled_non_periodic_tasks | 0 | 2147483647 | 10 | 0 > | 0 | 0 > scheduled_optional_tasks | 0 | 2147483647 | 5 | 8 > | 0 | 0 > scheduled_summary_builder | 0 | 2147483647 | 0 | 1 > | 0 | 0 > scheduled_tasks | 0 | 2147483647 | 194 | 74 > | 0 | 0 > secondary_index_management | 0 | 1 | 0 | 0 > | 0 | 0 > validation_executor | 0 | 2147483647 | 0 | 0 > | 0 | 0 > view_build_executor | 0 | 1 | 0 | 0 > | 0 | 0 > view_mutation_stage | 0 | 32 | 0 | 0 > | 0 | 0 > {code} -- This message was sent by Atlassian JIRA (v7.6.3#76005) --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org