Github user ChengXiangLi commented on the pull request:
https://github.com/apache/flink/pull/949#issuecomment-132864761
Thanks for the detail explanation, @tillrohrmann . As the owner of this
issue, make it work correctly and efficiently is my top desire, so i would
never say no to a reasonable and proper suggestion(although i did expect it
happens earlier :-)). `OutOfMemoryError` is one of my concern during the
implementation as well, unlimited usage of Java heap is unacceptable, so i
limit the reservoir size(`PriorityQueue` size in implementation) same as
`numSamples` parameter, which give the user full control of the memory used in
java heap for `sample` operator. It may lead to `OutOfMemoryError` if the
`numSamples` is too large, but other operators may lead to `OutOfMemoryError`
as well if user cache too much objects inside UDF. Anyway, it's just my first
thought, a fully managed memory usage is a better solution, and i would like to
implement that.
As mentioned in the previous comment, is this the next step you prefer?
1. move the `sample` and the `sampleWithSize` methods from the `DataSet` to
the `DataSetUtils` class, and merge this PR.
2. implement `topK` operator in a separate PR.
3. update `sampleWithSize` implementation, and move back to `DataSet`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
