CASSANDRA-17537: nodetool compact should support using a key string to find the range to avoid operators having to manually do this #1567
Conversation
dcapwell
force-pushed
the
CASSANDRA-17537
branch
from
880e349
to
33bf364
Compare
| // we check index file instead. | ||
| if (sstable.getBloomFilter() instanceof AlwaysPresentFilter && sstable.getPosition(key, SSTableReader.Operator.EQ, false) != null | ||
| || sstable.getBloomFilter().isPresent(key)) | ||
| if (sstable.isPresent(key)) |
There was a problem hiding this comment.
moved this logic into the sstable so it can be reused
| @@ -928,10 +929,10 @@ protected void runMayThrow() | |||
| return futures; | |||
| } | |||
|
|
|||
| public void forceCompactionForTokenRange(ColumnFamilyStore cfStore, Collection<Range<Token>> ranges) | |||
| public void forceCompaction(ColumnFamilyStore cfStore, Supplier<Collection<SSTableReader>> sstablesFn, com.google.common.base.Predicate<SSTableReader> sstablesPredicate) | |||
There was a problem hiding this comment.
It is slightly weird to have both the Supplier and the Predicate here - but i guess its needed due to that sstablesInBounds method. Maybe we should add an assertion that sstablesPredicate.test(..) returns true for sstables from sstablesFn
There was a problem hiding this comment.
not sure about this one, sstablesFn is called when compaction is paused, so if I call earlier than that could that not cause problems? Unless I call early not sure how to get access to the SSTables
There was a problem hiding this comment.
I meant that after calling sstablesFn.get() in taskCreator we just assert sstables.stream().allMatch(sstablesPredicate);
but its fine leaving out as well
There was a problem hiding this comment.
added
List<SSTableReader> sstablesNotInPredicate = sstables.stream().filter(a -> ! sstablesPredicate.test(a)).collect(Collectors.toList());
assert sstablesNotInPredicate.isEmpty() : String.format("Attempted to force compact %s, but predicate does not include", sstablesNotInPredicate);
I prefer it show which sstables not included than existence
There was a problem hiding this comment.
this check broke some tests due to (32, 31] finding files that had bounds (32, 32]
src/java/org/apache/cassandra/db/compaction/CompactionManager.java
Outdated
Show resolved
Hide resolved
src/java/org/apache/cassandra/io/sstable/format/SSTableReader.java
Outdated
Show resolved
Hide resolved
src/java/org/apache/cassandra/io/sstable/format/SSTableReader.java
Outdated
Show resolved
Hide resolved
…o find the range to avoid operators having to manually do this
…ated tests to show this
… directly access bf to avoid resource leak detection
dcapwell
force-pushed
the
CASSANDRA-17537
branch
from
3aa3373
to
676c52b
Compare
No description provided.