Re: Review Request 72380: HIVE-23207 Create integration tests for TxnManager for different rdbms metastores

2020-04-21 Thread Peter Varga via Review Board 


> On April 17, 2020, 7:03 p.m., Peter Vary wrote:
> > itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestMetaStoreHandler.java
> > Lines 67 (patched)
> > 
> >
> > Why is this needed?

These remained here because of some failing tests, but I was able to fix all of 
those, so I removed it.


- Peter


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


On April 21, 2020, 10:22 a.m., Peter Varga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72380/
> ---
> 
> (Updated April 21, 2020, 10:22 a.m.)
> 
> 
> Review request for hive, Denys Kuzmenko and Zoltan Chovan.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> In the final version the prepDb creates the transactional tables with the 
> init schema (and ignores the others if they exists). The cleanDb resets the 
> database to the starting point. So between the test cases the cleanDb call is 
> enough. If the prepDb is called unneccessary it will just check if the txns 
> table exist and then return, so it will be fast
> 
> 
> Diffs
> -
> 
>   contrib/src/test/queries/clientpositive/url_hook.q 512e579fb8 
>   itests/hive-blobstore/pom.xml 09955c55f3 
>   itests/qtest-accumulo/pom.xml a35d2a8a10 
>   itests/qtest-druid/pom.xml cc0cceff68 
>   itests/qtest-kudu/pom.xml f23399fa37 
>   
> itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CliAdapter.java 
> fcfc79059a 
>   
> itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreBeeLineDriver.java
>  5b08f8b894 
>   
> itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestMetaStoreHandler.java
>  b86d736a89 
>   itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestUtil.java 
> 38e4bac2cc 
>   pom.xml b29c06c69e 
>   ql/pom.xml a0e77a1d32 
>   
> ql/src/test/org/apache/hadoop/hive/metastore/txn/TestCompactionTxnHandler.java
>  15fcfc0e35 
>   ql/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandler.java 
> 1d211857bf 
>   
> ql/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandlerNoConnectionPool.java
>  ebe4880e3a 
>   ql/src/test/org/apache/hadoop/hive/ql/lockmgr/ITestDbTxnManager.java 
> PRE-CREATION 
>   ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java 
> 73d3b91585 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/DatabaseProduct.java
>  3e56ad513c 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnDbUtil.java
>  bb29410e7d 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
>  e7910c1c5d 
>   
> standalone-metastore/metastore-server/src/main/sql/derby/hive-schema-4.0.0.derby.sql
>  1ace9d3ef0 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStoreTxns.java
>  5f3db52c2f 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/rules/Mysql.java
>  c537d95470 
> 
> 
> Diff: https://reviews.apache.org/r/72380/diff/2/
> 
> 
> Testing
> ---
> 
> On my machine the 50 tests in TestDbTxnManager2 on postgres runs under 5 
> minutes.
> 
> 
> Thanks,
> 
> Peter Varga
> 
>

Re: Review Request 72380: HIVE-23207 Create integration tests for TxnManager for different rdbms metastores

2020-04-21 Thread Peter Varga via Review Board 

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

(Updated April 21, 2020, 10:22 a.m.)


Review request for hive, Denys Kuzmenko and Zoltan Chovan.


Repository: hive-git


Description
---

In the final version the prepDb creates the transactional tables with the init 
schema (and ignores the others if they exists). The cleanDb resets the database 
to the starting point. So between the test cases the cleanDb call is enough. If 
the prepDb is called unneccessary it will just check if the txns table exist 
and then return, so it will be fast


Diffs (updated)
-

  contrib/src/test/queries/clientpositive/url_hook.q 512e579fb8 
  itests/hive-blobstore/pom.xml 09955c55f3 
  itests/qtest-accumulo/pom.xml a35d2a8a10 
  itests/qtest-druid/pom.xml cc0cceff68 
  itests/qtest-kudu/pom.xml f23399fa37 
  itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CliAdapter.java 
fcfc79059a 
  
itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreBeeLineDriver.java
 5b08f8b894 
  
itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestMetaStoreHandler.java 
b86d736a89 
  itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestUtil.java 38e4bac2cc 
  pom.xml b29c06c69e 
  ql/pom.xml a0e77a1d32 
  
ql/src/test/org/apache/hadoop/hive/metastore/txn/TestCompactionTxnHandler.java 
15fcfc0e35 
  ql/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandler.java 
1d211857bf 
  
ql/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandlerNoConnectionPool.java
 ebe4880e3a 
  ql/src/test/org/apache/hadoop/hive/ql/lockmgr/ITestDbTxnManager.java 
PRE-CREATION 
  ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java 
73d3b91585 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/DatabaseProduct.java
 3e56ad513c 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnDbUtil.java
 bb29410e7d 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
 e7910c1c5d 
  
standalone-metastore/metastore-server/src/main/sql/derby/hive-schema-4.0.0.derby.sql
 1ace9d3ef0 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStoreTxns.java
 5f3db52c2f 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/rules/Mysql.java
 c537d95470 


Diff: https://reviews.apache.org/r/72380/diff/2/

Changes: https://reviews.apache.org/r/72380/diff/1-2/


Testing
---

On my machine the 50 tests in TestDbTxnManager2 on postgres runs under 5 
minutes.


Thanks,

Peter Varga

Re: Review Request 72380: HIVE-23207 Create integration tests for TxnManager for different rdbms metastores

2020-04-20 Thread Peter Vary via Review Board 


> On ápr. 17, 2020, 7:03 du, Peter Vary wrote:
> > Thanks Peter for the patch!
> > This fix is long overdue!
> > 
> > I do not understand one thing, see below.
> > 
> > Also I would like to ask Denys to confirm, that running the init sqls again 
> > and again will not cause too much overhead in test runtime as he mentioned 
> > in our discussion. (5 min tests are fine, 1 hour tests are not fine :))
> > 
> > Thanks!
> 
> Peter Varga wrote:
> In this version we do not run the init sql again and again. We just run 
> it once, and we just run the cleanDb between tests. There might be some Test 
> classes that is not true, because there are multiple prepDb calls (I solved 
> in in Qtest, TestDbTxnManager2) but in those places it will be just one call 
> to check if the txns table exists, and then we return

Got it. Thanks!


- Peter


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


On ápr. 17, 2020, 3:24 du, Peter Varga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72380/
> ---
> 
> (Updated ápr. 17, 2020, 3:24 du)
> 
> 
> Review request for hive, Denys Kuzmenko and Zoltan Chovan.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> In the final version the prepDb creates the transactional tables with the 
> init schema (and ignores the others if they exists). The cleanDb resets the 
> database to the starting point. So between the test cases the cleanDb call is 
> enough. If the prepDb is called unneccessary it will just check if the txns 
> table exist and then return, so it will be fast
> 
> 
> Diffs
> -
> 
>   itests/hive-blobstore/pom.xml 09955c55f3 
>   itests/qtest-accumulo/pom.xml a35d2a8a10 
>   itests/qtest-druid/pom.xml cc0cceff68 
>   itests/qtest-kudu/pom.xml f23399fa37 
>   
> itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestMetaStoreHandler.java
>  b86d736a89 
>   pom.xml 90e39702a1 
>   ql/pom.xml d1846c9245 
>   
> ql/src/test/org/apache/hadoop/hive/metastore/txn/TestCompactionTxnHandler.java
>  15fcfc0e35 
>   ql/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandler.java 
> 1d211857bf 
>   
> ql/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandlerNoConnectionPool.java
>  ebe4880e3a 
>   ql/src/test/org/apache/hadoop/hive/ql/lockmgr/ITestDbTxnManager.java 
> PRE-CREATION 
>   ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java 
> 73d3b91585 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/DatabaseProduct.java
>  3e56ad513c 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnDbUtil.java
>  a66e16973f 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
>  962a63d418 
>   
> standalone-metastore/metastore-server/src/main/sql/derby/hive-schema-4.0.0.derby.sql
>  1ace9d3ef0 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStoreTxns.java
>  5f3db52c2f 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/rules/Mysql.java
>  c537d95470 
> 
> 
> Diff: https://reviews.apache.org/r/72380/diff/1/
> 
> 
> Testing
> ---
> 
> On my machine the 50 tests in TestDbTxnManager2 on postgres runs under 5 
> minutes.
> 
> 
> Thanks,
> 
> Peter Varga
> 
>

Re: Review Request 72380: HIVE-23207 Create integration tests for TxnManager for different rdbms metastores

2020-04-20 Thread Peter Varga via Review Board 


> On April 17, 2020, 7:03 p.m., Peter Vary wrote:
> > Thanks Peter for the patch!
> > This fix is long overdue!
> > 
> > I do not understand one thing, see below.
> > 
> > Also I would like to ask Denys to confirm, that running the init sqls again 
> > and again will not cause too much overhead in test runtime as he mentioned 
> > in our discussion. (5 min tests are fine, 1 hour tests are not fine :))
> > 
> > Thanks!

In this version we do not run the init sql again and again. We just run it 
once, and we just run the cleanDb between tests. There might be some Test 
classes that is not true, because there are multiple prepDb calls (I solved in 
in Qtest, TestDbTxnManager2) but in those places it will be just one call to 
check if the txns table exists, and then we return


- Peter


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


On April 17, 2020, 3:24 p.m., Peter Varga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72380/
> ---
> 
> (Updated April 17, 2020, 3:24 p.m.)
> 
> 
> Review request for hive, Denys Kuzmenko and Zoltan Chovan.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> In the final version the prepDb creates the transactional tables with the 
> init schema (and ignores the others if they exists). The cleanDb resets the 
> database to the starting point. So between the test cases the cleanDb call is 
> enough. If the prepDb is called unneccessary it will just check if the txns 
> table exist and then return, so it will be fast
> 
> 
> Diffs
> -
> 
>   itests/hive-blobstore/pom.xml 09955c55f3 
>   itests/qtest-accumulo/pom.xml a35d2a8a10 
>   itests/qtest-druid/pom.xml cc0cceff68 
>   itests/qtest-kudu/pom.xml f23399fa37 
>   
> itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestMetaStoreHandler.java
>  b86d736a89 
>   pom.xml 90e39702a1 
>   ql/pom.xml d1846c9245 
>   
> ql/src/test/org/apache/hadoop/hive/metastore/txn/TestCompactionTxnHandler.java
>  15fcfc0e35 
>   ql/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandler.java 
> 1d211857bf 
>   
> ql/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandlerNoConnectionPool.java
>  ebe4880e3a 
>   ql/src/test/org/apache/hadoop/hive/ql/lockmgr/ITestDbTxnManager.java 
> PRE-CREATION 
>   ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java 
> 73d3b91585 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/DatabaseProduct.java
>  3e56ad513c 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnDbUtil.java
>  a66e16973f 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
>  962a63d418 
>   
> standalone-metastore/metastore-server/src/main/sql/derby/hive-schema-4.0.0.derby.sql
>  1ace9d3ef0 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStoreTxns.java
>  5f3db52c2f 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/rules/Mysql.java
>  c537d95470 
> 
> 
> Diff: https://reviews.apache.org/r/72380/diff/1/
> 
> 
> Testing
> ---
> 
> On my machine the 50 tests in TestDbTxnManager2 on postgres runs under 5 
> minutes.
> 
> 
> Thanks,
> 
> Peter Varga
> 
>

Re: Review Request 72380: HIVE-23207 Create integration tests for TxnManager for different rdbms metastores

2020-04-17 Thread Peter Vary via Review Board 

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



Thanks Peter for the patch!
This fix is long overdue!

I do not understand one thing, see below.

Also I would like to ask Denys to confirm, that running the init sqls again and 
again will not cause too much overhead in test runtime as he mentioned in our 
discussion. (5 min tests are fine, 1 hour tests are not fine :))

Thanks!


itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestMetaStoreHandler.java
Lines 67 (patched)


Why is this needed?



itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestMetaStoreHandler.java
Lines 119 (patched)


Why is this needed? Again?


- Peter Vary


On ápr. 17, 2020, 3:24 du, Peter Varga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72380/
> ---
> 
> (Updated ápr. 17, 2020, 3:24 du)
> 
> 
> Review request for hive, Denys Kuzmenko and Zoltan Chovan.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> In the final version the prepDb creates the transactional tables with the 
> init schema (and ignores the others if they exists). The cleanDb resets the 
> database to the starting point. So between the test cases the cleanDb call is 
> enough. If the prepDb is called unneccessary it will just check if the txns 
> table exist and then return, so it will be fast
> 
> 
> Diffs
> -
> 
>   itests/hive-blobstore/pom.xml 09955c55f3 
>   itests/qtest-accumulo/pom.xml a35d2a8a10 
>   itests/qtest-druid/pom.xml cc0cceff68 
>   itests/qtest-kudu/pom.xml f23399fa37 
>   
> itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestMetaStoreHandler.java
>  b86d736a89 
>   pom.xml 90e39702a1 
>   ql/pom.xml d1846c9245 
>   
> ql/src/test/org/apache/hadoop/hive/metastore/txn/TestCompactionTxnHandler.java
>  15fcfc0e35 
>   ql/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandler.java 
> 1d211857bf 
>   
> ql/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandlerNoConnectionPool.java
>  ebe4880e3a 
>   ql/src/test/org/apache/hadoop/hive/ql/lockmgr/ITestDbTxnManager.java 
> PRE-CREATION 
>   ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java 
> 73d3b91585 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/DatabaseProduct.java
>  3e56ad513c 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnDbUtil.java
>  a66e16973f 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
>  962a63d418 
>   
> standalone-metastore/metastore-server/src/main/sql/derby/hive-schema-4.0.0.derby.sql
>  1ace9d3ef0 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStoreTxns.java
>  5f3db52c2f 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/rules/Mysql.java
>  c537d95470 
> 
> 
> Diff: https://reviews.apache.org/r/72380/diff/1/
> 
> 
> Testing
> ---
> 
> On my machine the 50 tests in TestDbTxnManager2 on postgres runs under 5 
> minutes.
> 
> 
> Thanks,
> 
> Peter Varga
> 
>

Review Request 72380: HIVE-23207 Create integration tests for TxnManager for different rdbms metastores

2020-04-17 Thread Peter Varga via Review Board 

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

Review request for hive, Denys Kuzmenko and Zoltan Chovan.


Repository: hive-git


Description
---

In the final version the prepDb creates the transactional tables with the init 
schema (and ignores the others if they exists). The cleanDb resets the database 
to the starting point. So between the test cases the cleanDb call is enough. If 
the prepDb is called unneccessary it will just check if the txns table exist 
and then return, so it will be fast


Diffs
-

  itests/hive-blobstore/pom.xml 09955c55f3 
  itests/qtest-accumulo/pom.xml a35d2a8a10 
  itests/qtest-druid/pom.xml cc0cceff68 
  itests/qtest-kudu/pom.xml f23399fa37 
  
itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestMetaStoreHandler.java 
b86d736a89 
  pom.xml 90e39702a1 
  ql/pom.xml d1846c9245 
  
ql/src/test/org/apache/hadoop/hive/metastore/txn/TestCompactionTxnHandler.java 
15fcfc0e35 
  ql/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandler.java 
1d211857bf 
  
ql/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandlerNoConnectionPool.java
 ebe4880e3a 
  ql/src/test/org/apache/hadoop/hive/ql/lockmgr/ITestDbTxnManager.java 
PRE-CREATION 
  ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java 
73d3b91585 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/DatabaseProduct.java
 3e56ad513c 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnDbUtil.java
 a66e16973f 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
 962a63d418 
  
standalone-metastore/metastore-server/src/main/sql/derby/hive-schema-4.0.0.derby.sql
 1ace9d3ef0 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStoreTxns.java
 5f3db52c2f 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/rules/Mysql.java
 c537d95470 


Diff: https://reviews.apache.org/r/72380/diff/1/


Testing
---

On my machine the 50 tests in TestDbTxnManager2 on postgres runs under 5 
minutes.


Thanks,

Peter Varga