ekaterinadimitrova2 commented on a change in pull request #856:
URL: https://github.com/apache/cassandra/pull/856#discussion_r559845570
##########
File path: src/java/org/apache/cassandra/db/compaction/CompactionManager.java
##########
@@ -660,13 +660,9 @@ private boolean inCorrectLocation(SSTableReader sstable)
if (!cfs.getPartitioner().splitter().isPresent())
return true;
- int diskIndex = diskBoundaries.getDiskIndex(sstable);
- PartitionPosition diskLast =
diskBoundaries.positions.get(diskIndex);
-
- // the location we get from directoryIndex is based on the
first key in the sstable
- // now we need to make sure the last key is less than the
boundary as well:
- Directories.DataDirectory dataDirectory =
cfs.getDirectories().getDataDirectoryForFile(sstable.descriptor);
- return
diskBoundaries.directories.get(diskIndex).equals(dataDirectory) &&
sstable.last.compareTo(diskLast) <= 0;
+ // Compare the expected data directory for the sstable with
its current data directory
+ Directories.DataDirectory currentDirectory =
cfs.getDirectories().getDataDirectoryForFile(sstable.descriptor);
+ return diskBoundaries.isMisplaced(sstable, currentDirectory);
Review comment:
Shouldn't it be `return !diskBoundaries.isMisplaced(sstable,
currentDirectory);`?
My reading is that if it is in the correct location it won't be misplaced.
Also, I see you do the opposite check in diskBoundaries.isMisplaced
We don't have inCorrectLocation tested in the unit or in-jvm tests as far as
I can see, that is why it didn't pop up as an issue I think
----------------------------------------------------------------
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
