[GitHub] [cassandra] ekaterinadimitrova2 commented on a diff in pull request #1742: CASSANDRA-17575 3.11: Fix compaction token range bounds

[GitBox <gi...@apache.org>]

ekaterinadimitrova2 commented on code in PR #1742:
URL: https://github.com/apache/cassandra/pull/1742#discussion_r940673204


##########
src/java/org/apache/cassandra/service/StorageServiceMBean.java:
##########
@@ -265,8 +265,14 @@ public interface StorageServiceMBean extends NotificationEmitter
     @Deprecated
     public int relocateSSTables(String keyspace, String ... cfnames) throws IOException, ExecutionException, InterruptedException;
     public int relocateSSTables(int jobs, String keyspace, String ... cfnames) throws IOException, ExecutionException, InterruptedException;
+
     /**
-     * Forces major compaction of specified token range in a single keyspace
+     * Forces major compaction of specified token range in a single keyspace.
+     *
+     * @param keyspaceName the name of the keyspace to be compacted
+     * @param startToken the token at which the compaction range starts (inclusive)
+     * @param endToken the token at which compaction range ends (inclusive)
+     * @param tableNames the names of the tables to be compacted

Review Comment:
   Do we want also to add java doc to `forceKeyspaceCompactionForTokenRange` in `NodeProbe`, for completeness?



##########
src/java/org/apache/cassandra/db/compaction/CompactionManager.java:
##########
@@ -812,11 +812,20 @@ protected void runMayThrow()
         return futures;
     }
 
+    /**
+     * Forces a major compaction of specified token ranges of the specified column family.
+     * <p>
+     * The token ranges will be interpreted as closed intervals to match the closed interval defined by the first and
+     * last keys of a sstable, even though the {@link Range} class is suppossed to be half-open by definition.
+     *
+     * @param cfStore The column family store to be compacted.
+     * @param ranges The token ranges to be compacted, interpreted as closed intervals.
+     */
     public void forceCompactionForTokenRange(ColumnFamilyStore cfStore, Collection<Range<Token>> ranges)
     {
         Callable<Collection<AbstractCompactionTask>> taskCreator = () -> {
             Collection<SSTableReader> sstables = sstablesInBounds(cfStore, ranges);
-            if (sstables == null || sstables.isEmpty())
+            if (sstables.isEmpty())

Review Comment:
   This change is unrelated to the patch and my personal preference is to stick to the main patch in older versions



-- 
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