[
https://issues.apache.org/jira/browse/CASSANDRA-14654?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16812510#comment-16812510
]
Benedict edited comment on CASSANDRA-14654 at 4/8/19 3:35 PM:
--------------------------------------------------------------
Overall the patch looks pretty good. I've pushed some suggestions
[here|https://github.com/belliottsmith/cassandra/tree/14654] for you to
consider.
# preemptive was misspelled (I think this was inherited from an old
misspelling, but probably better to fix it before we expose over JMX)
# Tweaked EncodingStats merging to be agnostic to source type, and also avoid
unnecessary garbage if there's only one stats to merge
# {{keyCacheEnabled}} and {{getMigrateKeycacheOnCompaction}} renamed to
predicates _is_KeyCacheEnabled and _should_Migrate..
# Removed one unnecessary check from {{isKeyCacheEnabled}}, and removed
functionally equivalent {{isKeyCacheSetup}} (this required a bit of tweaking to
the test that used it, but IMO worth cleaning up)
# {{sstable_preemptive_open_interval_in_mb}} should probably be volatile now
we're updating it (though actually would be fine, we should make sure we are
technically correct)
Might also suggest a different name than {{migrate_keycache_on_compaction}} -
perhaps {{keycache_maintain_across_compaction}}, or
{{keycache_survives_compaction}}?
was (Author: benedict):
Overall the patch looks pretty good. I've pushed some suggestions
[here|https://github.com/belliottsmith/cassandra/tree/14654] for you to
consider.
Specifically:
# {{sstable_preemptive_open_interval_in_mb}} should probably be volatile now
we're updating it (though actually would be fine, we should make sure we are
technically correct)
# preemptive was misspelled (I think this was inherited from an old
misspelling, but probably better to fix it before we expose over JMX)
# Tweaked EncodingStats merging to be agnostic to source type, and also avoid
unnecessary garbage if there's only one stats to merge
# {{keyCacheEnabled}} and {{getMigrateKeycacheOnCompaction}} renamed to
predicates _is_KeyCacheEnabled and _should_Migrate..
# Removed one unnecessary check from {{isKeyCacheEnabled}}, and removed
functionally equivalent {{isKeyCacheSetup}} (this required a bit of tweaking to
the test that used it, but IMO worth cleaning up)
Might also suggest a different name than {{migrate_keycache_on_compaction}} -
perhaps {{keycache_maintain_across_compaction}}, or
{{keycache_survives_compaction}}?
> Reduce heap pressure during compactions
> ---------------------------------------
>
> Key: CASSANDRA-14654
> URL: https://issues.apache.org/jira/browse/CASSANDRA-14654
> Project: Cassandra
> Issue Type: Improvement
> Components: Local/Compaction
> Reporter: Chris Lohfink
> Assignee: Chris Lohfink
> Priority: Normal
> Labels: Performance, pull-request-available
> Fix For: 4.x
>
> Attachments: screenshot-1.png, screenshot-2.png, screenshot-3.png,
> screenshot-4.png
>
> Time Spent: 40m
> Remaining Estimate: 0h
>
> Small partition compactions are painfully slow with a lot of overhead per
> partition. There also tends to be an excess of objects created (ie
> 200-700mb/s) per compaction thread.
> The EncoderStats walks through all the partitions and with mergeWith it will
> create a new one per partition as it walks the potentially millions of
> partitions. In a test scenario of about 600byte partitions and a couple 100mb
> of data this consumed ~16% of the heap pressure. Changing this to instead
> mutably track the min values and create one in a EncodingStats.Collector
> brought this down considerably (but not 100% since the
> UnfilteredRowIterator.stats() still creates 1 per partition).
> The KeyCacheKey makes a full copy of the underlying byte array in
> ByteBufferUtil.getArray in its constructor. This is the dominating heap
> pressure as there are more sstables. By changing this to just keeping the
> original it completely eliminates the current dominator of the compactions
> and also improves read performance.
> Minor tweak included for this as well for operators when compactions are
> behind on low read clusters is to make the preemptive opening setting a
> hotprop.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
