[
https://issues.apache.org/jira/browse/CASSANDRA-16789?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17378026#comment-17378026
]
Paulo Motta commented on CASSANDRA-16789:
-----------------------------------------
Thanks for your patch update, looking good so far! Added a few comments to the
PR itself and below you can find some design comments:
I noticed the manifest file is being read in two places, which violates the
don't repeat yourself principle:
* [SnapshotDetails
constructor|https://github.com/apache/cassandra/pull/1046/files#diff-ad15ce2b3f9473fd99ec3f72362497ec43599c63b8f71b509e338677eeaca41aR40]
*
[SnapshotDetailsTabularData.from|https://github.com/apache/cassandra/pull/1046/files#diff-76d6ca8149019cb3e777d971489de8ba6c20d6caee2718f638ad044f7fddc3c9R87]
Another potential issue I spotted is that
{{Directories.getSnapshotManifestFile(snapshotName)}} is being used to find the
snapshot manifest location. As [~stefan.miklosovic] mentioned in an earlier
discussion, this method is only used to write snapshots, since it will pick one
of many data directories to write the manifest file. Using this same method for
reading can be a problem when using many data directories because a different
directory can be chosen from the one the manifest was written to, so the
manifest would not be found.
I think the best place to read the manifest is on
{{Directories.getSnapshotDetails}}, since this will already go through all the
data directories and you can try to find the manifest in all data directories.
We should probably merge the class {{SnapshotSizeDetails}} into
{{SnapshotDetails}} and use this single class to represent information about a
table snapshot, and this method would return {{SnapshotDetails}} instead of
{{SnapshotSizeDetails}}.
Please let me know if this makes sense to you or if you have any questions or
concerns.
> Add TTL support to nodetool snapshots
> -------------------------------------
>
> Key: CASSANDRA-16789
> URL: https://issues.apache.org/jira/browse/CASSANDRA-16789
> Project: Cassandra
> Issue Type: Sub-task
> Components: Tool/nodetool
> Reporter: Paulo Motta
> Assignee: Abuli Palagashvili
> Priority: Normal
>
> Add new parameter {{--ttl}} to {{nodetool snapshot}} command. This parameter
> can be specified in human readable duration (ie. 30mins, 1h, 300d) and should
> not be lower than 1 minute.
> The expiration date should be added to the snapshot manifest in ISO format.
> A periodic thread should efficiently scan snapshots and automatically clear
> those past expiration date. The periodicity of the scan thread should be 1
> minute by default but be overridable via a system property.
> The command {{nodetool listsnapshots}} should display the expiration date
> when the snapshot contains a TTL.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
