You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lens.apache.org by Lavkesh Lahngir <la...@gmail.com> on 2017/05/08 12:44:47 UTC

Review Request 59048: Add Memory gatekeepers for InMemoryResultSet to prevent any OOM on lens server

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59048/
-----------------------------------------------------------

Review request for lens and Puneet Gupta.


Bugs: lens-981
    https://issues.apache.org/jira/browse/lens-981


Repository: lens


Description
-------

See LENS-981


Diffs
-----

  lens-api/src/main/resources/lens-errors.conf 43de1e9 
  lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java 7fd487c 
  lens-server-api/src/main/java/org/apache/lens/server/api/driver/InMemoryResultSet.java 535065d 
  lens-server-api/src/main/java/org/apache/lens/server/api/driver/LensResultSet.java 805b0c1 
  lens-server-api/src/main/java/org/apache/lens/server/api/driver/PersistentResultSet.java 774f1ee 
  lens-server-api/src/main/java/org/apache/lens/server/api/error/LensServerAPIErrorCode.java PRE-CREATION 
  lens-server/src/main/java/org/apache/lens/server/query/QueryEndNotifier.java 2a34c68 
  lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java e70d290 
  lens-server/src/main/resources/lensserver-default.xml 28b1db2 


Diff: https://reviews.apache.org/r/59048/diff/1/


Testing
-------


Thanks,

Lavkesh Lahngir


Re: Review Request 59048: Add Memory gatekeepers for InMemoryResultSet to prevent any OOM on lens server

Posted by Puneet Gupta <pu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59048/#review175066
-----------------------------------------------------------




lens-server-api/src/main/java/org/apache/lens/server/api/driver/LensResultSet.java
Line 58 (original), 59 (patched)
<https://reviews.apache.org/r/59048/#comment248411>

    OK


- Puneet Gupta


On May 8, 2017, 12:44 p.m., Lavkesh Lahngir wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59048/
> -----------------------------------------------------------
> 
> (Updated May 8, 2017, 12:44 p.m.)
> 
> 
> Review request for lens and Puneet Gupta.
> 
> 
> Bugs: lens-981
>     https://issues.apache.org/jira/browse/lens-981
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> See LENS-981
> 
> 
> Diffs
> -----
> 
>   lens-api/src/main/resources/lens-errors.conf 43de1e9 
>   lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java 7fd487c 
>   lens-server-api/src/main/java/org/apache/lens/server/api/driver/InMemoryResultSet.java 535065d 
>   lens-server-api/src/main/java/org/apache/lens/server/api/driver/LensResultSet.java 805b0c1 
>   lens-server-api/src/main/java/org/apache/lens/server/api/driver/PersistentResultSet.java 774f1ee 
>   lens-server-api/src/main/java/org/apache/lens/server/api/error/LensServerAPIErrorCode.java PRE-CREATION 
>   lens-server/src/main/java/org/apache/lens/server/query/QueryEndNotifier.java 2a34c68 
>   lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java e70d290 
>   lens-server/src/main/resources/lensserver-default.xml 28b1db2 
> 
> 
> Diff: https://reviews.apache.org/r/59048/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Lavkesh Lahngir
> 
>


Re: Review Request 59048: Add Memory gatekeepers for InMemoryResultSet to prevent any OOM on lens server

Posted by Lavkesh Lahngir <la...@gmail.com>.

> On May 15, 2017, 6:40 a.m., Puneet Gupta wrote:
> > lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java
> > Lines 1054 (patched)
> > <https://reviews.apache.org/r/59048/diff/1/?file=1710363#file1710363line1054>
> >
> >     Can we add test case to check the max limit ?

Yes.Doing that.


> On May 15, 2017, 6:40 a.m., Puneet Gupta wrote:
> > lens-server-api/src/main/java/org/apache/lens/server/api/driver/LensResultSet.java
> > Line 58 (original), 59 (patched)
> > <https://reviews.apache.org/r/59048/diff/1/?file=1710365#file1710365line59>
> >
> >     Should we have this in InMemory result Set construtor instead or get it from a static method (since this is a server level config and should not be overridable via query or session params) ? Open to discussion ...

I think in that case we need to change every implementation. Also, by design this can't be overwritten by the query conf as it is a a server conf.


- Lavkesh


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59048/#review174926
-----------------------------------------------------------


On May 8, 2017, 12:44 p.m., Lavkesh Lahngir wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59048/
> -----------------------------------------------------------
> 
> (Updated May 8, 2017, 12:44 p.m.)
> 
> 
> Review request for lens and Puneet Gupta.
> 
> 
> Bugs: lens-981
>     https://issues.apache.org/jira/browse/lens-981
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> See LENS-981
> 
> 
> Diffs
> -----
> 
>   lens-api/src/main/resources/lens-errors.conf 43de1e9 
>   lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java 7fd487c 
>   lens-server-api/src/main/java/org/apache/lens/server/api/driver/InMemoryResultSet.java 535065d 
>   lens-server-api/src/main/java/org/apache/lens/server/api/driver/LensResultSet.java 805b0c1 
>   lens-server-api/src/main/java/org/apache/lens/server/api/driver/PersistentResultSet.java 774f1ee 
>   lens-server-api/src/main/java/org/apache/lens/server/api/error/LensServerAPIErrorCode.java PRE-CREATION 
>   lens-server/src/main/java/org/apache/lens/server/query/QueryEndNotifier.java 2a34c68 
>   lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java e70d290 
>   lens-server/src/main/resources/lensserver-default.xml 28b1db2 
> 
> 
> Diff: https://reviews.apache.org/r/59048/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Lavkesh Lahngir
> 
>


Re: Review Request 59048: Add Memory gatekeepers for InMemoryResultSet to prevent any OOM on lens server

Posted by Puneet Gupta <pu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59048/#review174926
-----------------------------------------------------------




lens-api/src/main/resources/lens-errors.conf
Lines 457 (patched)
<https://reviews.apache.org/r/59048/#comment248210>

    Can we change to 
    "Rows in the result set exceeded the maximum allowed number of rows %d"



lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java
Lines 1054 (patched)
<https://reviews.apache.org/r/59048/#comment248209>

    Can we add test case to check the max limit ?



lens-server-api/src/main/java/org/apache/lens/server/api/driver/LensResultSet.java
Line 58 (original), 59 (patched)
<https://reviews.apache.org/r/59048/#comment248211>

    Should we have this in InMemory result Set construtor instead or get it from a static method (since this is a server level config and should not be overridable via query or session params) ? Open to discussion ...



lens-server/src/main/resources/lensserver-default.xml
Lines 311 (patched)
<https://reviews.apache.org/r/59048/#comment248206>

    Can we add "Max" in the name of this property



lens-server/src/main/resources/lensserver-default.xml
Lines 313 (patched)
<https://reviews.apache.org/r/59048/#comment248208>

    Can we update to 
    
    "This property controls the maximum number of rows allowed for an InMemory result. This propertly helps in keeping a tab on memory foortprint of lens server. Persistent resultset is suggested for results with size greater than this value."


- Puneet Gupta


On May 8, 2017, 12:44 p.m., Lavkesh Lahngir wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59048/
> -----------------------------------------------------------
> 
> (Updated May 8, 2017, 12:44 p.m.)
> 
> 
> Review request for lens and Puneet Gupta.
> 
> 
> Bugs: lens-981
>     https://issues.apache.org/jira/browse/lens-981
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> See LENS-981
> 
> 
> Diffs
> -----
> 
>   lens-api/src/main/resources/lens-errors.conf 43de1e9 
>   lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java 7fd487c 
>   lens-server-api/src/main/java/org/apache/lens/server/api/driver/InMemoryResultSet.java 535065d 
>   lens-server-api/src/main/java/org/apache/lens/server/api/driver/LensResultSet.java 805b0c1 
>   lens-server-api/src/main/java/org/apache/lens/server/api/driver/PersistentResultSet.java 774f1ee 
>   lens-server-api/src/main/java/org/apache/lens/server/api/error/LensServerAPIErrorCode.java PRE-CREATION 
>   lens-server/src/main/java/org/apache/lens/server/query/QueryEndNotifier.java 2a34c68 
>   lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java e70d290 
>   lens-server/src/main/resources/lensserver-default.xml 28b1db2 
> 
> 
> Diff: https://reviews.apache.org/r/59048/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Lavkesh Lahngir
> 
>