You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Aihua Xu <ax...@cloudera.com> on 2016/10/13 15:25:40 UTC

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

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

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

Posted by Peter Vary <pv...@cloudera.com>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/52835/#comment221535>

    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)
<https://reviews.apache.org/r/52835/#comment221536>

    nit: space



beeline/src/java/org/apache/hive/beeline/HiveSchemaTool.java (line 167)
<https://reviews.apache.org/r/52835/#comment221537>

    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 
<https://reviews.apache.org/r/52835/#comment221539>

    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)
<https://reviews.apache.org/r/52835/#comment221541>

    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)
<https://reviews.apache.org/r/52835/#comment221542>

    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
> 
>


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

Posted by Peter Vary <pv...@cloudera.com>.

> On Oct. 13, 2016, 8:42 p.m., Aihua Xu wrote:
> > beeline/src/java/org/apache/hive/beeline/HiveSchemaTool.java, line 289
> > <https://reviews.apache.org/r/52835/diff/1/?file=1534847#file1534847line289>
> >
> >     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
> > <https://reviews.apache.org/r/52835/diff/1/?file=1534847#file1534847line214>
> >
> >     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

Posted by Aihua Xu <ax...@cloudera.com>.

> On Oct. 13, 2016, 8:42 p.m., Aihua Xu wrote:
> > beeline/src/java/org/apache/hive/beeline/HiveSchemaTool.java, line 289
> > <https://reviews.apache.org/r/52835/diff/1/?file=1534847#file1534847line289>
> >
> >     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
> > <https://reviews.apache.org/r/52835/diff/1/?file=1534847#file1534847line214>
> >
> >     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

Posted by Aihua Xu <ax...@cloudera.com>.
-----------------------------------------------------------
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 
<https://reviews.apache.org/r/52835/#comment221630>

    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)
<https://reviews.apache.org/r/52835/#comment221627>

    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)
<https://reviews.apache.org/r/52835/#comment221628>

    Yeah. Actually I will move to open the connection.



beeline/src/java/org/apache/hive/beeline/HiveSchemaTool.java (line 365)
<https://reviews.apache.org/r/52835/#comment221629>

    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

Posted by Aihua Xu <ax...@cloudera.com>.

> On Oct. 14, 2016, 2:18 p.m., Yongzhi Chen wrote:
> > beeline/src/java/org/apache/hive/beeline/HiveSchemaHelper.java, line 231
> > <https://reviews.apache.org/r/52835/diff/2/?file=1535138#file1535138line231>
> >
> >     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

Posted by Yongzhi Chen <yc...@cloudera.com>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/52835/#comment221767>

    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

Posted by Aihua Xu <ax...@cloudera.com>.

> On Oct. 14, 2016, 3:38 p.m., Aihua Xu wrote:
> > beeline/src/java/org/apache/hive/beeline/HiveSchemaTool.java, line 123
> > <https://reviews.apache.org/r/52835/diff/2/?file=1535139#file1535139line123>
> >
> >     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

Posted by Aihua Xu <ax...@cloudera.com>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/52835/#comment221773>

    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

Posted by Chaoyu Tang <ct...@gmail.com>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/52835/#comment221771>

    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

Posted by Aihua Xu <ax...@cloudera.com>.
-----------------------------------------------------------
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