[
https://issues.apache.org/jira/browse/CASSANDRA-19429?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17877386#comment-17877386
]
Stefan Miklosovic edited comment on CASSANDRA-19429 at 8/28/24 1:24 PM:
------------------------------------------------------------------------
I am looking into these flame charts again and I don't think that "just
wrapping it and skipping it when logging" will help to solve the issue. If you
look into the screenshot from 2024-02-27, this code is in the hot path, it is
basically called every read following that stack.
I am looking into 4.1:
BigTableReader.rowIterator -> SSTableReader.getPosition ->
BigTableReader.getPosition -> SSTableReader.cacheKey -> that calls
.getCapacity().
{code}
public void cacheKey(DecoratedKey key, RowIndexEntry info)
{
CachingParams caching = metadata().params.caching;
if (!caching.cacheKeys() || keyCache == null || keyCache.getCapacity()
== 0)
return;
KeyCacheKey cacheKey = new KeyCacheKey(metadata(), descriptor,
key.getKey());
logger.trace("Adding cache entry for {} -> {}", cacheKey, info);
keyCache.put(cacheKey, info);
}
{code}
Here it checks .getCapacity() to check if it makes sense to add it there, all
the time all over again, which is imho the culprit. So we should definitely do
something about this as well. My improved patch which approaches this
holistically does solve this. We can not just blindly put there "if (... ||
DatabaseDescriptor.getKeyCacheSizeInMiB() == 0)" because the problem is that
the size of cache might be set in runtime via
CacheServiceMBean.setKeyCacheCapacityInMB() which operates directly on the
cache itself and it does not update / set the changed value back into
DatabaseDescriptor.
Hence, I believe that unless we treat this robustly as I suggested we will not
see the desired improvement.
[~brandon.williams] what do you think?
was (Author: smiklosovic):
I am looking into these flame charts again and I don't think that "just
wrapping it and skipping it when logging" will help to solve the issue. If you
look into the screenshot from 2024-02-27, this code is in the hot path, it is
basically called every read following that stack.
I am looking into 4.1:
BigTableReader.rowIterator -> SSTableReader.getPosition ->
BigTableReader.getPosition -> SSTableReader.cacheKey -> that calls
.getCapacity().
{code}
public void cacheKey(DecoratedKey key, RowIndexEntry info)
{
CachingParams caching = metadata().params.caching;
if (!caching.cacheKeys() || keyCache == null || keyCache.getCapacity()
== 0)
return;
KeyCacheKey cacheKey = new KeyCacheKey(metadata(), descriptor,
key.getKey());
logger.trace("Adding cache entry for {} -> {}", cacheKey, info);
keyCache.put(cacheKey, info);
}
{code}
Here it checks .getCapacity() to check if it makes sense to add it there, all
the time all over again, which is imho the culprit. So we should definitely do
something about this as well. My improved patch which approaches this
holistically does solve this. We can not just blindly put there "if (... ||
DatabaseDescriptor.getKeyCacheSizeInMiB() == 0)" because the problem is that
the size of cache might be set in runtime via
CacheServiceMBean.setKeyCacheCapacityInMB() which operates directly on a cache
itself and it does not update / set the changed value back into
DatabaseDescriptor.
Hence, I believe that unless we treat this robustly as I suggested we will not
see the desired improvement.
[~brandon.williams] what do you think?
> Remove lock contention generated by getCapacity function in SSTableReader
> -------------------------------------------------------------------------
>
> Key: CASSANDRA-19429
> URL: https://issues.apache.org/jira/browse/CASSANDRA-19429
> Project: Cassandra
> Issue Type: Bug
> Components: Local/SSTable
> Reporter: Dipietro Salvatore
> Assignee: Dipietro Salvatore
> Priority: Normal
> Fix For: 4.0.x, 4.1.x
>
> Attachments: Screenshot 2024-02-26 at 10.27.10.png, Screenshot
> 2024-02-27 at 11.29.41.png, Screenshot 2024-03-19 at 15.22.50.png,
> asprof_cass4.1.3__lock_20240216052912lock.html,
> image-2024-03-08-15-51-30-439.png, image-2024-03-08-15-52-07-902.png
>
> Time Spent: 40m
> Remaining Estimate: 0h
>
> Profiling Cassandra 4.1.3 on large AWS instances, a high number of lock
> acquires is measured in the `getCapacity` function from
> `org/apache/cassandra/cache/InstrumentingCache` (1.9M lock acquires per 60
> seconds). Based on our tests on r8g.24xlarge instances (using Ubuntu 22.04),
> this limits the CPU utilization of the system to under 50% when testing at
> full load and therefore limits the achieved throughput.
> Removing the lock contention from the SSTableReader.java file by replacing
> the call to `getCapacity` with `size` achieves up to 2.95x increase in
> throughput on r8g.24xlarge and 2x on r7i.24xlarge:
> |Instance type|Cass 4.1.3|Cass 4.1.3 patched|
> |r8g.24xlarge|168k ops|496k ops (2.95x)|
> |r7i.24xlarge|153k ops|304k ops (1.98x)|
>
> Instructions to reproduce:
> {code:java}
> ## Requirements for Ubuntu 22.04
> sudo apt install -y ant git openjdk-11-jdk
> ## Build and run
> CASSANDRA_USE_JDK11=true ant realclean && CASSANDRA_USE_JDK11=true ant jar &&
> CASSANDRA_USE_JDK11=true ant stress-build && rm -rf data && bin/cassandra -f
> -R
> # Run
> bin/cqlsh -e 'drop table if exists keyspace1.standard1;' && \
> bin/cqlsh -e 'drop keyspace if exists keyspace1;' && \
> bin/nodetool clearsnapshot --all && tools/bin/cassandra-stress write
> n=10000000 cl=ONE -rate threads=384 -node 127.0.0.1 -log file=cload.log
> -graph file=cload.html && \
> bin/nodetool compact keyspace1 && sleep 30s && \
> tools/bin/cassandra-stress mixed ratio\(write=10,read=90\) duration=10m
> cl=ONE -rate threads=406 -node localhost -log file=result.log -graph
> file=graph.html
> {code}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
