Mailing-List: contact commits-help@cassandra.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@cassandra.apache.org
Date: Thu, 22 Oct 2015 17:54:27 +0000 (UTC)
From: "Benedict (JIRA)" <jira@apache.org>
To: commits@cassandra.apache.org
Message-ID: <JIRA.12851320.1438670968000.31264.1445536467857@Atlassian.JIRA>
In-Reply-To: <JIRA.12851320.1438670968000@Atlassian.JIRA>
References: <JIRA.12851320.1438670968000@Atlassian.JIRA>
 <JIRA.12851320.1438670968845@arcas>
Subject: [jira] [Commented] (CASSANDRA-9975) Flatten Iterator call hierarchy
 with a shared Transformer
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit


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

Benedict commented on CASSANDRA-9975:
-------------------------------------

bq. Wondering if BasePartitions.hasNext/applyOne shouldn't take responsibility of closing next if applyToPartitions returned null since other aspects of the transformer do close automatically?

I have been back and forth on this one myself. I just started to do it again, and remembered why I chose not to: half of the places we close the iterator in a transformation still return a non-null result. So the help offered is inconsistent. It also (slightly) makes the main transformation loop uglier. I'm happy to do this, as I don't have a strong feeling on it, but both times I've looked at it I've ultimately reversed course.

bq. toQuery in checkForShortRead no longer has division-by-zero protection. Is it no longer necessary?

Line 345 has added a check for if we've received any live data in the most recent request, which captures this AFAICT. i.e., if we get zero rows first time, there is no point asking for more, since there aren't any.

I've rebased and split out into a separate package. I've also run eclipse-warnings, but all I see is "[java] incorrect classpath: /home/benedict/git/cassandra/build/cobertura/classes". [~tjake], is this expected / indicates we're otherwise good? I assume I'm meant to see a warning or two (else Branimir probably wouldn't have mentioned it)

CI looked goodish last run. AFAICT it was equivalent to mainline. I'll see if I can get some certainty before commit.

> Flatten Iterator call hierarchy with a shared Transformer
> ---------------------------------------------------------
>
>                 Key: CASSANDRA-9975
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-9975
>             Project: Cassandra
>          Issue Type: Sub-task
>          Components: Core
>            Reporter: Benedict
>            Assignee: Benedict
>             Fix For: 3.0.0
>
>
> Stepping through a read response is made exceedingly difficult by the sheer depth of the call hierarchy, and how rapidly your context jumps around. This ticket intend to partially address that, by flattening one of the main causes of this: iterator transformations.
> I have a patch that attempts to mitigate (but not entirely eliminate) this, through the introduction of a {{RowTransformer}} class that all transformations are applied through. If a transformation has already been applied, the {{RowTransformer}} class does not wrap a new iterator, but instead returns a new {{RowTransformer}} that wraps the original underlying (untransformed) iterator and both transformations. This can accumulate an arbitrary number of transformations and, quite importantly, can apply the filtration step {{Unfiltered -> Row}}  in the same instance as well. The intention being that a majority of control flow happens inside this {{RowTransformer}}, so there is far less context jumping to cope with.


--
This message was sent by Atlassian JIRA
(v6.3.4#6332)