cassandra icon indicating copy to clipboard operation
cassandra copied to clipboard

STAR-1314 Homogenize to use public getters

Open k-rus opened this issue 2 years ago • 3 comments

There are public methods to get table and keyspace names from CFS and they are already in use in some places. So use them consistently instead of direct access to the attributes.

Introduce and use getter to retrieve KeyspaceMetrics from CFS, so it can be mocked.

k-rus avatar May 13 '22 16:05 k-rus

Coverage issues:

  • ActiveRepairService.getPendingStats and ActiveRepairService.cleanupPending contains the refactored line and is used in NodeTool.
  • CassandraCompressedStreamReader.read contains two calls in logging, one for debug and another in catch.
  • CassandraDaemon contains line in Logger.Info to be called when autocompaction is not enabled on start.
  • CassandraStreamReceiver.finished contains two calls in Logger.debug.
  • RepairRunnable.runMayThrow and RepairRunnable.maybeCreateTraceState contains calls.
  • SortedLocalRanges. getLocalRanges contains a call in a branch, which is not tested.
  • TableMetricTables.TableMetricTable.data contains a call but the method is not covered.
  • CassandraStreamReader.read contains a call in Logger.warn in a catch block.
  • ViewBuilderTask.call contains a call in Logger.warn in uncovered branch.
  • Keyspace.createOfflineTable contains a call to prevent double init, which is not tested.
  • SinglePartitionReadCommand.getThroughCache contains a line in a message in an assert, which is never fired. 🛑
  • CompactionManager.parallelAllSSTableOperation contains a call to log the case when lifecycle transaction failed to cleanup on a failure. 🛑
  • TableMetrics.totalNonSystemTablesSize contains a call in uncovered branch when number of replicas bigger than two. Tested outside unit tests?

k-rus avatar May 15 '22 07:05 k-rus

My concern about this is that the cassandra code style is to not use getters where possible and to prefer public final fields. I understand the reasoning behind these changes but I am concerned that this is causing more divergence from the OS codebase than is strictly necessary.

mike-tr-adamson avatar May 16 '22 12:05 mike-tr-adamson