Re: Review Request 29807: HIVE-9253: MetaStore server should support timeout for long running requests

2015-02-26 Thread Dong Chen 

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

(Updated Feb. 27, 2015, 1:48 a.m.)


Review request for hive.


Changes
---

Re-use the conf property hive.metastore.client.socket.timeout


Repository: hive-git


Description
---

HIVE-9253: MetaStore server should support timeout for long running requests


Diffs (updated)
-

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 8e072f7 
  metastore/src/java/org/apache/hadoop/hive/metastore/Deadline.java 
PRE-CREATION 
  metastore/src/java/org/apache/hadoop/hive/metastore/DeadlineException.java 
PRE-CREATION 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 
ab011fc 
  metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java 
574141c 
  metastore/src/java/org/apache/hadoop/hive/metastore/RetryingHMSHandler.java 
01ad36a 
  
metastore/src/java/org/apache/hadoop/hive/metastore/SessionPropertiesListener.java
 PRE-CREATION 
  metastore/src/test/org/apache/hadoop/hive/metastore/TestDeadline.java 
PRE-CREATION 
  
metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStoreTimeout.java
 PRE-CREATION 

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


Testing
---

UT passed


Thanks,

Dong Chen

Re: Review Request 29807: HIVE-9253: MetaStore server should support timeout for long running requests

2015-02-26 Thread Brock Noland 

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


This looks great to me! We can do the check in more places in a follow-on jira. 
I just had one thought about the property name and then I think let's commit!

Nice work Dong!!


common/src/java/org/apache/hadoop/hive/conf/HiveConf.java


I don't feel it makes any sense to have a smaller "long running timeout" 
than hive.metastore.client.socket.timeout or any larger. Thus as opposed to 
creating a new config I feel like we should re-use 
hive.metastore.client.socket.timeout.


- Brock Noland


On Feb. 26, 2015, 8:51 a.m., Dong Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29807/
> ---
> 
> (Updated Feb. 26, 2015, 8:51 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-9253: MetaStore server should support timeout for long running requests
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 8e072f7 
>   metastore/src/java/org/apache/hadoop/hive/metastore/Deadline.java 
> PRE-CREATION 
>   metastore/src/java/org/apache/hadoop/hive/metastore/DeadlineException.java 
> PRE-CREATION 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 
> ab011fc 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java 
> 574141c 
>   metastore/src/java/org/apache/hadoop/hive/metastore/RetryingHMSHandler.java 
> 01ad36a 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/SessionPropertiesListener.java
>  PRE-CREATION 
>   metastore/src/test/org/apache/hadoop/hive/metastore/TestDeadline.java 
> PRE-CREATION 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStoreTimeout.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/29807/diff/
> 
> 
> Testing
> ---
> 
> UT passed
> 
> 
> Thanks,
> 
> Dong Chen
> 
>

Re: Review Request 29807: HIVE-9253: MetaStore server should support timeout for long running requests

2015-02-26 Thread Dong Chen 

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

(Updated Feb. 26, 2015, 8:51 a.m.)


Review request for hive.


Changes
---

Address comments from Mohit.


Repository: hive-git


Description
---

HIVE-9253: MetaStore server should support timeout for long running requests


Diffs (updated)
-

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 8e072f7 
  metastore/src/java/org/apache/hadoop/hive/metastore/Deadline.java 
PRE-CREATION 
  metastore/src/java/org/apache/hadoop/hive/metastore/DeadlineException.java 
PRE-CREATION 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 
ab011fc 
  metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java 
574141c 
  metastore/src/java/org/apache/hadoop/hive/metastore/RetryingHMSHandler.java 
01ad36a 
  
metastore/src/java/org/apache/hadoop/hive/metastore/SessionPropertiesListener.java
 PRE-CREATION 
  metastore/src/test/org/apache/hadoop/hive/metastore/TestDeadline.java 
PRE-CREATION 
  
metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStoreTimeout.java
 PRE-CREATION 

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


Testing
---

UT passed


Thanks,

Dong Chen

Re: Review Request 29807: HIVE-9253: MetaStore server should support timeout for long running requests

2015-02-26 Thread Dong Chen 


> On Feb. 12, 2015, 8:42 a.m., Mohit Sabharwal wrote:
> > metastore/src/java/org/apache/hadoop/hive/metastore/Deadline.java, line 45
> > 
> >
> > Why do we need to store the method name of the HMS method we are timing 
> > ? Is it just so we can print it in the exception message ? 
> > 
> > Instead of storing it here, maybe we can log the method name when the 
> > DeadlineException is caught in HMS ?

It is a good idea to log the method name when the DeadlineException is caught 
in HMS. I added it in new patch.

How about we still keep the method name in the exception message? Since after 
logging the exception, it is re-thrown. Maybe upper caller or client side want 
to know the method info from the exception.


> On Feb. 12, 2015, 8:42 a.m., Mohit Sabharwal wrote:
> > metastore/src/java/org/apache/hadoop/hive/metastore/Deadline.java, line 110
> > 
> >
> > do we really need to record the method ?

As discussed above, if client side want to know method info from the exception, 
this maybe need to be record.


- Dong


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


On Jan. 28, 2015, 8:58 a.m., Dong Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29807/
> ---
> 
> (Updated Jan. 28, 2015, 8:58 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-9253: MetaStore server should support timeout for long running requests
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 66f436b 
>   metastore/src/java/org/apache/hadoop/hive/metastore/Deadline.java 
> PRE-CREATION 
>   metastore/src/java/org/apache/hadoop/hive/metastore/DeadlineException.java 
> PRE-CREATION 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 
> fc6f067 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java 
> 574141c 
>   metastore/src/java/org/apache/hadoop/hive/metastore/RetryingHMSHandler.java 
> 01ad36a 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/SessionPropertiesListener.java
>  PRE-CREATION 
>   metastore/src/test/org/apache/hadoop/hive/metastore/TestDeadline.java 
> PRE-CREATION 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStoreTimeout.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/29807/diff/
> 
> 
> Testing
> ---
> 
> UT passed
> 
> 
> Thanks,
> 
> Dong Chen
> 
>

Re: Review Request 29807: HIVE-9253: MetaStore server should support timeout for long running requests

2015-02-12 Thread Dong Chen 


> On Feb. 12, 2015, 8:42 a.m., Mohit Sabharwal wrote:
> > LGTM. Had a few questions.

Thank you so much for you feedbacks! Mohit.
I will update the patch based on your commnets after holiday. Thanks!


- Dong


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


On Jan. 28, 2015, 8:58 a.m., Dong Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29807/
> ---
> 
> (Updated Jan. 28, 2015, 8:58 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-9253: MetaStore server should support timeout for long running requests
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 66f436b 
>   metastore/src/java/org/apache/hadoop/hive/metastore/Deadline.java 
> PRE-CREATION 
>   metastore/src/java/org/apache/hadoop/hive/metastore/DeadlineException.java 
> PRE-CREATION 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 
> fc6f067 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java 
> 574141c 
>   metastore/src/java/org/apache/hadoop/hive/metastore/RetryingHMSHandler.java 
> 01ad36a 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/SessionPropertiesListener.java
>  PRE-CREATION 
>   metastore/src/test/org/apache/hadoop/hive/metastore/TestDeadline.java 
> PRE-CREATION 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStoreTimeout.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/29807/diff/
> 
> 
> Testing
> ---
> 
> UT passed
> 
> 
> Thanks,
> 
> Dong Chen
> 
>

Re: Review Request 29807: HIVE-9253: MetaStore server should support timeout for long running requests

2015-02-12 Thread Mohit Sabharwal 

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


LGTM. Had a few questions.


common/src/java/org/apache/hadoop/hive/conf/HiveConf.java


It might be important to highlight in the description that not all (long 
running) HMS methods will timeout with this config. Only certain ones "we think 
take long usually"maybe we can check with Lefty how to word this in the 
docs.



metastore/src/java/org/apache/hadoop/hive/metastore/Deadline.java


Why do we need to store the method name of the HMS method we are timing ? 
Is it just so we can print it in the exception message ? 

Instead of storing it here, maybe we can log the method name when the 
DeadlineException is caught in HMS ?



metastore/src/java/org/apache/hadoop/hive/metastore/Deadline.java


How about calling this startTimer() ? 

startCounting() seems to be confusing ... we're just starting a timer 
here...



metastore/src/java/org/apache/hadoop/hive/metastore/Deadline.java


do we really need to record the method ?



metastore/src/java/org/apache/hadoop/hive/metastore/Deadline.java


stopCounter()



metastore/src/java/org/apache/hadoop/hive/metastore/Deadline.java


null instead of "" ?



metastore/src/java/org/apache/hadoop/hive/metastore/Deadline.java


firstly -> first



metastore/src/java/org/apache/hadoop/hive/metastore/Deadline.java


execute -> executing



metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java


please move this up (alphabetically)



metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java


Let's call this TEST_TIMEOUT_ENABLED



metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java


can we just set it to an invalid value like -1 here ? Seems weird to 
initialize it to a unit test specific value in HMS class.

TEST_TIMEOUT_VALUE



metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java


please inline this, no need for separate method here.



metastore/src/java/org/apache/hadoop/hive/metastore/RetryingHMSHandler.java


Should we log the method name here instead of storing it in Deadline ?



metastore/src/java/org/apache/hadoop/hive/metastore/SessionPropertiesListener.java


nit: redundant comment



metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStoreTimeout.java


please move this import up (alphabetically)



metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStoreTimeout.java


nit: no point converting Throwable to an Exception. just 'throw e' should 
be fine.



metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStoreTimeout.java


nit: throw e



metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStoreTimeout.java


please use 
HiveConf.ConfVars.METASTORE_SERVER_LONG_RUNNING_METHOD_TIMEOUT.varname



metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStoreTimeout.java


please use 
HiveConf.ConfVars.METASTORE_SERVER_LONG_RUNNING_METHOD_TIMEOUT.varname


- Mohit Sabharwal


On Jan. 28, 2015, 8:58 a.m., Dong Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29807/
> ---
> 
> (Updated Jan. 28, 2015, 8:58 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-9253: MetaStore server should support timeout for long running requests
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 66f436b 
>   metastore/src/java/org/apache/hadoop/hive/metastore/Deadline.java 
> PRE-CREATION 
>   metastore/src/java/org/apache/hadoop/hive/metastore/DeadlineException.java 
> PRE-CREATION 
>   metastore/src/java/org/

Re: Review Request 29807: HIVE-9253: MetaStore server should support timeout for long running requests

2015-01-31 Thread Lefty Leverenz 

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

Ship it!


Ship It!

- Lefty Leverenz


On Jan. 28, 2015, 12:58 a.m., Dong Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29807/
> ---
> 
> (Updated Jan. 28, 2015, 12:58 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-9253: MetaStore server should support timeout for long running requests
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 66f436b 
>   metastore/src/java/org/apache/hadoop/hive/metastore/Deadline.java 
> PRE-CREATION 
>   metastore/src/java/org/apache/hadoop/hive/metastore/DeadlineException.java 
> PRE-CREATION 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 
> fc6f067 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java 
> 574141c 
>   metastore/src/java/org/apache/hadoop/hive/metastore/RetryingHMSHandler.java 
> 01ad36a 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/SessionPropertiesListener.java
>  PRE-CREATION 
>   metastore/src/test/org/apache/hadoop/hive/metastore/TestDeadline.java 
> PRE-CREATION 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStoreTimeout.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/29807/diff/
> 
> 
> Testing
> ---
> 
> UT passed
> 
> 
> Thanks,
> 
> Dong Chen
> 
>

Re: Review Request 29807: HIVE-9253: MetaStore server should support timeout for long running requests

2015-01-31 Thread Lefty Leverenz 


> On Jan. 20, 2015, 10:43 p.m., Lefty Leverenz wrote:
> > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java, lines 372-374
> > 
> >
> > Shouldn't "long" & "LONG" be included in the names 
> > "hive.metastore.server.running.method.timeout" & 
> > "METASTORE_SERVER_RUNNING_METHOD_TIMEOUT"?
> > 
> > Also, please specify the JIRA number (HIVE-9253) in this review 
> > request, either under Bugs in the Information section or in the Summary, or 
> > both.
> 
> Dong Chen wrote:
> Thanks for your review and suggestion! Lefty. 
> I have renamed it in the new patch.

Thanks very much.


- Lefty


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


On Jan. 28, 2015, 12:58 a.m., Dong Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29807/
> ---
> 
> (Updated Jan. 28, 2015, 12:58 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-9253: MetaStore server should support timeout for long running requests
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 66f436b 
>   metastore/src/java/org/apache/hadoop/hive/metastore/Deadline.java 
> PRE-CREATION 
>   metastore/src/java/org/apache/hadoop/hive/metastore/DeadlineException.java 
> PRE-CREATION 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 
> fc6f067 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java 
> 574141c 
>   metastore/src/java/org/apache/hadoop/hive/metastore/RetryingHMSHandler.java 
> 01ad36a 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/SessionPropertiesListener.java
>  PRE-CREATION 
>   metastore/src/test/org/apache/hadoop/hive/metastore/TestDeadline.java 
> PRE-CREATION 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStoreTimeout.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/29807/diff/
> 
> 
> Testing
> ---
> 
> UT passed
> 
> 
> Thanks,
> 
> Dong Chen
> 
>

Re: Review Request 29807: HIVE-9253: MetaStore server should support timeout for long running requests

2015-01-28 Thread Dong Chen 

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

(Updated Jan. 28, 2015, 8:58 a.m.)


Review request for hive.


Changes
---

Address comments from Lefty and Brock.


Repository: hive-git


Description
---

HIVE-9253: MetaStore server should support timeout for long running requests


Diffs (updated)
-

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 66f436b 
  metastore/src/java/org/apache/hadoop/hive/metastore/Deadline.java 
PRE-CREATION 
  metastore/src/java/org/apache/hadoop/hive/metastore/DeadlineException.java 
PRE-CREATION 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 
fc6f067 
  metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java 
574141c 
  metastore/src/java/org/apache/hadoop/hive/metastore/RetryingHMSHandler.java 
01ad36a 
  
metastore/src/java/org/apache/hadoop/hive/metastore/SessionPropertiesListener.java
 PRE-CREATION 
  metastore/src/test/org/apache/hadoop/hive/metastore/TestDeadline.java 
PRE-CREATION 
  
metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStoreTimeout.java
 PRE-CREATION 

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


Testing
---

UT passed


Thanks,

Dong Chen

Re: Review Request 29807: HIVE-9253: MetaStore server should support timeout for long running requests

2015-01-28 Thread Dong Chen 


> On Jan. 21, 2015, 6:43 a.m., Lefty Leverenz wrote:
> > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java, lines 372-374
> > 
> >
> > Shouldn't "long" & "LONG" be included in the names 
> > "hive.metastore.server.running.method.timeout" & 
> > "METASTORE_SERVER_RUNNING_METHOD_TIMEOUT"?
> > 
> > Also, please specify the JIRA number (HIVE-9253) in this review 
> > request, either under Bugs in the Information section or in the Summary, or 
> > both.

Thanks for your review and suggestion! Lefty. 
I have renamed it in the new patch.


- Dong


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


On Jan. 22, 2015, 8:22 a.m., Dong Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29807/
> ---
> 
> (Updated Jan. 22, 2015, 8:22 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-9253: MetaStore server should support timeout for long running requests
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 5e00575 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 
> caad948 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java 
> 564ac8b 
>   metastore/src/java/org/apache/hadoop/hive/metastore/RetryingHMSHandler.java 
> 01ad36a 
>   metastore/src/java/org/apache/hadoop/hive/metastore/RuntimeTimeout.java 
> PRE-CREATION 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/RuntimeTimeoutException.java
>  PRE-CREATION 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/SessionPropertiesListener.java
>  PRE-CREATION 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStoreTimeout.java
>  PRE-CREATION 
>   metastore/src/test/org/apache/hadoop/hive/metastore/TestRuntimeTimeout.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/29807/diff/
> 
> 
> Testing
> ---
> 
> UT passed
> 
> 
> Thanks,
> 
> Dong Chen
> 
>

Re: Review Request 29807: HIVE-9253: MetaStore server should support timeout for long running requests

2015-01-27 Thread Dong Chen 


> On Jan. 27, 2015, 6:20 a.m., Brock Noland wrote:
> > Hi Dong,
> > 
> > This looks great!!
> > 
> > Can we rename "RuntimeTimeout" to "Deadline" so it's more clear what this 
> > is? Also it appears we are taking the timeout configuration from local 
> > config. Do you plan on having the client setting the timeout value they are 
> > using somehow? Clients might use varying timeouts.
> > 
> > Thanks again! I really like this change.
> > 
> > Brock

Thanks for your review! Brock

About client setting the timeout, a SessionPropertiesListener extending 
MetaStoreEventListener was added in this patch for handling client requesting 
timeout change. I think this could introduce code change least. How does that 
sound?

For usage, client could use "set 
metaconf:hive.metastore.server.running.method.timeout 500s" to change timeout. 
The call stack in HS2/HMS is like SetProcesser.setVariable -> Hive.setMetaConf 
-> HiveMetaStoreClient.setMetaConf -> HiveMetaStore.HMSHandler.setMetaConf -> 
notify added listeners -> SessionPropertiesListener.onConfigChange.

A test was added in TestHiveMetaStoreTimeout.testResetTimeout.

Since there is no interface change for users and they just need to set the conf 
parameter in their session, maybe it is necessary to mention the set timeout 
command in wiki doc for user knowing it.


- Dong


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


On Jan. 22, 2015, 8:22 a.m., Dong Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29807/
> ---
> 
> (Updated Jan. 22, 2015, 8:22 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-9253: MetaStore server should support timeout for long running requests
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 5e00575 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 
> caad948 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java 
> 564ac8b 
>   metastore/src/java/org/apache/hadoop/hive/metastore/RetryingHMSHandler.java 
> 01ad36a 
>   metastore/src/java/org/apache/hadoop/hive/metastore/RuntimeTimeout.java 
> PRE-CREATION 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/RuntimeTimeoutException.java
>  PRE-CREATION 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/SessionPropertiesListener.java
>  PRE-CREATION 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStoreTimeout.java
>  PRE-CREATION 
>   metastore/src/test/org/apache/hadoop/hive/metastore/TestRuntimeTimeout.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/29807/diff/
> 
> 
> Testing
> ---
> 
> UT passed
> 
> 
> Thanks,
> 
> Dong Chen
> 
>

Re: Review Request 29807: HIVE-9253: MetaStore server should support timeout for long running requests

2015-01-26 Thread Brock Noland 

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


Hi Dong,

This looks great!!

Can we rename "RuntimeTimeout" to "Deadline" so it's more clear what this is? 
Also it appears we are taking the timeout configuration from local config. Do 
you plan on having the client setting the timeout value they are using somehow? 
Clients might use varying timeouts.

Thanks again! I really like this change.

Brock


metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java


an you add VisibleForTest?



metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStoreTimeout.java


We are mixing old-style TestCase and annotation based testing. We should 
remove TestCase super class and used @Test, @Before @After



metastore/src/test/org/apache/hadoop/hive/metastore/TestRuntimeTimeout.java


We are mixing TestCase and @Test annotations. I think we should remove the 
TestCase super class.



metastore/src/test/org/apache/hadoop/hive/metastore/TestRuntimeTimeout.java


let's log this exception



metastore/src/test/org/apache/hadoop/hive/metastore/TestRuntimeTimeout.java


let's log this exception



metastore/src/test/org/apache/hadoop/hive/metastore/TestRuntimeTimeout.java


let's log this exception



metastore/src/test/org/apache/hadoop/hive/metastore/TestRuntimeTimeout.java


let's log this exception and give a better error message



metastore/src/test/org/apache/hadoop/hive/metastore/TestRuntimeTimeout.java


let's log this exception


- Brock Noland


On Jan. 22, 2015, 8:22 a.m., Dong Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29807/
> ---
> 
> (Updated Jan. 22, 2015, 8:22 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-9253: MetaStore server should support timeout for long running requests
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 5e00575 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 
> caad948 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java 
> 564ac8b 
>   metastore/src/java/org/apache/hadoop/hive/metastore/RetryingHMSHandler.java 
> 01ad36a 
>   metastore/src/java/org/apache/hadoop/hive/metastore/RuntimeTimeout.java 
> PRE-CREATION 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/RuntimeTimeoutException.java
>  PRE-CREATION 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/SessionPropertiesListener.java
>  PRE-CREATION 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStoreTimeout.java
>  PRE-CREATION 
>   metastore/src/test/org/apache/hadoop/hive/metastore/TestRuntimeTimeout.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/29807/diff/
> 
> 
> Testing
> ---
> 
> UT passed
> 
> 
> Thanks,
> 
> Dong Chen
> 
>

Re: Review Request 29807: HIVE-9253: MetaStore server should support timeout for long running requests

2015-01-22 Thread Dong Chen 

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

(Updated Jan. 22, 2015, 8:22 a.m.)


Review request for hive.


Changes
---

fix the failed test cases and rebase with trunk


Repository: hive-git


Description
---

HIVE-9253: MetaStore server should support timeout for long running requests


Diffs (updated)
-

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 5e00575 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 
caad948 
  metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java 
564ac8b 
  metastore/src/java/org/apache/hadoop/hive/metastore/RetryingHMSHandler.java 
01ad36a 
  metastore/src/java/org/apache/hadoop/hive/metastore/RuntimeTimeout.java 
PRE-CREATION 
  
metastore/src/java/org/apache/hadoop/hive/metastore/RuntimeTimeoutException.java
 PRE-CREATION 
  
metastore/src/java/org/apache/hadoop/hive/metastore/SessionPropertiesListener.java
 PRE-CREATION 
  
metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStoreTimeout.java
 PRE-CREATION 
  metastore/src/test/org/apache/hadoop/hive/metastore/TestRuntimeTimeout.java 
PRE-CREATION 

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


Testing
---

UT passed


Thanks,

Dong Chen

Re: Review Request 29807: HIVE-9253: MetaStore server should support timeout for long running requests

2015-01-20 Thread Dong Chen 

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

(Updated Jan. 21, 2015, 6:47 a.m.)


Review request for hive.


Summary (updated)
-

HIVE-9253: MetaStore server should support timeout for long running requests


Repository: hive-git


Description (updated)
---

HIVE-9253: MetaStore server should support timeout for long running requests


Diffs
-

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 8264b16 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 
2db90cc 
  metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java 
564ac8b 
  metastore/src/java/org/apache/hadoop/hive/metastore/RetryingHMSHandler.java 
01ad36a 
  metastore/src/java/org/apache/hadoop/hive/metastore/RuntimeTimeout.java 
PRE-CREATION 
  
metastore/src/java/org/apache/hadoop/hive/metastore/RuntimeTimeoutException.java
 PRE-CREATION 
  
metastore/src/java/org/apache/hadoop/hive/metastore/SessionPropertiesListener.java
 PRE-CREATION 
  
metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStoreTimeout.java
 PRE-CREATION 
  metastore/src/test/org/apache/hadoop/hive/metastore/TestRuntimeTimeout.java 
PRE-CREATION 

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


Testing
---

UT passed


Thanks,

Dong Chen