[ 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)