[
https://issues.apache.org/jira/browse/CASSANDRA-9705?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14635504#comment-14635504
]
Aleksey Yeschenko commented on CASSANDRA-9705:
----------------------------------------------
Fill disclosure: the patch set is very sizable, so this is not a complete
thorough review - merely an overeview answering the question 'can we merge this
into trunk as is, and finish the review afterwards?'.
Broken:
1. {{ClusteringComponent.compareComponent()}} is broken for v1 == v2 == null
(the first ternary is wrong). We need tests for this.
Looks fishy:
2. {{RangeTombstoneBoundaryMarker}} {{createCorrespondingCloseMarker()}} and
{{createCorrespondingOpenMarker()}} have swapped their behavior in this patch
set. I'm assuming that this was intentional, but if so, it at least warrants a
comment
Unused fields and method arguments that are confusing (some not new to this
patch set). Most of these, if not all, are benign, but some wasted some time to
figure out:
3. {{UnfilteredDeserializer.OldFormatDeserializer.nextFlags}} is unused
4. {{MemtableAllocator.SubAllocator}} fields {{owns}} and {{reclaiming}} are
unused
5. {{BitTableWriter.StatsCollector.collectStatsOn()}} is unused (but turns out
to be benign)
6. {{SerializationHelper.endOfComplexColumn()}} method has an unused {{column}}
argument
Nits:
7. {{PartitionUpdate.CounterMark}} class should be static
Nomenclature is arguable, and there is plenty of minor nits that I won't list
here (but would urge Sylvain to go through each file in IDEA and pay attention
to the inspections).
Once cassci is happy (which is to say, once it shows that these changes don't
break anything new), I'm fine with committing it to trunk. Overall it's a
welcome simplification that should fix and make a lot of things easier
(including Tyler's backward compat work and Sam's 2i work).
> Simplify some of 8099's concrete implementations
> ------------------------------------------------
>
> Key: CASSANDRA-9705
> URL: https://issues.apache.org/jira/browse/CASSANDRA-9705
> Project: Cassandra
> Issue Type: Sub-task
> Reporter: Sylvain Lebresne
> Assignee: Sylvain Lebresne
> Fix For: 3.0 beta 1
>
>
> As mentioned in the ticket comments, some of the concrete implementations
> (for Cell, Row, Clustering, PartitionUpdate, ...) of the initial patch for
> CASSANDRA-8099 are more complex than they should be (the use of flyweight is
> typically probably ill-fitted), which probably has performance consequence.
> This ticket is to track the refactoring/simplifying those implementation
> (mainly by removing the use of flyweights and simplifying accordingly).
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
