Re: Review Request 52835: HIVE-14926: Keep Schema in consistent state where schemaTool fails or succeeds

2016-10-14 Thread Aihua Xu 


> On Oct. 14, 2016, 3:38 p.m., Aihua Xu wrote:
> > beeline/src/java/org/apache/hive/beeline/HiveSchemaTool.java, line 123
> > 
> >
> > I verified against derby and it works fine. 
> > 
> > Let me verify other databases. Seems as you said, some databases like 
> > mysql may have such issues.

OK. That seems to be a problem. It won't work as expected for some databases.


- Aihua


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


On Oct. 13, 2016, 8:43 p.m., Aihua Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52835/
> ---
> 
> (Updated Oct. 13, 2016, 8:43 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-14926: Keep Schema in consistent state where schemaTool fails or succeeds
> 
> 
> Diffs
> -
> 
>   beeline/src/java/org/apache/hive/beeline/HiveSchemaHelper.java 181f0d2 
>   beeline/src/java/org/apache/hive/beeline/HiveSchemaTool.java cd36ddf 
>   itests/hive-unit/src/test/java/org/apache/hive/beeline/TestSchemaTool.java 
> 0d5f9c8 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreSchemaInfo.java 
> 9c30ee7 
> 
> Diff: https://reviews.apache.org/r/52835/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Aihua Xu
> 
>

Re: Review Request 52835: HIVE-14926: Keep Schema in consistent state where schemaTool fails or succeeds

2016-10-14 Thread Aihua Xu 

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




beeline/src/java/org/apache/hive/beeline/HiveSchemaTool.java (line 115)


I verified against derby and it works fine. 

Let me verify other databases. Seems as you said, some databases like mysql 
may have such issues.


- Aihua Xu


On Oct. 13, 2016, 8:43 p.m., Aihua Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52835/
> ---
> 
> (Updated Oct. 13, 2016, 8:43 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-14926: Keep Schema in consistent state where schemaTool fails or succeeds
> 
> 
> Diffs
> -
> 
>   beeline/src/java/org/apache/hive/beeline/HiveSchemaHelper.java 181f0d2 
>   beeline/src/java/org/apache/hive/beeline/HiveSchemaTool.java cd36ddf 
>   itests/hive-unit/src/test/java/org/apache/hive/beeline/TestSchemaTool.java 
> 0d5f9c8 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreSchemaInfo.java 
> 9c30ee7 
> 
> Diff: https://reviews.apache.org/r/52835/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Aihua Xu
> 
>

Re: Review Request 52835: HIVE-14926: Keep Schema in consistent state where schemaTool fails or succeeds

2016-10-14 Thread Chaoyu Tang 

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




beeline/src/java/org/apache/hive/beeline/HiveSchemaTool.java (line 115)


Does the autoCommit false work for DDLs? As far as I know, some RDBMS and 
their drivers might not, and DDLs commit automatically regardless. Most sqls in 
HMS upgrade scripts are DDLs (e.g. alter table etc), so this patch might not 
work as expected.


- Chaoyu Tang


On Oct. 13, 2016, 8:43 p.m., Aihua Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52835/
> ---
> 
> (Updated Oct. 13, 2016, 8:43 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-14926: Keep Schema in consistent state where schemaTool fails or succeeds
> 
> 
> Diffs
> -
> 
>   beeline/src/java/org/apache/hive/beeline/HiveSchemaHelper.java 181f0d2 
>   beeline/src/java/org/apache/hive/beeline/HiveSchemaTool.java cd36ddf 
>   itests/hive-unit/src/test/java/org/apache/hive/beeline/TestSchemaTool.java 
> 0d5f9c8 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreSchemaInfo.java 
> 9c30ee7 
> 
> Diff: https://reviews.apache.org/r/52835/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Aihua Xu
> 
>

Re: Review Request 52835: HIVE-14926: Keep Schema in consistent state where schemaTool fails or succeeds

2016-10-14 Thread Aihua Xu 


> On Oct. 14, 2016, 2:18 p.m., Yongzhi Chen wrote:
> > beeline/src/java/org/apache/hive/beeline/HiveSchemaHelper.java, line 231
> > 
> >
> > Is that possible a command has more than one lines?

Yes. It's possible. HiveSchemaHelper will normalize them into a single line. 
(Of course there will be some limitation there but I haven't touched that logic 
there).


- Aihua


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


On Oct. 13, 2016, 8:43 p.m., Aihua Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52835/
> ---
> 
> (Updated Oct. 13, 2016, 8:43 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-14926: Keep Schema in consistent state where schemaTool fails or succeeds
> 
> 
> Diffs
> -
> 
>   beeline/src/java/org/apache/hive/beeline/HiveSchemaHelper.java 181f0d2 
>   beeline/src/java/org/apache/hive/beeline/HiveSchemaTool.java cd36ddf 
>   itests/hive-unit/src/test/java/org/apache/hive/beeline/TestSchemaTool.java 
> 0d5f9c8 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreSchemaInfo.java 
> 9c30ee7 
> 
> Diff: https://reviews.apache.org/r/52835/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Aihua Xu
> 
>

Re: Review Request 52835: HIVE-14926: Keep Schema in consistent state where schemaTool fails or succeeds

2016-10-14 Thread Yongzhi Chen 

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




beeline/src/java/org/apache/hive/beeline/HiveSchemaHelper.java (line 231)


Is that possible a command has more than one lines?


- Yongzhi Chen


On Oct. 13, 2016, 8:43 p.m., Aihua Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52835/
> ---
> 
> (Updated Oct. 13, 2016, 8:43 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-14926: Keep Schema in consistent state where schemaTool fails or succeeds
> 
> 
> Diffs
> -
> 
>   beeline/src/java/org/apache/hive/beeline/HiveSchemaHelper.java 181f0d2 
>   beeline/src/java/org/apache/hive/beeline/HiveSchemaTool.java cd36ddf 
>   itests/hive-unit/src/test/java/org/apache/hive/beeline/TestSchemaTool.java 
> 0d5f9c8 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreSchemaInfo.java 
> 9c30ee7 
> 
> Diff: https://reviews.apache.org/r/52835/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Aihua Xu
> 
>

Re: Review Request 52835: HIVE-14926: Keep Schema in consistent state where schemaTool fails or succeeds

2016-10-14 Thread Aihua Xu 


> On Oct. 13, 2016, 8:42 p.m., Aihua Xu wrote:
> > beeline/src/java/org/apache/hive/beeline/HiveSchemaTool.java, line 289
> > 
> >
> > Yeah. I thought about changing the error. But we are committing one 
> > script file at a time and you can continue to run from the failed script if 
> > one fails. So for a whole upgrade process, that could be still in half way 
> > (e.g., upgrade from 0.7 to 2.2, failed at 1.3. Then you can try to upgrade 
> > from 1.3 to 2.2). So I intentionally leave like this.
> 
> Peter Vary wrote:
> That is new for me, thanks for the info! Do we know here which scripts 
> are run, and which are not? Then it would be helpful to tell it to the user 
> how to continue the upgrade - Maybe it worth another jira

It will go through the upgrade list, so when you upgrade from 0.1 => 2.2, then 
it will run 0.1=>0.2 script, 0.2=>0.3 script and so on. The log tells you which 
one fails and then you can continue from there. So it won't fail inside the 
script (which is currently what it does and you have to recover the whole data 
and rerun the whole process).


> On Oct. 13, 2016, 8:42 p.m., Aihua Xu wrote:
> > beeline/src/java/org/apache/hive/beeline/HiveSchemaTool.java, line 214
> > 
> >
> > Now the dry run will execute the scripts but without commit. So it will 
> > get the correct schema version like real run.
> > 
> > Before this change dry run actually only lists the scripts it will 
> > execute.
> 
> Peter Vary wrote:
> We should not forget to change the documentation then to match the 
> feature change:
> https://cwiki.apache.org/confluence/display/Hive/Hive+Schema+Tool

Sure. Will do that after the change.


- Aihua


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


On Oct. 13, 2016, 8:43 p.m., Aihua Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52835/
> ---
> 
> (Updated Oct. 13, 2016, 8:43 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-14926: Keep Schema in consistent state where schemaTool fails or succeeds
> 
> 
> Diffs
> -
> 
>   beeline/src/java/org/apache/hive/beeline/HiveSchemaHelper.java 181f0d2 
>   beeline/src/java/org/apache/hive/beeline/HiveSchemaTool.java cd36ddf 
>   itests/hive-unit/src/test/java/org/apache/hive/beeline/TestSchemaTool.java 
> 0d5f9c8 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreSchemaInfo.java 
> 9c30ee7 
> 
> Diff: https://reviews.apache.org/r/52835/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Aihua Xu
> 
>

Re: Review Request 52835: HIVE-14926: Keep Schema in consistent state where schemaTool fails or succeeds

2016-10-14 Thread Peter Vary 


> On Oct. 13, 2016, 8:42 p.m., Aihua Xu wrote:
> > beeline/src/java/org/apache/hive/beeline/HiveSchemaTool.java, line 289
> > 
> >
> > Yeah. I thought about changing the error. But we are committing one 
> > script file at a time and you can continue to run from the failed script if 
> > one fails. So for a whole upgrade process, that could be still in half way 
> > (e.g., upgrade from 0.7 to 2.2, failed at 1.3. Then you can try to upgrade 
> > from 1.3 to 2.2). So I intentionally leave like this.

That is new for me, thanks for the info! Do we know here which scripts are run, 
and which are not? Then it would be helpful to tell it to the user how to 
continue the upgrade - Maybe it worth another jira


> On Oct. 13, 2016, 8:42 p.m., Aihua Xu wrote:
> > beeline/src/java/org/apache/hive/beeline/HiveSchemaTool.java, line 214
> > 
> >
> > Now the dry run will execute the scripts but without commit. So it will 
> > get the correct schema version like real run.
> > 
> > Before this change dry run actually only lists the scripts it will 
> > execute.

We should not forget to change the documentation then to match the feature 
change:
https://cwiki.apache.org/confluence/display/Hive/Hive+Schema+Tool


- Peter


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


On Oct. 13, 2016, 8:43 p.m., Aihua Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52835/
> ---
> 
> (Updated Oct. 13, 2016, 8:43 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-14926: Keep Schema in consistent state where schemaTool fails or succeeds
> 
> 
> Diffs
> -
> 
>   beeline/src/java/org/apache/hive/beeline/HiveSchemaHelper.java 181f0d2 
>   beeline/src/java/org/apache/hive/beeline/HiveSchemaTool.java cd36ddf 
>   itests/hive-unit/src/test/java/org/apache/hive/beeline/TestSchemaTool.java 
> 0d5f9c8 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreSchemaInfo.java 
> 9c30ee7 
> 
> Diff: https://reviews.apache.org/r/52835/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Aihua Xu
> 
>

Re: Review Request 52835: HIVE-14926: Keep Schema in consistent state where schemaTool fails or succeeds

2016-10-13 Thread Aihua Xu 

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

(Updated Oct. 13, 2016, 8:43 p.m.)


Review request for hive.


Changes
---

Unit tests change.


Repository: hive-git


Description
---

HIVE-14926: Keep Schema in consistent state where schemaTool fails or succeeds


Diffs (updated)
-

  beeline/src/java/org/apache/hive/beeline/HiveSchemaHelper.java 181f0d2 
  beeline/src/java/org/apache/hive/beeline/HiveSchemaTool.java cd36ddf 
  itests/hive-unit/src/test/java/org/apache/hive/beeline/TestSchemaTool.java 
0d5f9c8 
  metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreSchemaInfo.java 
9c30ee7 

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


Testing
---


Thanks,

Aihua Xu

Re: Review Request 52835: HIVE-14926: Keep Schema in consistent state where schemaTool fails or succeeds

2016-10-13 Thread Aihua Xu 

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




beeline/src/java/org/apache/hive/beeline/HiveSchemaTool.java 


Now the dry run will execute the scripts but without commit. So it will get 
the correct schema version like real run.

Before this change dry run actually only lists the scripts it will execute.



beeline/src/java/org/apache/hive/beeline/HiveSchemaTool.java (line 256)


Yeah. I thought about changing the error. But we are committing one script 
file at a time and you can continue to run from the failed script if one fails. 
So for a whole upgrade process, that could be still in half way (e.g., upgrade 
from 0.7 to 2.2, failed at 1.3. Then you can try to upgrade from 1.3 to 2.2). 
So I intentionally leave like this.



beeline/src/java/org/apache/hive/beeline/HiveSchemaTool.java (line 365)


Yeah. Actually I will move to open the connection.



beeline/src/java/org/apache/hive/beeline/HiveSchemaTool.java (line 365)


Yeah. Actually I will move to open the connection.


- Aihua Xu


On Oct. 13, 2016, 3:25 p.m., Aihua Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52835/
> ---
> 
> (Updated Oct. 13, 2016, 3:25 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-14926: Keep Schema in consistent state where schemaTool fails or succeeds
> 
> 
> Diffs
> -
> 
>   beeline/src/java/org/apache/hive/beeline/HiveSchemaHelper.java 
> 181f0d28243cb5f9c4a14fa142b6b94009d77ea4 
>   beeline/src/java/org/apache/hive/beeline/HiveSchemaTool.java 
> cd36ddf3860cf56c7d4a7eadcc5bbb173e93a860 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreSchemaInfo.java 
> 9c30ee7add2eda912ab0a27283d1c0f4c689baee 
> 
> Diff: https://reviews.apache.org/r/52835/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Aihua Xu
> 
>

Re: Review Request 52835: HIVE-14926: Keep Schema in consistent state where schemaTool fails or succeeds

2016-10-13 Thread Peter Vary 

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



Thanks Aihua for your patch.
Only some nits, and questions.

Thanks,
Peter


beeline/src/java/org/apache/hive/beeline/HiveSchemaHelper.java (lines 257 - 258)


nit: I think it would be nicer to change the comment is a little - I was 
trying to locate the buffer for a while, until I found out that it was the 
String List :D



beeline/src/java/org/apache/hive/beeline/HiveSchemaTool.java (line 165)


nit: space



beeline/src/java/org/apache/hive/beeline/HiveSchemaTool.java (line 167)


nit: extra spaces. I see that it was the same before, but rb shows this 
line as a new one so it might be ok to change this.



beeline/src/java/org/apache/hive/beeline/HiveSchemaTool.java 


Is it not a problem, that we check the version if it is a dry run? The 
comment said it was intentional, and it changes previous behavior



beeline/src/java/org/apache/hive/beeline/HiveSchemaTool.java (line 256)


Is the metastore state is really inconsistent? I thought we are trying to 
prevent this, and hopefully rollback solves this.



beeline/src/java/org/apache/hive/beeline/HiveSchemaTool.java (line 365)


Is it possible to set automcommit false to every database which could be 
used as a metastore db in every configuration? Sounds reasonable, since we want 
that feature elsewhere too, but might cause a big headache here if the 
autocommit is true (no dry run, and no rollback in case of faliure)


- Peter Vary


On Oct. 13, 2016, 3:25 p.m., Aihua Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52835/
> ---
> 
> (Updated Oct. 13, 2016, 3:25 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-14926: Keep Schema in consistent state where schemaTool fails or succeeds
> 
> 
> Diffs
> -
> 
>   beeline/src/java/org/apache/hive/beeline/HiveSchemaHelper.java 
> 181f0d28243cb5f9c4a14fa142b6b94009d77ea4 
>   beeline/src/java/org/apache/hive/beeline/HiveSchemaTool.java 
> cd36ddf3860cf56c7d4a7eadcc5bbb173e93a860 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreSchemaInfo.java 
> 9c30ee7add2eda912ab0a27283d1c0f4c689baee 
> 
> Diff: https://reviews.apache.org/r/52835/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Aihua Xu
> 
>