cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Piotr Kołaczkowski (JIRA) <j...@apache.org>
Subject [jira] [Commented] (CASSANDRA-6311) Add CqlRecordReader to take advantage of native CQL pagination
Date Wed, 05 Mar 2014 12:19:43 GMT

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

Piotr Kołaczkowski commented on CASSANDRA-6311:
-----------------------------------------------

org/apache/cassandra/hadoop/cql3/CqlConfigHelper.java:275
{noformat}
Optional<SSLOptions> ssLOptions = getSSLOptions(conf);
{noformat}
typo: ssL -> ssl
--------------------------------------------------------------
org/apache/cassandra/hadoop/cql3/CqlConfigHelper.java:398:
{noformat}
Optional<Integer> maxSimultaneousRequests = getInputMinSimultReqPerConnections(conf);
Optional<Integer> minSimultaneousRequests = getInputMaxSimultReqPerConnections(conf);
{noformat}
min and max swapped?       
--------------------------------------------------------------
org/apache/cassandra/hadoop/cql3/CqlConfigHelper.java:549:
{noformat}
Optional<String> keystorePassword = getInputNativeSSLTruststorePassword(conf);
{noformat}
should be:
{noformat}
Optional<String> keystorePassword = getInputNativeSSLKeystorePassword(conf);
{noformat}
--------------------------------------------------------------
org/apache/cassandra/hadoop/cql3/CqlConfigHelper.java:524:
{noformat}
              return new AbstractIterator<Host>()
                        {
                            protected Host computeNext()
                            {
                                return origHost;
                            }  
                        };
{noformat}
Not sure if it was the intent to create an infinite iterator returning nulls or the same host
over and over again here. According to the docs, guava iterator implementations *must* invoke
endOfData() to terminate iteration. Don't we need here an iterator returning just one item
stickHost and let the driver handle the rest?

Also, not sure if returning nulls here is allowed at all (the driver docs isn't explicit on
that).
I guess very likely it is going to NPE if there is a connection problem which might cause
confusion. Probably a better solution would be to just return stickHost and let the driver
attempt connecting and throwing a meaningful error message upon failure.

BTW the implementation of the LoadBalancingPolicy, having two fields origHost and stickHost
is redundant and using null on one of those for marking the host is down / unreachable does
not convey the intent clearly to me. Can't we just use stickHost and a direct boolean flag
for denoting whether it is reachable or not?

--------------------------------------------------------------
org/apache/cassandra/hadoop/cql3/CqlConfigHelper.java:591:
{noformat}
    private static Optional<String> getStringSetting(String parameter, Configuration
conf)
    {
        String setting = conf.get(parameter);
        if (setting == null || setting.isEmpty())
            return Optional.absent();
        return Optional.of(setting);  
    }
{noformat}
In getStringSetting, setting an empty string is considered an absent option - so it is not
possible to have an empty string setting (not sure if it would be useful - just double checking
if it was on purpose or by omission)
--------------------------------------------------------------
{noformat}
     *  2) where clause must include token(partition_key1 ... partition_keyn) > ? and 
     *     token(partition_key1 ... partition_keyn) <= ?
{noformat}

Would be nice to have at least some basic validation of the WHERE clause, so the user gets
a nice error message when one screws it up. 
--------------------------------------------------------------
org/apache/cassandra/hadoop/cql3/CqlRecordReader.java:230
{noformat}
       public RowIterator(Configuration conf)
{noformat}
conf not used
--------------------------------------------------------------
org/apache/cassandra/hadoop/cql3/CqlRecordReader.java:268
{noformat}
            return Pair.create(Long.valueOf(keyId), row);
{noformat}
Boxing is not needed here.




> Add CqlRecordReader to take advantage of native CQL pagination
> --------------------------------------------------------------
>
>                 Key: CASSANDRA-6311
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-6311
>             Project: Cassandra
>          Issue Type: New Feature
>          Components: Hadoop
>            Reporter: Alex Liu
>            Assignee: Alex Liu
>             Fix For: 2.0.6
>
>         Attachments: 6311-v3-2.0-branch.txt, 6311-v4.txt, 6311-v5-2.0-branch.txt, 6331-2.0-branch.txt,
6331-v2-2.0-branch.txt
>
>
> Since the latest Cql pagination is done and it should be more efficient, so we need update
CqlPagingRecordReader to use it instead of the custom thrift paging.



--
This message was sent by Atlassian JIRA
(v6.2#6252)

Mime
View raw message