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

Berenguer Blasi edited comment on CASSANDRA-16818 at 8/9/21, 9:24 AM:
----------------------------------------------------------------------

That schema loading check is noop imo. If you remove the {{if (not 
loadschema)}} 
[line|https://github.com/apache/cassandra/pull/1121/commits/f90301251dd3d761e18a9731025b364f7fc106f8#diff-71a75df34f06faf848378bdf1a20a7d54a3e282abfe24bb9def78d99b857081fR247]
 making the check trigger always the test will still pass!

And j11 seems to be loading that class at some point 'randomly'. So we can go 
down the rabbit hole of trying to pin down why/when j11 will load the class or 
just accept that's how it works and remove that check.

Also notice we have interleaved tests checking for the class to be loaded 
true/false, never clearing the loaded status in between, so once one test would 
have loaded it all later ones expecting it not to be loaded should fail. Which 
they don't bc of the above.

I would revert the schema class loading logic to what we had previously and 
then the test seems to pass for me locally.


was (Author: bereng):
That schema loading check is noop imo. If you remove the {{if (!loadschema)}} 
[line|https://github.com/apache/cassandra/pull/1121/commits/f90301251dd3d761e18a9731025b364f7fc106f8#diff-71a75df34f06faf848378bdf1a20a7d54a3e282abfe24bb9def78d99b857081fR247]
 making the check trigger always the test will still pass!

And j11 seems to be loading that class at some point 'randomly'. So we can go 
down the rabbit hole of trying to pin down why/when j11 will load the class or 
just accept that's how it works and remove that check.

Also notice we have interleaved tests checking for the class to be loaded 
true/false, never clearing the loaded status in between, so once one test would 
have loaded it all later ones expecting it not to be loaded should fail. Which 
they don't bc of the above.

I would revert the schema class loading logic to what we had previously and 
then the test seems to pass for me locally.

> Fix usage instructions about sstabledump -k and -x options
> ----------------------------------------------------------
>
>                 Key: CASSANDRA-16818
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-16818
>             Project: Cassandra
>          Issue Type: Bug
>          Components: Tool/sstable
>            Reporter: Andres de la Peña
>            Assignee: Andres de la Peña
>            Priority: Normal
>             Fix For: 4.0.1, 4.1
>
>          Time Spent: 20m
>  Remaining Estimate: 0h
>
> The options {{-k}} and {{-x}} of {{sstabledump}} admit multiple arguments, so 
> users can include or exclude multiple partitions. The intended usage is, for 
> example:
> {code:java}
> $ sstablepartitions <sstable_path> -k 1
> $ sstablepartitions <sstable_path> -k 1 2 3
> {code}
> However, the following command will fail:
> {code:java}
> $ sstablepartitions -k 1 <sstable_path>
> You must supply exactly one sstable
> usage: sstabledump <sstable file path> <options>
> Dump contents of given SSTable to standard output in JSON format.
>  -d         CQL row per line internal representation
>  -e         enumerate partition keys only
>  -k <arg>   Partition key
>  -t         Print raw timestamps instead of iso8601 date strings
>  -x <arg>   Excluded partition key
> {code}
> This command fails because the sstable path is considered a fourth partition 
> key, and the mandatory argument for the sstable path is missed. While I think 
> this behaviour is correct, it can be a bit confusing for users, especially 
> when the information about usage describes both {{-k}} and {{-x}} as 
> single-argument.
> I think that at least we should fix the description of the options to 
> indicate that there could be multiple included/excluded keys, and probably 
> improve the message about the missing sstable path when those options are 
> used.
> Alternatively we could modify the options to have a single argument and allow 
> to repeat them, so we could accept things like:
> {code:java}
> $ sstablepartitions -k 1 <sstable_path>
> $ sstablepartitions -k 1 -k 2 -k 3 <sstable_path>
> $ sstablepartitions <sstable_path> -k 1 -k 2 -k 3
> {code}
> The main downside of the latter approach is that the change in the syntax of 
> the command might cause compatibility issues. Also we would need to upgrade 
> {{commons-cli}} to at least 1.2 due to CLI-137.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

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

Reply via email to