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