[jira] [Commented] (CASSANDRA-10661) Integrate SASI to Cassandra

["Sam Tunnicliffe (JIRA)" <ji...@apache.org>]

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

Sam Tunnicliffe commented on CASSANDRA-10661:
---------------------------------------------

This is looking pretty good. 

A problem (which isn't caught by any of the unit tests btw) is that due to the fact that under the hood 3.x considers all compact storage columns as static. This breaks interactions with sasi-indexed tables via CQL - for example, try running through the examples in the original [SASI readme|https://github.com/xedin/sasi/blob/master/README.md] and you'll find querying mostly broken. 

{code}
cqlsh:demo> select first_name, last_name, age, height, created_at from sasi where first_name = 'M';
InvalidRequest: code=2200 [Invalid query] message="Queries using 2ndary indexes don't support selecting only static columns"
cqlsh:demo>
cqlsh:demo>
cqlsh:demo> select * from sasi where first_name = 'M';

 id | age | created_at | first_name | height | last_name
----+-----+------------+------------+--------+-----------

(0 rows)
{code}

Fortunately, I believe we can simply drop the use of COMPACT STORAGE. My (limited) testing suggests that when tables are created without it, everything that's currently implemented works as expected.

The new SASI specific tests look good and are all green, but we obviously need to run this through CI before it's committed. On a related note, are there any dtests that may be worth adding? The utest coverage is pretty comprehensive (modulo the CQL issues) so I wouldn't say it was absolutely critical, but some multi-node & CQL based tests would be nice to have.

Otherwise, this first phase of integration looks good to me. On initial review I found one bug and a handful of nits. I have a few scenarios I want to run through, mostly to verify how sasi interacts with some of the parts of the index subsystem that were changed in 3.0.

Initial review comments:

* The regex matching in o.a.c.io.sstable.Component.Type::fromRepresentation throws an NPE when it encounters an unknown name and tries to match it to a CUSTOM component.
* In SASIIndex, getMetadataReloadTask & getBlockingFlushTask should be able to just return null (like getInitializationTask does). In the case of the former, that is true right now as the only call site is in SIM where nulls are properly handled. getBlockingFlushTask is also called from KeyCacheCqlTest which doesn't check for nulls so would need tweaking slightly. (This is totally minor, the irregularity in SASIIndex just bugged me).
* I couldn't see why a PeekingIterator is used in OnDiskIndex::search
* The use of "a" and "b" in the o.a.c.i.sasi.plan.Expression ctor seems like it could have the potential for pain when debugging. I'm sure that it isn't very likely we'll ever care too much & I don't have any particularly better suggestion but if you do, could these be changed to something more greppable (or extracted to constants)?
* The anonymous extension of Expression in Operation::analyzeGroup can be replaced with {{perColumn.add(new Expression(controller, columnIndex).add(e.operator(), token));}}
* MemIndex::estimateSize is unused
* It doesn't really affect anything, but just for clarity I would rename MemoryUtil.DIRECT_BYTE_BUFFER_R_CLASS to RO_DIRECT_BYTE_BUFFER_CLASS
* Most trivial of nits: brace placement in SchemaLoader (ln 255)


> Integrate SASI to Cassandra
> ---------------------------
>
>                 Key: CASSANDRA-10661
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-10661
>             Project: Cassandra
>          Issue Type: Improvement
>          Components: Local Write-Read Paths
>            Reporter: Pavel Yaskevich
>            Assignee: Pavel Yaskevich
>              Labels: sasi
>             Fix For: 3.x
>
>
> We have recently released new secondary index engine (https://github.com/xedin/sasi) build using SecondaryIndex API, there are still couple of things to work out regarding 3.x since it's currently targeted on 2.0 released. I want to make this an umbrella issue to all of the things related to integration of SASI, which are also tracked in [sasi_issues|https://github.com/xedin/sasi/issues], into mainline Cassandra 3.x release.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)