[GitHub] phoenix issue #308: Client-side hash aggregation

[geraldss]

Github user geraldss commented on the issue:

https://github.com/apache/phoenix/pull/308
  
Thanks @JamesRTaylor for the reviews and suggestions.


---

[GitHub] phoenix issue #308: Client-side hash aggregation

[JamesRTaylor]

Github user JamesRTaylor commented on the issue:

https://github.com/apache/phoenix/pull/308
  
+1. Nice work, @geraldss. @twdsilva - would you (or maybe someone else?) 
have some spare cycles to check this in assuming the test run comes back clean?


---

[GitHub] phoenix issue #308: Client-side hash aggregation

[geraldss]

Github user geraldss commented on the issue:

https://github.com/apache/phoenix/pull/308
  
@JamesRTaylor - Handled reverse sort; added explanatory comments about 
sorting; added tests for sorting and non-sorting, including EXPLAIN.

Pushed changes; uploaded new .patch file; resubmitted patch.


---

[GitHub] phoenix issue #308: Client-side hash aggregation

[geraldss]

Github user geraldss commented on the issue:

https://github.com/apache/phoenix/pull/308
  
@JamesRTaylor - Done. I switched the PR to 4.x-HBase-1.4, attached the 
.patch file, and issued "Submit Patch".


---

[GitHub] phoenix issue #308: Client-side hash aggregation

[geraldss]

Github user geraldss commented on the issue:

https://github.com/apache/phoenix/pull/308
  
@JamesRTaylor -- ok, I will submit a new PR. Can you comment on my last 
push and comments?


---

[GitHub] phoenix issue #308: Client-side hash aggregation

[JamesRTaylor]

Github user JamesRTaylor commented on the issue:

https://github.com/apache/phoenix/pull/308
  
Looks like the move of 5.x branch to become master messed up this PR. Can 
you please start a new pull request against the 4.x-HBase-1.4 branch (this is 
what was master before), @geraldss? FYI, the last step will be to get a test 
run with your patch in place to make sure there are no regressions. To do that, 
attach a .patch file to the JIRA (include the branch name in the name of the 
patch file) and click on the Submit Patch button. See 
http://phoenix.apache.org/contributing.html#Generate_a_patch for more info on 
that.


---

[GitHub] phoenix issue #308: Client-side hash aggregation

[geraldss]

Github user geraldss commented on the issue:

https://github.com/apache/phoenix/pull/308
  
Hi @JamesRTaylor, I pushed another change and replied with some comments. 
Please review, thanks.


---

[GitHub] phoenix issue #308: Client-side hash aggregation

[geraldss]

Github user geraldss commented on the issue:

https://github.com/apache/phoenix/pull/308
  
@JamesRTaylor - pushed another change per your feedback. Fingers crossed :) 
Thanks.


---

[GitHub] phoenix issue #308: Client-side hash aggregation

[geraldss]

Github user geraldss commented on the issue:

https://github.com/apache/phoenix/pull/308
  
@JamesRTaylor - I made the changes and they are ready for review. Thanks.


---

[GitHub] phoenix issue #308: Client-side hash aggregation

[solzy]

Github user solzy commented on the issue:

https://github.com/apache/phoenix/pull/308
  
*@JamesRTaylor * Thanks for your clarify.



   Yun Zhang
   Best regards!


2018-07-17 6:18 GMT+08:00 James Taylor :

> *@JamesRTaylor* commented on this pull request.
> --
>
> In phoenix-core/src/main/java/org/apache/phoenix/iterate/
> ClientHashAggregatingResultIterator.java
> :
>
> > +protected ImmutableBytesWritable getGroupingKey(Tuple tuple, 
ImmutableBytesWritable ptr) throws SQLException {
> +try {
> +ImmutableBytesWritable key = 
TupleUtil.getConcatenatedValue(tuple, groupByExpressions);
> +ptr.set(key.get(), key.getOffset(), key.getLength());
> +return ptr;
> +} catch (IOException e) {
> +throw new SQLException(e);
> +}
> +}
> +
> +// Copied from ClientGroupedAggregatingResultIterator
> +protected Tuple wrapKeyValueAsResult(KeyValue keyValue) {
> +return new MultiKeyValueTuple(Collections. 
singletonList(keyValue));
> +}
> +
> +private void populateHash() throws SQLException {
>
> @geraldss  - memory management is tracked by
> the GlobalMemoryManager. Operations that potentially use memory allocate
> (and eventually free) a set of MemoryChunk instances. You can see an
> example of this in GroupedAggregateRegionObserver (the runtime code for
> aggregation). If the memory used goes over a threshold 
(phoenix.query.maxGlobalMemoryPercentage
> and phoenix.query.maxTenantMemoryPercentage as the allowed percentage of
> Java heap across all queries that is allowed to be used), then the query
> will fail. Most typically, this mechanism is used on the server side as we
> don't typically use a lot of memory on the client-side (as we're mostly
> doing merge joins). One example where we use this on the client side is 
for
> our broadcast join implementation (see HashCacheClient) to track memory
> held onto for Hash Join caches.
>
> Classes you may want to look at (or perhaps you already have?):
> OrderedResultIterator and MappedByteBufferSortedQueue. Above a certain
> configurable threshold (phoenix.query.spoolThresholdBytes defaults to
> 20MB), we output results into memory mapped files. Have you tried
> decreasing that threshold?
>
> Couple of JIRAs you may want to take a look at: PHOENIX-2405 (unclear if
> this is still an issue) and PHOENIX-3289. Are you running into issues with
> too many memory mapped files?
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> , or 
mute
> the thread
> 

> .
>



---

[GitHub] phoenix issue #308: Client-side hash aggregation

[geraldss]

Github user geraldss commented on the issue:

https://github.com/apache/phoenix/pull/308
  
@joshelser Thanks for the review. I can make these changes now and push 
them. Or should I wait for any other reviewer?


---