[
https://issues.apache.org/jira/browse/CASSANDRA-6719?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16433839#comment-16433839
]
Marcus Eriksson commented on CASSANDRA-6719:
--------------------------------------------
pushed a commit with the comments addressed
[here|https://github.com/krummas/cassandra/commits/marcuse/6719]
bq. FSUtils.handleCorruptSSTable/handleFSError are no longer called
this is on purpose and we should probably fix this in 3.0+ as well - I don't
think we want to trigger the disk failure policy if import fails - instead we
should abort the import. If someone has configured {{disk_failure_policy:
die_paranoid}} trying to load a corrupt file would actually stop the node
bq. Row cache invalidation was not previously performed — this is a good thing
regardless, so maybe skip an option for this one.
added an option to explicitly skip the row cache invalidation - also added a
{{--quick}} option which makes it behave more like the old version
bq. If using nodetool refresh with JBOD, the counting keys per boundary work is
done just to throw it away.
added a check if there is only a single data directory (and row cache is not
enabled)
bq. Minor/naming nit: consider renaming CFS#loadSSTables’s dirPath -> srcPath
and findBestDiskAndInvalidCache’s path -> srcPath
fixed
bq. Minor/usability nit: I couldn’t find many cases where
@Option(required=true) is used. WDYT about moving the path to a positional
argument since its required and this command does not take a variable number of
positional args?
makes sense, made it {{nodetool import <ks> <cf> <dir>}}
bq. Minor/usability nit: Instead of noVerify=true,noVerifyTokens=false being an
invalid state, make noVerify=true imply noVerifyTokens=true.
yup, makes sense
bq. The JavaDoc for CFS.loadNewSSTables should be updated to point to the new
StorageService.loadSSTables.
bq. The comment on CFS#L861 is useful but out of place.
bq. Minor/naming nit: The naming of the “allKeys” variable in
ImportTest#testImportInvalidateCache is misleading.
bq. Instead of hardcoding token values what about using e.g.
t.compareTo(mock.getDiskBoundaries().positions.get(0).getToken()) <= 0?
bq. Are you intentionally leaving the Random seed hardcoded?
fixed
> redesign loadnewsstables
> ------------------------
>
> Key: CASSANDRA-6719
> URL: https://issues.apache.org/jira/browse/CASSANDRA-6719
> Project: Cassandra
> Issue Type: New Feature
> Components: Tools
> Reporter: Jonathan Ellis
> Assignee: Marcus Eriksson
> Priority: Minor
> Labels: lhf
> Fix For: 4.x
>
> Attachments: 6719.patch
>
>
> CFSMBean.loadNewSSTables scans data directories for new sstables dropped
> there by an external agent. This is dangerous because of possible filename
> conflicts with existing or newly generated sstables.
> Instead, we should support leaving the new sstables in a separate directory
> (specified by a parameter, or configured as a new location in yaml) and take
> care of renaming as necessary automagically.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
