[
https://issues.apache.org/jira/browse/PHOENIX-1779?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14497559#comment-14497559
]
James Taylor commented on PHOENIX-1779:
---------------------------------------
Thanks for the revisions, [~samarthjain]. It's looking very good. The tests are
fine, but I think the code can be tightened up a bit for the
RoundRobinResultIterator:
- add Tuple member variable to RoundRobinIteratorState and get rid of
LinkedList<Tuple>.
- return RoundRobinIteratorState for currentIterator() and
List<RoundRobinIteratorState> for fetchNextBatch().
- in next(), when currentIterator() returns, check state.getTuple() != null and
return the Tuple if not null. Otherwise return state.iterator.next(). In either
case, you increment state.recordsRead (seems like you have the potential for an
off by one error, since you've read one row already with the initial next call).
- when is this else case ever executed in getIterators(), as you initialize
this in the constructor? Seems like you just need the if statement:
{code}
+ private List<RoundRobinIteratorState> getIterators() throws SQLException {
+ if (closed) { return Collections.emptyList(); }
+ if (openIterators.size() > 0 && openIterators.size() ==
numScannersCacheExhausted) {
+ /*
+ * All the scanners have exhausted their cache. Submit the
scanners back to the pool so that they can fetch
+ * the next batch of records in parallel.
+ */
+ initOpenIterators(fetchNextBatch());
+ } else if (openIterators.size() == 0 && resultIterators != null) {
+ List<PeekingResultIterator> iterators =
resultIterators.getIterators();
+ initOpenIterators(iterators);
+ }
+ return openIterators;
+ }
+
{code}
- I feel like the currentIterator() logic could be simplified a bit. For
example, you don't need the else { break; } because the loop will terminate in
those cases (as size == 0). Also, the first else isn't needed, as you return
from the if. You can likely return a RoundRobinIteratorState.EMPTY constant if
there are no more rows.
> Parallelize fetching of next batch of records for scans corresponding to
> queries with no order by
> --------------------------------------------------------------------------------------------------
>
> Key: PHOENIX-1779
> URL: https://issues.apache.org/jira/browse/PHOENIX-1779
> Project: Phoenix
> Issue Type: Improvement
> Reporter: Samarth Jain
> Assignee: Samarth Jain
> Attachments: PHOENIX-1779.patch, PHOENIX-1779_v2.patch, wip.patch,
> wip3.patch, wipwithsplits.patch
>
>
> Today in Phoenix we parallelize the first execution of scans i.e. we load
> only the first batch of records up to the scan's cache size in parallel.
> Loading of subsequent batches of records in scanners is essentially serial.
> This could be improved especially for queries, including the ones with no
> order by clauses, that do not need any kind of merge sort on the client.
> This could also potentially improve the performance of UPSERT SELECT
> statements that load data from one table and insert into another. One such
> use case being creating immutable indexes for tables that already have data.
> It could also potentially improve the performance of our MapReduce solution
> for bulk loading data by improving the speed of the loading/mapping phase.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
