cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sam Tunnicliffe (JIRA)" <>
Subject [jira] [Commented] (CASSANDRA-10661) Integrate SASI to Cassandra
Date Wed, 06 Jan 2016 18:02:40 GMT


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|] and you'll find querying
mostly broken. 

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> select * from sasi where first_name = 'M';

 id | age | created_at | first_name | height | last_name

(0 rows)

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 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
* Most trivial of nits: brace placement in SchemaLoader (ln 255)

> Integrate SASI to Cassandra
> ---------------------------
>                 Key: CASSANDRA-10661
>                 URL:
>             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 (
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|],
into mainline Cassandra 3.x release.

This message was sent by Atlassian JIRA

View raw message