[ 
https://issues.apache.org/jira/browse/CASSANDRA-3143?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13169365#comment-13169365
 ] 

Pavel Yaskevich edited comment on CASSANDRA-3143 at 12/14/11 2:19 PM:
----------------------------------------------------------------------

bq. I don't think CacheService.reloadKeyCache works correctly. It only populate 
the cache with fake values that won't get updated unless you reload the 
sstables, which has no reason to happen. I'm fine removing the key cache 
reloading altogether, but as an alternative, why not save the value of the key 
cache too? The thing is, I'm not very comfortable with the current 'two phase' 
key cache loading: if we ever have a bug in the SSTReader.load method, the 
actual pre-loading with -1 values will be harmful, which seems unnecessarily 
fragile. Saving the values on disk would avoid that.

Agreed, I just thought that it's valuable to have that key cache reload around, 
I'm going to remove it. But - save values to avoid 'two phrase' key cache 
loading - would require to use a common interface for values in key/row caches 
with serialize/deserialize functionality which is not suitable e.g. for 
ColumnFamily that we store in row cache... That is why we still rely on 
SSTableReader.load I think, saving values would limit flexibility of the cache 
interface...
 
bq. In CFS, it's probably faster/simpler to use metadata.cfId rather than 
Schema.instance.getId(table.name, this.columnFamily)

This would mean that we will be caching even secondary index CFs which is, as 
was said, is not desired. 

bq. In CacheService, calling scheduleSaving with -1 as second argument would be 
slightly faster than using Integer.MAX_VALUE.

This could have even worse performance because it will change semantics and 
call hotKeySet method on the ICache, for CLHM this is _not_ O(1) operation as 
doc for "descendingKeySetWithLimit(int limit);" says.

bq. I wonder if the reduce cache capacity thing still makes sense after this 
patch?

I think it does because it also helps to reclaim some memory when system is 
starving.


                
      was (Author: xedin):
    bq. I don't think CacheService.reloadKeyCache works correctly. It only 
populate the cache with fake values that won't get updated unless you reload 
the sstables, which has no reason to happen. I'm fine removing the key cache 
reloading altogether, but as an alternative, why not save the value of the key 
cache too? The thing is, I'm not very comfortable with the current 'two phase' 
key cache loading: if we ever have a bug in the SSTReader.load method, the 
actual pre-loading with -1 values will be harmful, which seems unnecessarily 
fragile. Saving the values on disk would avoid that.

I agree, it just thought that it's valuable to have that reload around. I'm 
going to remove key cache reload and save values to avoid 'two phrase' key 
cache loading.
 
bq. In CFS, it's probably faster/simpler to use metadata.cfId rather than 
Schema.instance.getId(table.name, this.columnFamily)

This would mean that we will be caching even secondary index CFs which is, as 
was said, is not desired. 

bq. In CacheService, calling scheduleSaving with -1 as second argument would be 
slightly faster than using Integer.MAX_VALUE.

This could have even worse performance because it will change semantics and 
call hotKeySet method on the ICache, for CLHM this is _not_ O(1) operation as 
doc for "descendingKeySetWithLimit(int limit);" says.

bq. I wonder if the reduce cache capacity thing still makes sense after this 
patch?

I think it does because it also helps to reclaim some memory when system is 
starving.


                  
> Global caches (key/row)
> -----------------------
>
>                 Key: CASSANDRA-3143
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-3143
>             Project: Cassandra
>          Issue Type: Improvement
>            Reporter: Pavel Yaskevich
>            Assignee: Pavel Yaskevich
>            Priority: Minor
>              Labels: Core
>             Fix For: 1.1
>
>         Attachments: 0001-global-key-cache.patch, 
> 0002-global-row-cache-and-ASC.readSaved-changed-to-abstra.patch, 
> 0003-CacheServiceMBean-and-correct-key-cache-loading.patch, 
> 0004-key-row-cache-tests-and-tweaks.patch, 
> 0005-cleanup-of-the-CFMetaData-and-thrift-avro-CfDef-and-.patch, 
> 0006-row-key-cache-improvements-according-to-Sylvain-s-co.patch
>
>
> Caches are difficult to configure well as ColumnFamilies are added, similar 
> to how memtables were difficult pre-CASSANDRA-2006.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Reply via email to