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




lens-cube/src/test/java/org/apache/lens/driver/cube/TestWeightedCostSelector.java
 (lines 28 - 34)
<https://reviews.apache.org/r/47777/#comment199664>

    It's not using any of `lens-cube` dependencies, so this test class can move 
to `lens-server-api` module. Same should be possible with 
`TestMinCostSelector`. Let's move both there.



lens-cube/src/test/java/org/apache/lens/driver/cube/TestWeightedCostSelector.java
 (line 66)
<https://reviews.apache.org/r/47777/#comment199669>

    This method is performing 2 tests sequentially. The problem with that is 
that if the first one fails, the second one never gets to run. This method 
should be split into multiple methods. Ideally, one method per test should be 
followed. 
    
    1. 3 Drivers with ratio -- Should distribute in ratio.
    2. 3 Drivers without ratio -- Should distribute equally.
    3. 2 Drivers, where cost of queries on one driver is always less than the 
other one -- The first one should always be picked.
    
    For 1 and 2, let's have one extra driver which always gives a high cost. 
Assertions will be that that driver should never get picked.



lens-cube/src/test/java/org/apache/lens/driver/cube/TestWeightedCostSelector.java
 (line 78)
<https://reviews.apache.org/r/47777/#comment199670>

    Define constants for property name and default value in `LensConfConstants`.



lens-cube/src/test/java/org/apache/lens/driver/cube/TestWeightedCostSelector.java
 (line 132)
<https://reviews.apache.org/r/47777/#comment199672>

    Maybe we should rename it to `lens.driver.weight`. Having both `ratio` and 
`weight` might be confusing to the end-user. For end users, they need to set 
the selector class name in `lens-site.xml` and this property in driver's 
configuration files. The combination of <WeightedSelector, lens.driver.weight> 
sounds more intuitive.



lens-server-api/src/main/java/org/apache/lens/server/api/driver/WeightCostSelector.java
 (line 32)
<https://reviews.apache.org/r/47777/#comment199671>

    Rename to a more explicit name like `WeightedQueryCostDriverSelector`.


The reactor summary is incomplete. It doesn't cover all the modules.

- Rajat Khandelwal


On May 25, 2016, 4:52 p.m., Anshul Gupta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47777/
> -----------------------------------------------------------
> 
> (Updated May 25, 2016, 4:52 p.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-661
>     https://issues.apache.org/jira/browse/LENS-661
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> Added Weighted selector
> 
> 
> Diffs
> -----
> 
>   
> lens-cube/src/test/java/org/apache/lens/driver/cube/TestWeightedCostSelector.java
>  PRE-CREATION 
>   
> lens-server-api/src/main/java/org/apache/lens/server/api/driver/WeightCostSelector.java
>  PRE-CREATION 
>   
> lens-server-api/src/main/java/org/apache/lens/server/api/query/AbstractQueryContext.java
>  e160f58bfb45cffb05b9f6be19db3bc7605dde57 
> 
> Diff: https://reviews.apache.org/r/47777/diff/
> 
> 
> Testing
> -------
> 
> Here's the reactor summary after running mvn clean package
> [INFO] 
> ------------------------------------------------------------------------
> [INFO] Reactor Summary:
> [INFO]
> [INFO] Lens Server ........................................ SUCCESS [11:32 
> min]
> [INFO] Lens client ........................................ SUCCESS [01:49 
> min]
> [INFO] Lens CLI ........................................... SUCCESS [02:02 
> min]
> [INFO] Lens Examples ...................................... SUCCESS [  8.430 
> s]
> [INFO] Lens Ship Jars to Distributed Cache ................ SUCCESS [  2.801 
> s]
> [INFO] Lens Distribution .................................. SUCCESS [ 21.429 
> s]
> [INFO] Lens ML Lib ........................................ SUCCESS [02:28 
> min]
> [INFO] Lens ML Ext Distribution ........................... SUCCESS [ 20.690 
> s]
> [INFO] Lens Regression .................................... SUCCESS [  9.313 
> s]
> [INFO] Lens UI ............................................ SUCCESS [ 14.163 
> s]
> [INFO] 
> ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] 
> ------------------------------------------------------------------------
> [INFO] Total time: 19:10 min
> [INFO] Finished at: 2016-05-24T20:31:02+05:30
> [INFO] Final Memory: 103M/466M
> [INFO] 
> ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> Anshul Gupta
> 
>

Reply via email to