cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Pavel Yaskevich (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (CASSANDRA-10661) Integrate SASI to Cassandra
Date Wed, 06 Jan 2016 23:57:40 GMT

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

Pavel Yaskevich commented on CASSANDRA-10661:
---------------------------------------------

Thanks for the review!

This is pretty ironic that COMPACT STORAGE is broken now, but we are going to fit this anyway
once clustering support is added. :) dtests are definitely a good idea, I think we should
have some already and we'll definitely port them main repo, I will also add my branch to CI
tonight.

Regarding review comments (all of the changes are pushed): 

bq. The regex matching in o.a.c.io.sstable.Component.Type::fromRepresentation throws an NPE
Fixed.

bq. In SASIIndex, getMetadataReloadTask & getBlockingFlushTask should be able to just
return null
Made both return null in SASIndex and made sure there would be no NPEs in existing call sites.

bq. I couldn't see why a PeekingIterator is used in OnDiskIndex::search
Moved back to Iterator<ByteBuffer> that, since PeekingIterator was a rudiment of the
previous version if search.

bq. 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.
Renamed to "sasi", "internal". maybe it will make it a bit clearer...

bq. The anonymous extension of Expression in Operation::analyzeGroup can be replaced
Fixed, Thanks for catching that!

bq. MemIndex::estimateSize is unused
Is a TODO item for me as memory tracking is different in 3.0 (see my previous comment).

bq. 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
Makes sense and done.

bq. Most trivial of nits: brace placement in SchemaLoader (ln 255)
Fixed :)

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

Mime
View raw message