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