[
https://issues.apache.org/jira/browse/CASSANDRA-18111?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17881819#comment-17881819
]
Paulo Motta commented on CASSANDRA-18111:
-----------------------------------------
Thanks for the update, the patch is looking good so far but I think there is
still some internal snapshot logic leaking to other classes (ie.
{{{}Keyspace{}}}/ColumnFamilyStore). It would be ideal if we could centralize
most if not all internal snapshot logic on the package
*org.apache.cassandra.service.snapshot* as part of this effort.
Added some review comments directly to the
[PR|https://github.com/apache/cassandra/pull/3374#pullrequestreview-2305171293]
and some other comments below.
I see some internal code/tests using old snapshot methods from
{{StorageService}} - (for example
[StandaloneUpgraderOnSStablesTest#testUpgradeSnapshot|https://github.com/apache/cassandra/blob/trunk/test/unit/org/apache/cassandra/tools/StandaloneUpgraderOnSStablesTest.java#L96]).
Should we deprecate {{StorageService}} snapshot methods to discourage its use
(similar to {{{}StorageServiceMBean{}}}) and update all uses to use
{{SnapshotManager}} methods ?
It looks like some internal code is still referring to {{ColumnFamilyStore}}
legacy snapshot verbs (ie.
[SnapshotVerbHandler.doVerb|https://github.com/apache/cassandra/blob/fe025c7f79e76d99e0db347518a7872fd4a114bc/src/java/org/apache/cassandra/service/SnapshotVerbHandler.java#L49])
- should we update all uses to use {{SnapshotManager}} and remove
{{ColumnFamilyStore}} snapshot methods in favor of {{SnapshotManager}} methods ?
It looks like there are some legacy snapshot tests without assertions on
[StorageServiceServerTest|https://github.com/apache/cassandra/blob/trunk/test/unit/org/apache/cassandra/service/StorageServiceServerTest.java#L158].
I think we should try to add the missing assertions and move them to
{{SnapshotManagerTest}} if they're not already being tested somewhere else.
> Centralize all snapshot operations to SnapshotManager and cache snapshots
> -------------------------------------------------------------------------
>
> Key: CASSANDRA-18111
> URL: https://issues.apache.org/jira/browse/CASSANDRA-18111
> Project: Cassandra
> Issue Type: Improvement
> Components: Local/Snapshots
> Reporter: Paulo Motta
> Assignee: Stefan Miklosovic
> Priority: Normal
> Fix For: 5.x
>
> Time Spent: 5h
> Remaining Estimate: 0h
>
> Everytime {{nodetool listsnapshots}} is called, all data directories are
> scanned to find snapshots, what is inefficient.
> For example, fetching the
> {{org.apache.cassandra.metrics:type=ColumnFamily,name=SnapshotsSize}} metric
> can take half a second (CASSANDRA-13338).
> This improvement will also allow snapshots to be efficiently queried via
> virtual tables (CASSANDRA-18102).
> In order to do this, we should:
> a) load all snapshots from disk during initialization
> b) keep a collection of snapshots on {{SnapshotManager}}
> c) update the snapshots collection anytime a new snapshot is taken or cleared
> d) detect when a snapshot is manually removed from disk.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
