Re: Review Request 52982: Adding a message based retry policy

2016-10-27 Thread Rajat Khandelwal

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


Ship it!




Ship It!

- Rajat Khandelwal


On Oct. 26, 2016, 6:17 p.m., Lavkesh Lahngir wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52982/
> ---
> 
> (Updated Oct. 26, 2016, 6:17 p.m.)
> 
> 
> Review request for lens.
> 
> 
> Repository: lens
> 
> 
> Description
> ---
> 
> If any one of the configured messages are found then the retry will happen
> 
> 
> Diffs
> -
> 
>   lens-driver-hive/src/main/resources/hivedriver-default.xml f5fd3bb 
>   lens-driver-jdbc/src/main/resources/jdbcdriver-default.xml 89726c4 
>   
> lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java
>  3ae59c6 
>   
> lens-server-api/src/main/java/org/apache/lens/server/api/retry/BackOffRetryHandler.java
>  5ea5710 
>   
> lens-server-api/src/main/java/org/apache/lens/server/api/retry/FibonacciExponentialBackOffRetryHandler.java
>  01da25d 
>   
> lens-server-api/src/main/java/org/apache/lens/server/api/retry/ImmediateRetryHandler.java
>  c1c0126 
>   
> lens-server-api/src/main/java/org/apache/lens/server/api/retry/SubstringMessagePolicyDecider.java
>  PRE-CREATION 
>   
> lens-server-api/src/test/java/org/apache/lens/server/api/retry/TestSubStringMessagePolicyDecider.java
>  PRE-CREATION 
>   src/site/apt/admin/hivedriver-config.apt b1a25c3 
>   src/site/apt/admin/jdbcdriver-config.apt c64d6ee 
> 
> Diff: https://reviews.apache.org/r/52982/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Lavkesh Lahngir
> 
>



Re: Review Request 52982: Adding a message based retry policy

2016-10-26 Thread Rajat Khandelwal

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




lens-driver-hive/src/main/resources/hivedriver-default.xml (line 168)


Let's rename this as well to something like `retry.messages.contains.map`.



lens-driver-hive/src/main/resources/hivedriver-default.xml (lines 174 - 175)


Class name change missed here



lens-driver-jdbc/src/main/resources/jdbcdriver-default.xml (line 273)


Class name change missed here



lens-server-api/src/main/java/org/apache/lens/server/api/retry/SubstringMessagePolicyDecider.java
 (line 45)


generics. `Map.Entry`



lens-server-api/src/main/java/org/apache/lens/server/api/retry/SubstringMessagePolicyDecider.java
 (lines 46 - 47)


Avoid cast by using generics



lens-server-api/src/main/java/org/apache/lens/server/api/retry/SubstringMessagePolicyDecider.java
 (lines 76 - 77)


Generics



lens-server-api/src/main/java/org/apache/lens/server/api/retry/SubstringMessagePolicyDecider.java
 (lines 82 - 84)


minor:  
https://docs.oracle.com/javase/7/docs/api/java/util/Arrays.html#fill(java.lang.Object[],%20java.lang.Object)


- Rajat Khandelwal


On Oct. 25, 2016, 6:08 p.m., Lavkesh Lahngir wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52982/
> ---
> 
> (Updated Oct. 25, 2016, 6:08 p.m.)
> 
> 
> Review request for lens.
> 
> 
> Repository: lens
> 
> 
> Description
> ---
> 
> If any one of the configured messages are found then the retry will happen
> 
> 
> Diffs
> -
> 
>   lens-driver-hive/src/main/resources/hivedriver-default.xml f5fd3bb 
>   lens-driver-jdbc/src/main/resources/jdbcdriver-default.xml 89726c4 
>   
> lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java
>  3ae59c6 
>   
> lens-server-api/src/main/java/org/apache/lens/server/api/retry/FibonacciExponentialBackOffRetryHandler.java
>  01da25d 
>   
> lens-server-api/src/main/java/org/apache/lens/server/api/retry/ImmediateRetryHandler.java
>  c1c0126 
>   
> lens-server-api/src/main/java/org/apache/lens/server/api/retry/SubstringMessagePolicyDecider.java
>  PRE-CREATION 
>   
> lens-server-api/src/test/java/org/apache/lens/server/api/retry/TestSubStringMessagePolicyDecider.java
>  PRE-CREATION 
>   src/site/apt/admin/hivedriver-config.apt b1a25c3 
>   src/site/apt/admin/jdbcdriver-config.apt c64d6ee 
> 
> Diff: https://reviews.apache.org/r/52982/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Lavkesh Lahngir
> 
>



Re: Review Request 52982: Adding a message based retry policy

2016-10-25 Thread Lavkesh Lahngir

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

(Updated Oct. 25, 2016, 12:38 p.m.)


Review request for lens.


Repository: lens


Description
---

If any one of the configured messages are found then the retry will happen


Diffs (updated)
-

  lens-driver-hive/src/main/resources/hivedriver-default.xml f5fd3bb 
  lens-driver-jdbc/src/main/resources/jdbcdriver-default.xml 89726c4 
  
lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java 
3ae59c6 
  
lens-server-api/src/main/java/org/apache/lens/server/api/retry/FibonacciExponentialBackOffRetryHandler.java
 01da25d 
  
lens-server-api/src/main/java/org/apache/lens/server/api/retry/ImmediateRetryHandler.java
 c1c0126 
  
lens-server-api/src/main/java/org/apache/lens/server/api/retry/SubstringMessagePolicyDecider.java
 PRE-CREATION 
  
lens-server-api/src/test/java/org/apache/lens/server/api/retry/TestSubStringMessagePolicyDecider.java
 PRE-CREATION 
  src/site/apt/admin/hivedriver-config.apt b1a25c3 
  src/site/apt/admin/jdbcdriver-config.apt c64d6ee 

Diff: https://reviews.apache.org/r/52982/diff/


Testing
---


Thanks,

Lavkesh Lahngir



Re: Review Request 52982: Adding a message based retry policy

2016-10-24 Thread Rajat Khandelwal

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




lens-server-api/src/main/java/org/apache/lens/server/api/retry/ConfiguredPolicyDecider.java
 (line 35)


Name is a bit ambiguous regarding the functionality. Can we rename it to 
reflect that it's checking `contains` in error message?



lens-server-api/src/main/java/org/apache/lens/server/api/retry/ConfiguredPolicyDecider.java
 (line 45)


We should handle for cases when either of the above two are null.



lens-server-api/src/main/java/org/apache/lens/server/api/retry/FibonacciExponentialBackOffRetryHandler.java
 (line 37)


The arguments are int, long, long



lens-server-api/src/test/java/org/apache/lens/server/api/retry/TestConfiguredPolicyDecider.java
 (lines 54 - 56)


This seems like a problem in `ImmediateRetryHandler`. Instead of 
```
  @Override
  public boolean hasExhaustedRetries(FC failContext) {
return ++retriesDone > retries;
  }
```
It should be

```
  @Override
  public boolean hasExhaustedRetries(FC failContext) {
failContext.getFailCount() > retries;
  }

```

Can you make the change?


- Rajat Khandelwal


On Oct. 20, 2016, 6:27 p.m., Lavkesh Lahngir wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52982/
> ---
> 
> (Updated Oct. 20, 2016, 6:27 p.m.)
> 
> 
> Review request for lens.
> 
> 
> Repository: lens
> 
> 
> Description
> ---
> 
> If any one of the configured messages are found then the retry will happen
> 
> 
> Diffs
> -
> 
>   lens-driver-hive/src/main/resources/hivedriver-default.xml 
> f5fd3bb20b9d6b82292c4483d860ea14c8104c5c 
>   lens-driver-jdbc/src/main/resources/jdbcdriver-default.xml 
> 89726c4f352a6e57dd81b6ae84cca221a345ebb6 
>   
> lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java
>  3ae59c6ca126f0a3535974352adf97e7a495838f 
>   
> lens-server-api/src/main/java/org/apache/lens/server/api/retry/ConfiguredPolicyDecider.java
>  PRE-CREATION 
>   
> lens-server-api/src/main/java/org/apache/lens/server/api/retry/FibonacciExponentialBackOffRetryHandler.java
>  01da25d12393615cfb4e4680c97c7445df0bbe83 
>   
> lens-server-api/src/main/java/org/apache/lens/server/api/retry/ImmediateRetryHandler.java
>  c1c0126ae9ac89cb4a50fe5948075094f10cb518 
>   
> lens-server-api/src/test/java/org/apache/lens/server/api/retry/TestConfiguredPolicyDecider.java
>  PRE-CREATION 
>   src/site/apt/admin/hivedriver-config.apt 
> b1a25c3b5bb58fc2d0773dc168c110ee5e60c635 
>   src/site/apt/admin/jdbcdriver-config.apt 
> c64d6ee0d4ac1dff9fadd02826e67a8e4839d99f 
> 
> Diff: https://reviews.apache.org/r/52982/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Lavkesh Lahngir
> 
>



Re: Review Request 52982: Adding a message based retry policy

2016-10-20 Thread Lavkesh Lahngir

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

(Updated Oct. 20, 2016, 12:57 p.m.)


Review request for lens.


Changes
---

Added test cases


Repository: lens


Description
---

If any one of the configured messages are found then the retry will happen


Diffs (updated)
-

  lens-driver-hive/src/main/resources/hivedriver-default.xml 
f5fd3bb20b9d6b82292c4483d860ea14c8104c5c 
  lens-driver-jdbc/src/main/resources/jdbcdriver-default.xml 
89726c4f352a6e57dd81b6ae84cca221a345ebb6 
  
lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java 
3ae59c6ca126f0a3535974352adf97e7a495838f 
  
lens-server-api/src/main/java/org/apache/lens/server/api/retry/ConfiguredPolicyDecider.java
 PRE-CREATION 
  
lens-server-api/src/main/java/org/apache/lens/server/api/retry/FibonacciExponentialBackOffRetryHandler.java
 01da25d12393615cfb4e4680c97c7445df0bbe83 
  
lens-server-api/src/main/java/org/apache/lens/server/api/retry/ImmediateRetryHandler.java
 c1c0126ae9ac89cb4a50fe5948075094f10cb518 
  
lens-server-api/src/test/java/org/apache/lens/server/api/retry/TestConfiguredPolicyDecider.java
 PRE-CREATION 
  src/site/apt/admin/hivedriver-config.apt 
b1a25c3b5bb58fc2d0773dc168c110ee5e60c635 
  src/site/apt/admin/jdbcdriver-config.apt 
c64d6ee0d4ac1dff9fadd02826e67a8e4839d99f 

Diff: https://reviews.apache.org/r/52982/diff/


Testing
---


Thanks,

Lavkesh Lahngir



Re: Review Request 52982: Adding a message based retry policy

2016-10-18 Thread Rajat Khandelwal

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




lens-server-api/src/main/java/org/apache/lens/server/api/retry/ConfiguredPolicyDecider.java
 (line 27)


Let's check regex match instead of contains


- Rajat Khandelwal


On Oct. 18, 2016, 8:18 p.m., Lavkesh Lahngir wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52982/
> ---
> 
> (Updated Oct. 18, 2016, 8:18 p.m.)
> 
> 
> Review request for lens.
> 
> 
> Repository: lens
> 
> 
> Description
> ---
> 
> If any one of the configured messages are found then the retry will happen
> 
> 
> Diffs
> -
> 
>   lens-driver-hive/src/main/resources/hivedriver-default.xml 
> f5fd3bb20b9d6b82292c4483d860ea14c8104c5c 
>   lens-driver-jdbc/src/main/resources/jdbcdriver-default.xml 
> 89726c4f352a6e57dd81b6ae84cca221a345ebb6 
>   
> lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java
>  3ae59c6ca126f0a3535974352adf97e7a495838f 
>   
> lens-server-api/src/main/java/org/apache/lens/server/api/retry/ConfiguredPolicyDecider.java
>  PRE-CREATION 
>   src/site/apt/admin/hivedriver-config.apt 
> b1a25c3b5bb58fc2d0773dc168c110ee5e60c635 
>   src/site/apt/admin/jdbcdriver-config.apt 
> c64d6ee0d4ac1dff9fadd02826e67a8e4839d99f 
> 
> Diff: https://reviews.apache.org/r/52982/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Lavkesh Lahngir
> 
>



Re: Review Request 52982: Adding a message based retry policy

2016-10-18 Thread Lavkesh Lahngir

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

(Updated Oct. 18, 2016, 2:48 p.m.)


Review request for lens.


Repository: lens


Description
---

If any one of the configured messages are found then the retry will happen


Diffs (updated)
-

  lens-driver-hive/src/main/resources/hivedriver-default.xml 
f5fd3bb20b9d6b82292c4483d860ea14c8104c5c 
  lens-driver-jdbc/src/main/resources/jdbcdriver-default.xml 
89726c4f352a6e57dd81b6ae84cca221a345ebb6 
  
lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java 
3ae59c6ca126f0a3535974352adf97e7a495838f 
  
lens-server-api/src/main/java/org/apache/lens/server/api/retry/ConfiguredPolicyDecider.java
 PRE-CREATION 
  src/site/apt/admin/hivedriver-config.apt 
b1a25c3b5bb58fc2d0773dc168c110ee5e60c635 
  src/site/apt/admin/jdbcdriver-config.apt 
c64d6ee0d4ac1dff9fadd02826e67a8e4839d99f 

Diff: https://reviews.apache.org/r/52982/diff/


Testing
---


Thanks,

Lavkesh Lahngir



Review Request 52982: Adding a message based retry policy

2016-10-18 Thread Lavkesh Lahngir

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

Review request for lens.


Repository: lens


Description
---

If any one of the configured messages are found then the retry will happen


Diffs
-

  lens-driver-hive/src/main/resources/hivedriver-default.xml 
f5fd3bb20b9d6b82292c4483d860ea14c8104c5c 
  lens-driver-jdbc/src/main/resources/jdbcdriver-default.xml 
89726c4f352a6e57dd81b6ae84cca221a345ebb6 
  
lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java 
3ae59c6ca126f0a3535974352adf97e7a495838f 
  
lens-server-api/src/main/java/org/apache/lens/server/api/retry/SingleRetryPolicyDecider.java
 PRE-CREATION 
  src/site/apt/admin/hivedriver-config.apt 
b1a25c3b5bb58fc2d0773dc168c110ee5e60c635 
  src/site/apt/admin/jdbcdriver-config.apt 
c64d6ee0d4ac1dff9fadd02826e67a8e4839d99f 

Diff: https://reviews.apache.org/r/52982/diff/


Testing
---


Thanks,

Lavkesh Lahngir