[jira] [Commented] (CASSANDRA-14417) nodetool import cleanup/fixes

2018-05-16 Thread Jordan West (JIRA) 

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

Jordan West commented on CASSANDRA-14417:
-

+1

> nodetool import cleanup/fixes
> -
>
> Key: CASSANDRA-14417
> URL: https://issues.apache.org/jira/browse/CASSANDRA-14417
> Project: Cassandra
>  Issue Type: Improvement
>Reporter: Marcus Eriksson
>Assignee: Marcus Eriksson
>Priority: Major
> Fix For: 4.x
>
>
> * We shouldn't expose importNewSSTables in both StorageServiceMBean and 
> CFSMbean
> * Allow a quicker token check without doing an extended verify
> * Introduce an ImportOptions class to avoid passing in 100 booleans in 
> importNewSSTables



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org

[jira] [Commented] (CASSANDRA-14417) nodetool import cleanup/fixes

2018-05-16 Thread Marcus Eriksson (JIRA) 

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

Marcus Eriksson commented on CASSANDRA-14417:
-

yeah, unintended, fixed in a new commit

> nodetool import cleanup/fixes
> -
>
> Key: CASSANDRA-14417
> URL: https://issues.apache.org/jira/browse/CASSANDRA-14417
> Project: Cassandra
>  Issue Type: Improvement
>Reporter: Marcus Eriksson
>Assignee: Marcus Eriksson
>Priority: Major
> Fix For: 4.x
>
>
> * We shouldn't expose importNewSSTables in both StorageServiceMBean and 
> CFSMbean
> * Allow a quicker token check without doing an extended verify
> * Introduce an ImportOptions class to avoid passing in 100 booleans in 
> importNewSSTables



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org

[jira] [Commented] (CASSANDRA-14417) nodetool import cleanup/fixes

2018-05-16 Thread Jordan West (JIRA) 

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

Jordan West commented on CASSANDRA-14417:
-

Looks like the most recent changes moved the second {{if (shouldCountKeys)}} 
into the upper while loop, which I don't think was intended.

branch: 
[https://github.com/krummas/cassandra/blob/f207720a45c9106cfbdd4e8ab8f34283c58cba52/src/java/org/apache/cassandra/db/ColumnFamilyStore.java#L741-L756]
 

vs. 

Trunk: 
[https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/db/ColumnFamilyStore.java#L741-L757]

> nodetool import cleanup/fixes
> -
>
> Key: CASSANDRA-14417
> URL: https://issues.apache.org/jira/browse/CASSANDRA-14417
> Project: Cassandra
>  Issue Type: Improvement
>Reporter: Marcus Eriksson
>Assignee: Marcus Eriksson
>Priority: Major
> Fix For: 4.x
>
>
> * We shouldn't expose importNewSSTables in both StorageServiceMBean and 
> CFSMbean
> * Allow a quicker token check without doing an extended verify
> * Introduce an ImportOptions class to avoid passing in 100 booleans in 
> importNewSSTables



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org

[jira] [Commented] (CASSANDRA-14417) nodetool import cleanup/fixes

2018-05-16 Thread Marcus Eriksson (JIRA) 

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

Marcus Eriksson commented on CASSANDRA-14417:
-

bq. While cleaning things up, should 
ColumnFamilyStore#findBestDiskAndInvalidateCaches be refactored to use 
KeyIterator as well?
yep! nice catch
bq. : Also, just noticed the failing dtest. 
yeah I should have a look, but I think it is unrelated to this

pushed a new commit addressing the comments

> nodetool import cleanup/fixes
> -
>
> Key: CASSANDRA-14417
> URL: https://issues.apache.org/jira/browse/CASSANDRA-14417
> Project: Cassandra
>  Issue Type: Improvement
>Reporter: Marcus Eriksson
>Assignee: Marcus Eriksson
>Priority: Major
> Fix For: 4.x
>
>
> * We shouldn't expose importNewSSTables in both StorageServiceMBean and 
> CFSMbean
> * Allow a quicker token check without doing an extended verify
> * Introduce an ImportOptions class to avoid passing in 100 booleans in 
> importNewSSTables



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org

[jira] [Commented] (CASSANDRA-14417) nodetool import cleanup/fixes

2018-05-15 Thread Jordan West (JIRA) 

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

Jordan West commented on CASSANDRA-14417:
-

* The output when {{extended=false}}, {{checkOwnsTokens=true}} is still debug 
(because the message was removed and the exception is logged as debug). I see 
it changed when extended=true. Verifier#L217.
 * While cleaning things up, should 
{{ColumnFamilyStore#findBestDiskAndInvalidateCaches}} be refactored to use 
{{KeyIterator}} as well?

> nodetool import cleanup/fixes
> -
>
> Key: CASSANDRA-14417
> URL: https://issues.apache.org/jira/browse/CASSANDRA-14417
> Project: Cassandra
>  Issue Type: Improvement
>Reporter: Marcus Eriksson
>Assignee: Marcus Eriksson
>Priority: Major
> Fix For: 4.x
>
>
> * We shouldn't expose importNewSSTables in both StorageServiceMBean and 
> CFSMbean
> * Allow a quicker token check without doing an extended verify
> * Introduce an ImportOptions class to avoid passing in 100 booleans in 
> importNewSSTables



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org

[jira] [Commented] (CASSANDRA-14417) nodetool import cleanup/fixes

2018-05-15 Thread Marcus Eriksson (JIRA) 

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

Marcus Eriksson commented on CASSANDRA-14417:
-

pushed a new commit addressing the comments: 
https://github.com/krummas/cassandra/commits/marcuse/14417
bq. In Verifier, is it more appropriate to favor OutputHandler#output over 
OutputHandler#debug for the error message when a key is out of range?
yeah, made it {{OutputHandler#warn}} 
bq. Would like to see some tests (including base/empty case, edge cases like 
wrap around) for {{RangeOwnHelper}}
added, also made it clearer that ROH can't continue to be used after finding an 
out-of-range token by making it throw exceptions instead of returning true/false

Also refactored {{Verifier}} to use a {{KeyIterator}} instead of manually 
deserializing the index when checking keys.

> nodetool import cleanup/fixes
> -
>
> Key: CASSANDRA-14417
> URL: https://issues.apache.org/jira/browse/CASSANDRA-14417
> Project: Cassandra
>  Issue Type: Improvement
>Reporter: Marcus Eriksson
>Assignee: Marcus Eriksson
>Priority: Major
> Fix For: 4.x
>
>
> * We shouldn't expose importNewSSTables in both StorageServiceMBean and 
> CFSMbean
> * Allow a quicker token check without doing an extended verify
> * Introduce an ImportOptions class to avoid passing in 100 booleans in 
> importNewSSTables



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org

[jira] [Commented] (CASSANDRA-14417) nodetool import cleanup/fixes

2018-05-14 Thread Jordan West (JIRA) 

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

Jordan West commented on CASSANDRA-14417:
-

All minor comments:
 * Deprecated {{CFS#loadNewSSTables()}} no longer needs to be synchronized. It 
just constructs an {{ImportOptions}} instance and passes it to the synchronized 
{{loadNewSSTables(ImportOptions).}}
 * Add a reference (e.g. @see) to {{CFSMBean.importNewSSTables}} from 
{{SSMBean.loadNewSSTables}}
 * In Verifier, is it more appropriate to favor {{OutputHandler#output}} over 
{{OutputHandler#debug}} for the error message when a key is out of range?
 * Would like to see some tests (including base/empty case, edge cases like 
wrap around) for {{RangeOwnHelper}}
 * {{nodetool refresh}}: Is the removal of the deprecation output intentional?

> nodetool import cleanup/fixes
> -
>
> Key: CASSANDRA-14417
> URL: https://issues.apache.org/jira/browse/CASSANDRA-14417
> Project: Cassandra
>  Issue Type: Improvement
>Reporter: Marcus Eriksson
>Assignee: Marcus Eriksson
>Priority: Major
> Fix For: 4.x
>
>
> * We shouldn't expose importNewSSTables in both StorageServiceMBean and 
> CFSMbean
> * Allow a quicker token check without doing an extended verify
> * Introduce an ImportOptions class to avoid passing in 100 booleans in 
> importNewSSTables



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org