CASSANDRA-21423: CQLSSTableWriter produces empty bloom filter#4929
Open
liuisaac wants to merge 4 commits into
Open
CASSANDRA-21423: CQLSSTableWriter produces empty bloom filter#4929liuisaac wants to merge 4 commits into
liuisaac wants to merge 4 commits into
Conversation
Rebuilds the Bloom filter for sstables produced by CQLSSTableWriter after the writer completes, since the writer sizes its filter from a hardcoded partition count of 0 (the real count is unknown up front), producing a negligibly small, saturated filter regardless of actual data written. Adds SSTableReaderLoadingBuilder#rebuildFilter, which reuses the existing per-format key-reading infrastructure to size the filter from the real partition count in Statistics.db and repopulate it from the on-disk index. AbstractSSTableSimpleWriter invokes this after each produced sstable and swaps the corrected filter into any already-opened reader via cloneAndReplace, so callers using openSSTableOnProduced also get the corrected filter rather than the writer's original in-memory copy. Patch by Isaac Liu; reviewed by TBD for CASSANDRA-21423
Guard rebuildBloomFilter against applying one sstable's filter to a different sstable's reader. Safe today, finish() only ever returns a singleton here, but would silently corrupt filters if reused elsewhere. Fails loudly via Preconditions.checkState instead. Fix rebuildFilter's catch block to delete the broken Filter.db on any failure, not just failures during save(). Previously a failure during the index walk left the original broken file untouched. Add a test forcing multiple sstable rotations, verifying every produced Filter.db is rebuilt, not just the first. Patch by Isaac Liu; reviewed by TBD for CASSANDRA-21423
…iuisaac/cassandra into liuisaac/CASSANDRA-21423/trunk
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue: CASSANDRA-21423
Purpose
CQLSSTableWriterdoes not know the key size onINSERTand thus produces near-empty Filter.db files. This saturates every flushed SSTable's Bloom Filter and recalculation never gets triggered, causing unnecessary disk reads for every partition key lookup to tables written viaCQLSSTableWriter.Context
CQLSSTableWriterallows callers to generate SSTable entries outside of a running Cassandra process.CQLSSTableWriterinitializes anAbstractSSTableSimpleWriterwriter that gets instantiated to eitherSSTableSimpleWriteror anSSTableSimpleUnsortedWriter. Either writer implementation callsAbstractSSTableSimpleWriter.createWriter()which hardcodes a partition count of 0, as neither path knows the true key count.Problem
Hardcoded 0 flows into
FilterFactory.getFilter(0, fpChance), which allocates a filter of 0 elements +BITSET_EXCESS(20 bits). Resulting Filter.db is ~16 bytes in size. Effectively every bloom filter check will fall into a disk read.Solution
Add
SSTableReaderLoadingBuilder.rebuildFilter(), which pullstotalRows(upper bound of partition count) from Statistics.db, then walk the on-disk primary index to repopulate the filter with every key, and overwrites Filter.db.AbstractSSTableSimpleWriterimplementations callrebuildFilter()after each SSTable is produced before notifyingsstableProducedListener. Becausefinish(openResult)may already have opened anSSTableReaderholding a shared in-memory copy of the original (broken) filter, copied in viagetFilterCopy()before this method runs, independent of what gets written to disk afterward, the fix also swaps the corrected filter into any already-opened readers viacloneAndReplace, releasing the original.Skips entirely when bloom filtering is disabled for the table. On rebuild failure, failed Filter.db is deleted so a node will safely build on load instead.