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

Sylvain Lebresne commented on CASSANDRA-3303:
---------------------------------------------

On the patch, a few minor remarks:
* There is a weird UnImplNode on top of ReadCommand.java (that prevents 
compilation)
* I think we could replace isLongRead and trimLongRead by a maybeTrim() method 
that would be a noop by default. Basically trimLongRead requires that 
isLongRead() has been called (otherwise it throws an unsupported exception), so 
in those case I think it's easier to use if it's just one method. Same for the 
pair (isShortRead, generateShortRetry) actually.
* nit: There is some imports rewrite (java.io.* -> multiple imports) in SFRC. 
It's a big deal but that kind of thing makes diffs bigger than necessary so 
it's nice to take the habit to no do that.

Now I don't think this patch addresses CASSANDRA-3395 as is. For that, we would 
basically need to move the trimLongRead to SliceFromReadCommand directly (the 
bug of CASSANDRA-3395 is that normal SliceFromReadCommand can have more that 
requested columns because of reconciliation of the result of different nodes). 
The easiest way here is probably to move the trim code to a 
SliceFromReadCommand.maybeTrim() and to have that method uses some 
getRequestedCount() method. For SFRC, the latter method would return count, for 
RSFRC, it would return originalCount.

                
> Short reads protection results in returning more columns than asked for
> -----------------------------------------------------------------------
>
>                 Key: CASSANDRA-3303
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-3303
>             Project: Cassandra
>          Issue Type: Bug
>          Components: Core
>    Affects Versions: 1.0.0
>            Reporter: Sylvain Lebresne
>            Assignee: Byron Clark
>            Priority: Minor
>             Fix For: 1.0.2
>
>         Attachments: cassandra-3303-1.patch, cassandra-3303-2.patch, 
> cassandra-3303-3.patch, cassandra-3303-4.patch, long_read.sh
>
>
> When we detect a short read (in SP.fetchRows), we retry a new command created 
> by:
> {noformat}
> logger.debug("detected short read: expected {} columns, but only resolved {} 
> columns", sliceCommand.count, liveColumnsInRow);
> int retryCount = sliceCommand.count + sliceCommand.count - liveColumnsInRow;
> SliceFromReadCommand retryCommand = new SliceFromReadCommand(command.table,
>                                                              command.key,
>                                                              
> command.queryPath,
>                                                              
> sliceCommand.start,
>                                                              
> sliceCommand.finish,
>                                                              
> sliceCommand.reversed,
>                                                              retryCount);
> {noformat}
> That is, in that new command, the count is greater than what asked in the 
> initial command. But we never cut back the result of that new retried query.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Reply via email to