subject:"Review Request 72388\: HIVE\-23048 Use sequences for TXN

Re: Review Request 72388: HIVE-23048 Use sequences for TXN_ID generation

2020-04-28 Thread Peter Varga via Review Board


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

(Updated April 28, 2020, 3:38 p.m.)


Review request for hive and Peter Vary.


Repository: hive-git


Description
---

* Use sequences for TXN_ID generation.
  * Make it possible to open transactions in parallel
  * drop Oracle 11g support


Diffs (updated)
-

  ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DbTxnManager.java b4dac4346e 
  ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java 37a5862791 
  
ql/src/test/org/apache/hadoop/hive/metastore/txn/TestCompactionTxnHandler.java 
7c8903fbae 
  ql/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandler.java 
3916e88a9d 
  ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands2.java 2c13e8dd03 
  ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands3.java 51b0fa336f 
  ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommandsForMmTable.java 
6525ffc00a 
  ql/src/test/org/apache/hadoop/hive/ql/TxnCommandsBaseForTests.java 1435269ed3 
  
ql/src/test/org/apache/hadoop/hive/ql/lockmgr/DbTxnManagerEndToEndTestBase.java 
PRE-CREATION 
  ql/src/test/org/apache/hadoop/hive/ql/lockmgr/ITestDbTxnManager.java 
a085e9ff6f 
  ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java 
f90396b2a3 
  
ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManagerIsolationProperties.java
 PRE-CREATION 
  ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestInitiator.java 
1151466f8c 
  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
 a874121e12 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/SQLGenerator.java
 49b737ecf9 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java
 2344c2d5f6 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnDbUtil.java
 97a083399a 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
 fe39b0b36e 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnStore.java
 87130a519d 
  
standalone-metastore/metastore-server/src/main/sql/derby/hive-schema-4.0.0.derby.sql
 366b6f02c1 
  
standalone-metastore/metastore-server/src/main/sql/derby/upgrade-3.2.0-to-4.0.0.derby.sql
 8a3cd56658 
  
standalone-metastore/metastore-server/src/main/sql/mssql/hive-schema-4.0.0.mssql.sql
 2e0177723d 
  
standalone-metastore/metastore-server/src/main/sql/mssql/upgrade-3.2.0-to-4.0.0.mssql.sql
 9f3951575b 
  
standalone-metastore/metastore-server/src/main/sql/mysql/hive-schema-4.0.0.mysql.sql
 0512a45cad 
  
standalone-metastore/metastore-server/src/main/sql/mysql/upgrade-3.2.0-to-4.0.0.mysql.sql
 4b82e36ab4 
  
standalone-metastore/metastore-server/src/main/sql/oracle/hive-schema-4.0.0.oracle.sql
 db398e5f66 
  
standalone-metastore/metastore-server/src/main/sql/oracle/upgrade-3.2.0-to-4.0.0.oracle.sql
 1be83fc4a9 
  
standalone-metastore/metastore-server/src/main/sql/postgres/hive-schema-4.0.0.postgres.sql
 e6e30160af 
  
standalone-metastore/metastore-server/src/main/sql/postgres/upgrade-3.2.0-to-4.0.0.postgres.sql
 b90cecb173 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/DbInstallBase.java
 c1a1629548 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/rules/DatabaseRule.java
 a6d22d19ef 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/rules/Oracle.java
 0b070e19ac 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/txn/TestOpenTxn.java
 PRE-CREATION 


Diff: https://reviews.apache.org/r/72388/diff/6/

Changes: https://reviews.apache.org/r/72388/diff/5-6/


Testing
---


Thanks,

Peter Varga

Re: Review Request 72388: HIVE-23048 Use sequences for TXN_ID generation

2020-04-28 Thread Peter Varga via Review Board



> On April 27, 2020, 3:04 p.m., Peter Vary wrote:
> > Fix it and ship it

Thank you for the reviews, fixed the issues and got a green run.


> On April 27, 2020, 3:04 p.m., Peter Vary wrote:
> > standalone-metastore/metastore-server/src/main/sql/oracle/upgrade-3.2.0-to-4.0.0.oracle.sql
> > Lines 76 (patched)
> > 
> >
> > TXN_ID should be 10-1

I left the max committed insert and inserted the 10 value this way if 
some administrator want to fix the db manually after the schematool run, it 
will be easier.


> On April 27, 2020, 3:04 p.m., Peter Vary wrote:
> > standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/rules/Oracle.java
> > Line 27 (original), 27 (patched)
> > 
> >
> > Is this public?

Yes it is.


- Peter


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


On April 24, 2020, 3:40 p.m., Peter Varga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72388/
> ---
> 
> (Updated April 24, 2020, 3:40 p.m.)
> 
> 
> Review request for hive and Peter Vary.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> * Use sequences for TXN_ID generation.
>   * Make it possible to open transactions in parallel
>   * drop Oracle 11g support
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java 
> 37a5862791 
>   
> ql/src/test/org/apache/hadoop/hive/metastore/txn/TestCompactionTxnHandler.java
>  15fcfc0e35 
>   ql/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandler.java 
> f512c1df19 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands2.java 2c13e8dd03 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands3.java 51b0fa336f 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommandsForMmTable.java 
> 6525ffc00a 
>   ql/src/test/org/apache/hadoop/hive/ql/TxnCommandsBaseForTests.java 
> 1435269ed3 
>   
> ql/src/test/org/apache/hadoop/hive/ql/lockmgr/DbTxnManagerEndToEndTestBase.java
>  PRE-CREATION 
>   ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java 
> 497cedd61f 
>   
> ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManagerIsolationProperties.java
>  PRE-CREATION 
>   ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestInitiator.java 
> 1151466f8c 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  a874121e12 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/SQLGenerator.java
>  49b737ecf9 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java
>  2344c2d5f6 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnDbUtil.java
>  385f9d72cd 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
>  4a6fa6f620 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnStore.java
>  87130a519d 
>   
> standalone-metastore/metastore-server/src/main/sql/derby/hive-schema-4.0.0.derby.sql
>  1ace9d3ef0 
>   
> standalone-metastore/metastore-server/src/main/sql/derby/upgrade-3.2.0-to-4.0.0.derby.sql
>  8a3cd56658 
>   
> standalone-metastore/metastore-server/src/main/sql/mssql/hive-schema-4.0.0.mssql.sql
>  2e0177723d 
>   
> standalone-metastore/metastore-server/src/main/sql/mssql/upgrade-3.2.0-to-4.0.0.mssql.sql
>  9f3951575b 
>   
> standalone-metastore/metastore-server/src/main/sql/mysql/hive-schema-4.0.0.mysql.sql
>  0512a45cad 
>   
> standalone-metastore/metastore-server/src/main/sql/mysql/upgrade-3.2.0-to-4.0.0.mysql.sql
>  4b82e36ab4 
>   
> standalone-metastore/metastore-server/src/main/sql/oracle/hive-schema-4.0.0.oracle.sql
>  db398e5f66 
>   
> standalone-metastore/metastore-server/src/main/sql/oracle/upgrade-3.2.0-to-4.0.0.oracle.sql
>  1be83fc4a9 
>   
> standalone-metastore/metastore-server/src/main/sql/postgres/hive-schema-4.0.0.postgres.sql
>  e6e30160af 
>   
> standalone-metastore/metastore-server/src/main/sql/postgres/upgrade-3.2.0-to-4.0.0.postgres.sql
>  b90cecb173 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/DbInstallBase.java
>  c1a1629548 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/rules/DatabaseRule.java
>  a6d22d19ef 
>   
>

Re: Review Request 72388: HIVE-23048 Use sequences for TXN_ID generation

2020-04-27 Thread Peter Vary via Review Board


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


Fix it, then Ship it!




Fix it and ship it


standalone-metastore/metastore-server/src/main/sql/derby/upgrade-3.2.0-to-4.0.0.derby.sql
Lines 72 (patched)


The TXN_ID should be 10-1, otherwise we will try to generate a 
quite big list of openTxns :)



standalone-metastore/metastore-server/src/main/sql/oracle/upgrade-3.2.0-to-4.0.0.oracle.sql
Lines 76 (patched)


TXN_ID should be 10-1



standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/rules/Oracle.java
Line 27 (original), 27 (patched)


Is this public?


- Peter Vary


On ápr. 24, 2020, 3:40 du, Peter Varga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72388/
> ---
> 
> (Updated ápr. 24, 2020, 3:40 du)
> 
> 
> Review request for hive and Peter Vary.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> * Use sequences for TXN_ID generation.
>   * Make it possible to open transactions in parallel
>   * drop Oracle 11g support
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java 
> 37a5862791 
>   
> ql/src/test/org/apache/hadoop/hive/metastore/txn/TestCompactionTxnHandler.java
>  15fcfc0e35 
>   ql/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandler.java 
> f512c1df19 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands2.java 2c13e8dd03 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands3.java 51b0fa336f 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommandsForMmTable.java 
> 6525ffc00a 
>   ql/src/test/org/apache/hadoop/hive/ql/TxnCommandsBaseForTests.java 
> 1435269ed3 
>   
> ql/src/test/org/apache/hadoop/hive/ql/lockmgr/DbTxnManagerEndToEndTestBase.java
>  PRE-CREATION 
>   ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java 
> 497cedd61f 
>   
> ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManagerIsolationProperties.java
>  PRE-CREATION 
>   ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestInitiator.java 
> 1151466f8c 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  a874121e12 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/SQLGenerator.java
>  49b737ecf9 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java
>  2344c2d5f6 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnDbUtil.java
>  385f9d72cd 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
>  4a6fa6f620 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnStore.java
>  87130a519d 
>   
> standalone-metastore/metastore-server/src/main/sql/derby/hive-schema-4.0.0.derby.sql
>  1ace9d3ef0 
>   
> standalone-metastore/metastore-server/src/main/sql/derby/upgrade-3.2.0-to-4.0.0.derby.sql
>  8a3cd56658 
>   
> standalone-metastore/metastore-server/src/main/sql/mssql/hive-schema-4.0.0.mssql.sql
>  2e0177723d 
>   
> standalone-metastore/metastore-server/src/main/sql/mssql/upgrade-3.2.0-to-4.0.0.mssql.sql
>  9f3951575b 
>   
> standalone-metastore/metastore-server/src/main/sql/mysql/hive-schema-4.0.0.mysql.sql
>  0512a45cad 
>   
> standalone-metastore/metastore-server/src/main/sql/mysql/upgrade-3.2.0-to-4.0.0.mysql.sql
>  4b82e36ab4 
>   
> standalone-metastore/metastore-server/src/main/sql/oracle/hive-schema-4.0.0.oracle.sql
>  db398e5f66 
>   
> standalone-metastore/metastore-server/src/main/sql/oracle/upgrade-3.2.0-to-4.0.0.oracle.sql
>  1be83fc4a9 
>   
> standalone-metastore/metastore-server/src/main/sql/postgres/hive-schema-4.0.0.postgres.sql
>  e6e30160af 
>   
> standalone-metastore/metastore-server/src/main/sql/postgres/upgrade-3.2.0-to-4.0.0.postgres.sql
>  b90cecb173 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/DbInstallBase.java
>  c1a1629548 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/rules/DatabaseRule.java
>  a6d22d19ef 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/rules/Oracle.java
>  0b070e19ac 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/txn/TestOpenTxn.java
>  PRE-CREATION 
> 
> 
> Diff:

Re: Review Request 72388: HIVE-23048 Use sequences for TXN_ID generation

2020-04-24 Thread Peter Varga via Review Board



> On April 24, 2020, 3:14 p.m., Peter Vary wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
> > Lines 1453 (patched)
> > 
> >
> > Why is this change?

Just to satisfy the yetus checkstyle, it does not like the else statement 
without an op :)


> On April 24, 2020, 3:14 p.m., Peter Vary wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnStore.java
> > Line 47 (original), 47 (patched)
> > 
> >
> > What is the reason behind this change?

an interface variable will be public static final anyway.


- Peter


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


On April 24, 2020, 3:40 p.m., Peter Varga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72388/
> ---
> 
> (Updated April 24, 2020, 3:40 p.m.)
> 
> 
> Review request for hive and Peter Vary.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> * Use sequences for TXN_ID generation.
>   * Make it possible to open transactions in parallel
>   * drop Oracle 11g support
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java 
> 37a5862791 
>   
> ql/src/test/org/apache/hadoop/hive/metastore/txn/TestCompactionTxnHandler.java
>  15fcfc0e35 
>   ql/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandler.java 
> f512c1df19 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands2.java 2c13e8dd03 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands3.java 51b0fa336f 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommandsForMmTable.java 
> 6525ffc00a 
>   ql/src/test/org/apache/hadoop/hive/ql/TxnCommandsBaseForTests.java 
> 1435269ed3 
>   
> ql/src/test/org/apache/hadoop/hive/ql/lockmgr/DbTxnManagerEndToEndTestBase.java
>  PRE-CREATION 
>   ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java 
> 497cedd61f 
>   
> ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManagerIsolationProperties.java
>  PRE-CREATION 
>   ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestInitiator.java 
> 1151466f8c 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  a874121e12 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/SQLGenerator.java
>  49b737ecf9 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java
>  2344c2d5f6 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnDbUtil.java
>  385f9d72cd 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
>  4a6fa6f620 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnStore.java
>  87130a519d 
>   
> standalone-metastore/metastore-server/src/main/sql/derby/hive-schema-4.0.0.derby.sql
>  1ace9d3ef0 
>   
> standalone-metastore/metastore-server/src/main/sql/derby/upgrade-3.2.0-to-4.0.0.derby.sql
>  8a3cd56658 
>   
> standalone-metastore/metastore-server/src/main/sql/mssql/hive-schema-4.0.0.mssql.sql
>  2e0177723d 
>   
> standalone-metastore/metastore-server/src/main/sql/mssql/upgrade-3.2.0-to-4.0.0.mssql.sql
>  9f3951575b 
>   
> standalone-metastore/metastore-server/src/main/sql/mysql/hive-schema-4.0.0.mysql.sql
>  0512a45cad 
>   
> standalone-metastore/metastore-server/src/main/sql/mysql/upgrade-3.2.0-to-4.0.0.mysql.sql
>  4b82e36ab4 
>   
> standalone-metastore/metastore-server/src/main/sql/oracle/hive-schema-4.0.0.oracle.sql
>  db398e5f66 
>   
> standalone-metastore/metastore-server/src/main/sql/oracle/upgrade-3.2.0-to-4.0.0.oracle.sql
>  1be83fc4a9 
>   
> standalone-metastore/metastore-server/src/main/sql/postgres/hive-schema-4.0.0.postgres.sql
>  e6e30160af 
>   
> standalone-metastore/metastore-server/src/main/sql/postgres/upgrade-3.2.0-to-4.0.0.postgres.sql
>  b90cecb173 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/DbInstallBase.java
>  c1a1629548 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/rules/DatabaseRule.java
>  a6d22d19ef 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/rules/Oracle.java
>  0b070e19ac 
>   
>

Re: Review Request 72388: HIVE-23048 Use sequences for TXN_ID generation

2020-04-24 Thread Peter Varga via Review Board



> On April 24, 2020, 2:16 p.m., Denys Kuzmenko wrote:
> > ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java
> > Lines 1068 (patched)
> > 
> >
> > Is it important? I thought TXNS table is cleaned before test.

you are right, the clean is not neccessary, just to wait for the end of the 
opentimeperiod, fixed.


> On April 24, 2020, 2:16 p.m., Denys Kuzmenko wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java
> > Line 578 (original), 588 (patched)
> > 
> >
> > Shouldn't it be under debug?

fixed


> On April 24, 2020, 2:16 p.m., Denys Kuzmenko wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnDbUtil.java
> > Lines 86 (patched)
> > 
> >
> > I think, in future, we should move out from this class everyting test 
> > related.

Actually, if you look at the javadoc at the class, this class supposed to be 
used only by tests :) But I agree that we shouuld separate the test and the 
production code.


> On April 24, 2020, 2:16 p.m., Denys Kuzmenko wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
> > Lines 228 (patched)
> > 
> >
> > Could we exend TxnStatus enum with char representation?

I have seen the TODO, will do that in a follow up Jira.


> On April 24, 2020, 2:16 p.m., Denys Kuzmenko wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
> > Line 433 (original), 517 (patched)
> > 
> >
> > Should we log here something meaningfull under error level?

Actually the checkRetryable will log every detail neccessary, I don't know why 
the commit and rollback is logged everywhere, but it is quit consistent in this 
class.


> On April 24, 2020, 2:16 p.m., Denys Kuzmenko wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
> > Lines 559 (patched)
> > 
> >
> > Could we extract openTxnLowBoundary calc logic? it's the same as in 
> > getOPenTxnInfo

We talked about this with Peter, I would like to extact the whole logic from 
getOpenTxns and getOpenTXnInfo to remove the duplication, I just need to create 
some inner datastructure, because now both the return values missing somethings 
from the other. I will do that in a next jira.


> On April 24, 2020, 2:16 p.m., Denys Kuzmenko wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
> > Line 575 (original), 681 (patched)
> > 
> >
> > Not very useful log trace

do we want change this everywhere? I would prefer to be consistent in this 
class.


> On April 24, 2020, 2:16 p.m., Denys Kuzmenko wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
> > Lines 696 (patched)
> > 
> >
> > Should we log this under error, otherwise we could miss some issues in 
> > a system?

fixed


> On April 24, 2020, 2:16 p.m., Denys Kuzmenko wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
> > Lines 815 (patched)
> > 
> >
> > Based on method declaration I wouldn't expect it to return something - 
> > list of txnIds, but not mutate state using reference.

fixed


> On April 24, 2020, 2:16 p.m., Denys Kuzmenko wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
> > Lines 891 (patched)
> > 
> >
> > redundant " + "

fixed


- Peter


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


On April 24, 2020, 3:40 p.m., Peter Varga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72388/
> ---
> 
> (Updated April 24, 2020, 3:40 p.m.)
> 
> 
> Review request for hive and Peter Vary.
> 
> 
>

Re: Review Request 72388: HIVE-23048 Use sequences for TXN_ID generation

2020-04-24 Thread Peter Varga via Review Board


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

(Updated April 24, 2020, 3:40 p.m.)


Review request for hive and Peter Vary.


Repository: hive-git


Description
---

* Use sequences for TXN_ID generation.
  * Make it possible to open transactions in parallel
  * drop Oracle 11g support


Diffs (updated)
-

  ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java 37a5862791 
  
ql/src/test/org/apache/hadoop/hive/metastore/txn/TestCompactionTxnHandler.java 
15fcfc0e35 
  ql/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandler.java 
f512c1df19 
  ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands2.java 2c13e8dd03 
  ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands3.java 51b0fa336f 
  ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommandsForMmTable.java 
6525ffc00a 
  ql/src/test/org/apache/hadoop/hive/ql/TxnCommandsBaseForTests.java 1435269ed3 
  
ql/src/test/org/apache/hadoop/hive/ql/lockmgr/DbTxnManagerEndToEndTestBase.java 
PRE-CREATION 
  ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java 
497cedd61f 
  
ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManagerIsolationProperties.java
 PRE-CREATION 
  ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestInitiator.java 
1151466f8c 
  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
 a874121e12 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/SQLGenerator.java
 49b737ecf9 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java
 2344c2d5f6 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnDbUtil.java
 385f9d72cd 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
 4a6fa6f620 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnStore.java
 87130a519d 
  
standalone-metastore/metastore-server/src/main/sql/derby/hive-schema-4.0.0.derby.sql
 1ace9d3ef0 
  
standalone-metastore/metastore-server/src/main/sql/derby/upgrade-3.2.0-to-4.0.0.derby.sql
 8a3cd56658 
  
standalone-metastore/metastore-server/src/main/sql/mssql/hive-schema-4.0.0.mssql.sql
 2e0177723d 
  
standalone-metastore/metastore-server/src/main/sql/mssql/upgrade-3.2.0-to-4.0.0.mssql.sql
 9f3951575b 
  
standalone-metastore/metastore-server/src/main/sql/mysql/hive-schema-4.0.0.mysql.sql
 0512a45cad 
  
standalone-metastore/metastore-server/src/main/sql/mysql/upgrade-3.2.0-to-4.0.0.mysql.sql
 4b82e36ab4 
  
standalone-metastore/metastore-server/src/main/sql/oracle/hive-schema-4.0.0.oracle.sql
 db398e5f66 
  
standalone-metastore/metastore-server/src/main/sql/oracle/upgrade-3.2.0-to-4.0.0.oracle.sql
 1be83fc4a9 
  
standalone-metastore/metastore-server/src/main/sql/postgres/hive-schema-4.0.0.postgres.sql
 e6e30160af 
  
standalone-metastore/metastore-server/src/main/sql/postgres/upgrade-3.2.0-to-4.0.0.postgres.sql
 b90cecb173 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/DbInstallBase.java
 c1a1629548 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/rules/DatabaseRule.java
 a6d22d19ef 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/rules/Oracle.java
 0b070e19ac 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/txn/TestOpenTxn.java
 PRE-CREATION 


Diff: https://reviews.apache.org/r/72388/diff/5/

Changes: https://reviews.apache.org/r/72388/diff/4-5/


Testing
---


Thanks,

Peter Varga

Re: Review Request 72388: HIVE-23048 Use sequences for TXN_ID generation

2020-04-24 Thread Peter Vary via Review Board


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



Overall looks good.
There are some changes which might not be needed.
Please check the access modifiers for functions and decrease it whenever 
possible.

Would be good to see the perf numbers for all of the db-s

Thanks for the patch


standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/SQLGenerator.java
Lines 76 (patched)


Do we need this change?



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Lines 428 (patched)


nit: extra line missing
Why is this needed?



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Lines 847 (patched)


private?



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Lines 910 (patched)


Maybe private?



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Lines 1453 (patched)


Why is this change?



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnStore.java
Line 47 (original), 47 (patched)


What is the reason behind this change?


- Peter Vary


On ápr. 23, 2020, 2:48 du, Peter Varga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72388/
> ---
> 
> (Updated ápr. 23, 2020, 2:48 du)
> 
> 
> Review request for hive and Peter Vary.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> * Use sequences for TXN_ID generation.
>   * Make it possible to open transactions in parallel
>   * drop Oracle 11g support
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java 
> 37a5862791 
>   
> 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/ql/TestTxnCommands2.java 2c13e8dd03 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands3.java 51b0fa336f 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommandsForMmTable.java 
> 6525ffc00a 
>   ql/src/test/org/apache/hadoop/hive/ql/TxnCommandsBaseForTests.java 
> 1435269ed3 
>   
> ql/src/test/org/apache/hadoop/hive/ql/lockmgr/DbTxnManagerEndToEndTestBase.java
>  PRE-CREATION 
>   ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java 
> 73d3b91585 
>   
> ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManagerIsolationProperties.java
>  PRE-CREATION 
>   ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestInitiator.java 
> 1151466f8c 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  a874121e12 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/SQLGenerator.java
>  49b737ecf9 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java
>  2344c2d5f6 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnDbUtil.java
>  385f9d72cd 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
>  d0d0320584 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnStore.java
>  87130a519d 
>   
> standalone-metastore/metastore-server/src/main/sql/derby/hive-schema-4.0.0.derby.sql
>  1ace9d3ef0 
>   
> standalone-metastore/metastore-server/src/main/sql/derby/upgrade-3.2.0-to-4.0.0.derby.sql
>  8a3cd56658 
>   
> standalone-metastore/metastore-server/src/main/sql/mssql/hive-schema-4.0.0.mssql.sql
>  2e0177723d 
>   
> standalone-metastore/metastore-server/src/main/sql/mssql/upgrade-3.2.0-to-4.0.0.mssql.sql
>  9f3951575b 
>   
> standalone-metastore/metastore-server/src/main/sql/mysql/hive-schema-4.0.0.mysql.sql
>  0512a45cad 
>   
> standalone-metastore/metastore-server/src/main/sql/mysql/upgrade-3.2.0-to-4.0.0.mysql.sql
>  4b82e36ab4 
>   
> standalone-metastore/metastore-server/src/main/sql/oracle/hive-schema-4.0.0.oracle.sql
>  db398e5f66 
>   
> standalone-metastore/metastore-server/src/main/sql/oracle/upgrade-3.2.0-to-4.0.0.oracle.sql
>  1be83fc4a9 
>   
>

Re: Review Request 72388: HIVE-23048 Use sequences for TXN_ID generation

2020-04-24 Thread Denys Kuzmenko via Review Board


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




ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java
Lines 1068 (patched)


Is it important? I thought TXNS table is cleaned before test.



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java
Line 578 (original), 588 (patched)


Shouldn't it be under debug?



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnDbUtil.java
Lines 86 (patched)


I think, in future, we should move out from this class everyting test 
related.



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Lines 228 (patched)


Could we exend TxnStatus enum with char representation?



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Line 433 (original), 517 (patched)


Should we log here something meaningfull under error level?



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Lines 559 (patched)


Could we extract openTxnLowBoundary calc logic? it's the same as in 
getOPenTxnInfo



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Line 575 (original), 681 (patched)


Not very useful log trace



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Lines 696 (patched)


Should we log this under error, otherwise we could miss some issues in a 
system?



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Lines 815 (patched)


Based on method declaration I wouldn't expect it to return something - list 
of txnIds, but not mutate state using reference.



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Lines 891 (patched)


redundant " + "


- Denys Kuzmenko


On April 23, 2020, 2:48 p.m., Peter Varga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72388/
> ---
> 
> (Updated April 23, 2020, 2:48 p.m.)
> 
> 
> Review request for hive and Peter Vary.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> * Use sequences for TXN_ID generation.
>   * Make it possible to open transactions in parallel
>   * drop Oracle 11g support
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java 
> 37a5862791 
>   
> 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/ql/TestTxnCommands2.java 2c13e8dd03 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands3.java 51b0fa336f 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommandsForMmTable.java 
> 6525ffc00a 
>   ql/src/test/org/apache/hadoop/hive/ql/TxnCommandsBaseForTests.java 
> 1435269ed3 
>   
> ql/src/test/org/apache/hadoop/hive/ql/lockmgr/DbTxnManagerEndToEndTestBase.java
>  PRE-CREATION 
>   ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java 
> 73d3b91585 
>   
> ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManagerIsolationProperties.java
>  PRE-CREATION 
>   ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestInitiator.java 
> 1151466f8c 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  a874121e12 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/SQLGenerator.java
>  49b737ecf9 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java
>  2344c2d5f6 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnDbUtil.java
>  385f9d72cd 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
>  d0d0320584 
>   
>

Re: Review Request 72388: HIVE-23048 Use sequences for TXN_ID generation

2020-04-23 Thread Peter Varga via Review Board


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

(Updated April 23, 2020, 2:48 p.m.)


Review request for hive and Peter Vary.


Changes
---

Modified the txn lock to use the same connection as the opentxns statements for 
performance gains, for that I had to give up to use table lock on every db, but 
actually the code got simpler.


Repository: hive-git


Description
---

* Use sequences for TXN_ID generation.
  * Make it possible to open transactions in parallel
  * drop Oracle 11g support


Diffs (updated)
-

  ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java 37a5862791 
  
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/ql/TestTxnCommands2.java 2c13e8dd03 
  ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands3.java 51b0fa336f 
  ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommandsForMmTable.java 
6525ffc00a 
  ql/src/test/org/apache/hadoop/hive/ql/TxnCommandsBaseForTests.java 1435269ed3 
  
ql/src/test/org/apache/hadoop/hive/ql/lockmgr/DbTxnManagerEndToEndTestBase.java 
PRE-CREATION 
  ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java 
73d3b91585 
  
ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManagerIsolationProperties.java
 PRE-CREATION 
  ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestInitiator.java 
1151466f8c 
  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
 a874121e12 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/SQLGenerator.java
 49b737ecf9 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java
 2344c2d5f6 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnDbUtil.java
 385f9d72cd 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
 d0d0320584 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnStore.java
 87130a519d 
  
standalone-metastore/metastore-server/src/main/sql/derby/hive-schema-4.0.0.derby.sql
 1ace9d3ef0 
  
standalone-metastore/metastore-server/src/main/sql/derby/upgrade-3.2.0-to-4.0.0.derby.sql
 8a3cd56658 
  
standalone-metastore/metastore-server/src/main/sql/mssql/hive-schema-4.0.0.mssql.sql
 2e0177723d 
  
standalone-metastore/metastore-server/src/main/sql/mssql/upgrade-3.2.0-to-4.0.0.mssql.sql
 9f3951575b 
  
standalone-metastore/metastore-server/src/main/sql/mysql/hive-schema-4.0.0.mysql.sql
 0512a45cad 
  
standalone-metastore/metastore-server/src/main/sql/mysql/upgrade-3.2.0-to-4.0.0.mysql.sql
 4b82e36ab4 
  
standalone-metastore/metastore-server/src/main/sql/oracle/hive-schema-4.0.0.oracle.sql
 db398e5f66 
  
standalone-metastore/metastore-server/src/main/sql/oracle/upgrade-3.2.0-to-4.0.0.oracle.sql
 1be83fc4a9 
  
standalone-metastore/metastore-server/src/main/sql/postgres/hive-schema-4.0.0.postgres.sql
 e6e30160af 
  
standalone-metastore/metastore-server/src/main/sql/postgres/upgrade-3.2.0-to-4.0.0.postgres.sql
 b90cecb173 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/DbInstallBase.java
 c1a1629548 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/rules/DatabaseRule.java
 a6d22d19ef 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/rules/Oracle.java
 0b070e19ac 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/txn/TestOpenTxn.java
 PRE-CREATION 


Diff: https://reviews.apache.org/r/72388/diff/4/

Changes: https://reviews.apache.org/r/72388/diff/3-4/


Testing
---


Thanks,

Peter Varga

Re: Review Request 72388: HIVE-23048 Use sequences for TXN_ID generation

2020-04-21 Thread Peter Varga via Review Board



> On April 20, 2020, 10:40 a.m., Denys Kuzmenko wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
> > Lines 902 (patched)
> > 
> >
> > Can it be null? Could we add constraint on db level instead of multiple 
> > checks in backend code?
> 
> Peter Varga wrote:
> It should not be null, but the constraint is that there must be always 
> one record in the table, not a nonnull column.
> 
> Denys Kuzmenko wrote:
> ok, don't you cover this with if (!maxTxnIdRs.next()) { .. } ?
> 
> Peter Varga wrote:
> No, if there is no record, the max will return one row with a null value
> 
> Denys Kuzmenko wrote:
> ok, got it, in this case why are we doing throw new 
> IllegalStateException("Scalar query returned no rows?!?!!")?

removed that.


- Peter


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


On April 21, 2020, 12:51 p.m., Peter Varga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72388/
> ---
> 
> (Updated April 21, 2020, 12:51 p.m.)
> 
> 
> Review request for hive and Peter Vary.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> * Use sequences for TXN_ID generation.
>   * Make it possible to open transactions in parallel
>   * drop Oracle 11g support
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java 
> 37a5862791 
>   
> 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/ql/TestTxnCommands2.java 2c13e8dd03 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands3.java 51b0fa336f 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommandsForMmTable.java 
> 6525ffc00a 
>   ql/src/test/org/apache/hadoop/hive/ql/TxnCommandsBaseForTests.java 
> 1435269ed3 
>   
> ql/src/test/org/apache/hadoop/hive/ql/lockmgr/DbTxnManagerEndToEndTestBase.java
>  PRE-CREATION 
>   ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java 
> 73d3b91585 
>   
> ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManagerIsolationProperties.java
>  PRE-CREATION 
>   ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestInitiator.java 
> 1151466f8c 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  a874121e12 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/SQLGenerator.java
>  49b737ecf9 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java
>  2344c2d5f6 
>   
> 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/java/org/apache/hadoop/hive/metastore/txn/TxnStore.java
>  87130a519d 
>   
> standalone-metastore/metastore-server/src/main/sql/derby/hive-schema-4.0.0.derby.sql
>  1ace9d3ef0 
>   
> standalone-metastore/metastore-server/src/main/sql/derby/upgrade-3.2.0-to-4.0.0.derby.sql
>  8a3cd56658 
>   
> standalone-metastore/metastore-server/src/main/sql/mssql/hive-schema-4.0.0.mssql.sql
>  2e0177723d 
>   
> standalone-metastore/metastore-server/src/main/sql/mssql/upgrade-3.2.0-to-4.0.0.mssql.sql
>  9f3951575b 
>   
> standalone-metastore/metastore-server/src/main/sql/mysql/hive-schema-4.0.0.mysql.sql
>  0512a45cad 
>   
> standalone-metastore/metastore-server/src/main/sql/mysql/upgrade-3.2.0-to-4.0.0.mysql.sql
>  4b82e36ab4 
>   
> standalone-metastore/metastore-server/src/main/sql/oracle/hive-schema-4.0.0.oracle.sql
>  db398e5f66 
>   
> standalone-metastore/metastore-server/src/main/sql/oracle/upgrade-3.2.0-to-4.0.0.oracle.sql
>  1be83fc4a9 
>   
> standalone-metastore/metastore-server/src/main/sql/postgres/hive-schema-4.0.0.postgres.sql
>  e6e30160af 
>   
> standalone-metastore/metastore-server/src/main/sql/postgres/upgrade-3.2.0-to-4.0.0.postgres.sql
>  b90cecb173 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/DbInstallBase.java
>  c1a1629548 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/rules/DatabaseRule.java
>  a6d22d19ef 
>   
>

Re: Review Request 72388: HIVE-23048 Use sequences for TXN_ID generation

2020-04-21 Thread Peter Varga via Review Board



> On April 20, 2020, 12:19 p.m., Peter Vary wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
> > Lines 699 (patched)
> > 
> >
> > openTxns(dbConn, stmt, rqst) is used in replTableWriteIdState as well. 
> > Do we have to check there for timeout as well?
> 
> Peter Varga wrote:
> This is an alarming catch. What kind of queries run on a replication. I 
> put there the table lock, but it will be a rather long synchronized block. On 
> the other hand aren't the replication tasks somehow serialized? If there are 
> no select queries on replication the timeout is not needed, if there are, we 
> might have a problem, because we do a lot of think before the commit, where 
> we can measure.
> And the most seriuos problem, if I see correctly, the replication will 
> use this newly opened transactionId-s to write into the TXN_TO_WRITE_ID, 
> wouldn't be a problem if this will be different than the original value?

As we discussed, the txn_id change is not a problem, and we don't need either 
the timeout check or the txnLock, because all of the opened transactions will 
be aborted in the same rdbms transaction


> On April 20, 2020, 12:19 p.m., Peter Vary wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
> > Lines 780 (patched)
> > 
> >
> > TXN_META_INFO? What is it used for before? Or is it new? Could we use a 
> > specific "state" for example?
> 
> Denys Kuzmenko wrote:
> My understanding was, it's kinda marker to flag the new txn entries and 
> get their ids.
> 
> Peter Varga wrote:
> It is a marker, I just used an existing field. It is updated to null 
> later.

change it to txn_state


> On April 20, 2020, 12:19 p.m., Peter Vary wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
> > Line 641 (original), 783 (patched)
> > 
> >
> > Could we use a correct PreparedStatement, with the values set without 
> > the "hacky" quoting?
> > 
> > Like: pstmt.setXX for state/user/host/type/metainfo?
> > 
> > Or does this have a noticable performance gain?
> 
> Peter Varga wrote:
> Changed the metainfo select back and update to preparedstatements. The 
> original insert will entirely change in your batch insert patch if I know 
> correctly, so I will leave that.

incorporated your batchinsert patch also


- Peter


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


On April 21, 2020, 12:51 p.m., Peter Varga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72388/
> ---
> 
> (Updated April 21, 2020, 12:51 p.m.)
> 
> 
> Review request for hive and Peter Vary.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> * Use sequences for TXN_ID generation.
>   * Make it possible to open transactions in parallel
>   * drop Oracle 11g support
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java 
> 37a5862791 
>   
> 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/ql/TestTxnCommands2.java 2c13e8dd03 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands3.java 51b0fa336f 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommandsForMmTable.java 
> 6525ffc00a 
>   ql/src/test/org/apache/hadoop/hive/ql/TxnCommandsBaseForTests.java 
> 1435269ed3 
>   
> ql/src/test/org/apache/hadoop/hive/ql/lockmgr/DbTxnManagerEndToEndTestBase.java
>  PRE-CREATION 
>   ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java 
> 73d3b91585 
>   
> ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManagerIsolationProperties.java
>  PRE-CREATION 
>   ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestInitiator.java 
> 1151466f8c 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  a874121e12 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/SQLGenerator.java
>  49b737ecf9 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java
>  2344c2d5f6 
>   
>

Re: Review Request 72388: HIVE-23048 Use sequences for TXN_ID generation

2020-04-21 Thread Peter Varga via Review Board


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

(Updated April 21, 2020, 12:51 p.m.)


Review request for hive and Peter Vary.


Repository: hive-git


Description
---

* Use sequences for TXN_ID generation.
  * Make it possible to open transactions in parallel
  * drop Oracle 11g support


Diffs (updated)
-

  ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java 37a5862791 
  
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/ql/TestTxnCommands2.java 2c13e8dd03 
  ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands3.java 51b0fa336f 
  ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommandsForMmTable.java 
6525ffc00a 
  ql/src/test/org/apache/hadoop/hive/ql/TxnCommandsBaseForTests.java 1435269ed3 
  
ql/src/test/org/apache/hadoop/hive/ql/lockmgr/DbTxnManagerEndToEndTestBase.java 
PRE-CREATION 
  ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java 
73d3b91585 
  
ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManagerIsolationProperties.java
 PRE-CREATION 
  ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestInitiator.java 
1151466f8c 
  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
 a874121e12 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/SQLGenerator.java
 49b737ecf9 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java
 2344c2d5f6 
  
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/java/org/apache/hadoop/hive/metastore/txn/TxnStore.java
 87130a519d 
  
standalone-metastore/metastore-server/src/main/sql/derby/hive-schema-4.0.0.derby.sql
 1ace9d3ef0 
  
standalone-metastore/metastore-server/src/main/sql/derby/upgrade-3.2.0-to-4.0.0.derby.sql
 8a3cd56658 
  
standalone-metastore/metastore-server/src/main/sql/mssql/hive-schema-4.0.0.mssql.sql
 2e0177723d 
  
standalone-metastore/metastore-server/src/main/sql/mssql/upgrade-3.2.0-to-4.0.0.mssql.sql
 9f3951575b 
  
standalone-metastore/metastore-server/src/main/sql/mysql/hive-schema-4.0.0.mysql.sql
 0512a45cad 
  
standalone-metastore/metastore-server/src/main/sql/mysql/upgrade-3.2.0-to-4.0.0.mysql.sql
 4b82e36ab4 
  
standalone-metastore/metastore-server/src/main/sql/oracle/hive-schema-4.0.0.oracle.sql
 db398e5f66 
  
standalone-metastore/metastore-server/src/main/sql/oracle/upgrade-3.2.0-to-4.0.0.oracle.sql
 1be83fc4a9 
  
standalone-metastore/metastore-server/src/main/sql/postgres/hive-schema-4.0.0.postgres.sql
 e6e30160af 
  
standalone-metastore/metastore-server/src/main/sql/postgres/upgrade-3.2.0-to-4.0.0.postgres.sql
 b90cecb173 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/DbInstallBase.java
 c1a1629548 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/rules/DatabaseRule.java
 a6d22d19ef 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/rules/Oracle.java
 0b070e19ac 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/txn/TestOpenTxn.java
 PRE-CREATION 


Diff: https://reviews.apache.org/r/72388/diff/3/

Changes: https://reviews.apache.org/r/72388/diff/2-3/


Testing
---


Thanks,

Peter Varga

Re: Review Request 72388: HIVE-23048 Use sequences for TXN_ID generation

2020-04-21 Thread Denys Kuzmenko via Review Board



> On April 20, 2020, 10:40 a.m., Denys Kuzmenko wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
> > Lines 902 (patched)
> > 
> >
> > Can it be null? Could we add constraint on db level instead of multiple 
> > checks in backend code?
> 
> Peter Varga wrote:
> It should not be null, but the constraint is that there must be always 
> one record in the table, not a nonnull column.
> 
> Denys Kuzmenko wrote:
> ok, don't you cover this with if (!maxTxnIdRs.next()) { .. } ?
> 
> Peter Varga wrote:
> No, if there is no record, the max will return one row with a null value

ok, got it, in this case why are we doing throw new 
IllegalStateException("Scalar query returned no rows?!?!!")?


- Denys


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


On April 21, 2020, 7:53 a.m., Peter Varga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72388/
> ---
> 
> (Updated April 21, 2020, 7:53 a.m.)
> 
> 
> Review request for hive and Peter Vary.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> * Use sequences for TXN_ID generation.
>   * Make it possible to open transactions in parallel
>   * drop Oracle 11g support
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java 
> 37a5862791 
>   
> 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/ql/TestTxnCommands2.java 2c13e8dd03 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands3.java 51b0fa336f 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommandsForMmTable.java 
> 6525ffc00a 
>   ql/src/test/org/apache/hadoop/hive/ql/TxnCommandsBaseForTests.java 
> 1435269ed3 
>   
> ql/src/test/org/apache/hadoop/hive/ql/lockmgr/DbTxnManagerEndToEndTestBase.java
>  PRE-CREATION 
>   ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java 
> 73d3b91585 
>   
> ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManagerIsolationProperties.java
>  PRE-CREATION 
>   ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestInitiator.java 
> 1151466f8c 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  a874121e12 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/SQLGenerator.java
>  49b737ecf9 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java
>  2344c2d5f6 
>   
> 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/java/org/apache/hadoop/hive/metastore/txn/TxnStore.java
>  87130a519d 
>   
> standalone-metastore/metastore-server/src/main/sql/derby/hive-schema-4.0.0.derby.sql
>  1ace9d3ef0 
>   
> standalone-metastore/metastore-server/src/main/sql/derby/upgrade-3.2.0-to-4.0.0.derby.sql
>  8a3cd56658 
>   
> standalone-metastore/metastore-server/src/main/sql/mssql/hive-schema-4.0.0.mssql.sql
>  2e0177723d 
>   
> standalone-metastore/metastore-server/src/main/sql/mssql/upgrade-3.2.0-to-4.0.0.mssql.sql
>  9f3951575b 
>   
> standalone-metastore/metastore-server/src/main/sql/mysql/hive-schema-4.0.0.mysql.sql
>  0512a45cad 
>   
> standalone-metastore/metastore-server/src/main/sql/mysql/upgrade-3.2.0-to-4.0.0.mysql.sql
>  4b82e36ab4 
>   
> standalone-metastore/metastore-server/src/main/sql/oracle/hive-schema-4.0.0.oracle.sql
>  db398e5f66 
>   
> standalone-metastore/metastore-server/src/main/sql/oracle/upgrade-3.2.0-to-4.0.0.oracle.sql
>  1be83fc4a9 
>   
> standalone-metastore/metastore-server/src/main/sql/postgres/hive-schema-4.0.0.postgres.sql
>  e6e30160af 
>   
> standalone-metastore/metastore-server/src/main/sql/postgres/upgrade-3.2.0-to-4.0.0.postgres.sql
>  b90cecb173 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/DbInstallBase.java
>  c1a1629548 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/rules/DatabaseRule.java
>  a6d22d19ef 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/rules/Oracle.java
>  0b070e19ac 
>   
>

Re: Review Request 72388: HIVE-23048 Use sequences for TXN_ID generation

2020-04-21 Thread Peter Varga via Review Board



> On April 20, 2020, 10:40 a.m., Denys Kuzmenko wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
> > Lines 902 (patched)
> > 
> >
> > Can it be null? Could we add constraint on db level instead of multiple 
> > checks in backend code?
> 
> Peter Varga wrote:
> It should not be null, but the constraint is that there must be always 
> one record in the table, not a nonnull column.
> 
> Denys Kuzmenko wrote:
> ok, don't you cover this with if (!maxTxnIdRs.next()) { .. } ?

No, if there is no record, the max will return one row with a null value


- Peter


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


On April 21, 2020, 7:53 a.m., Peter Varga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72388/
> ---
> 
> (Updated April 21, 2020, 7:53 a.m.)
> 
> 
> Review request for hive and Peter Vary.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> * Use sequences for TXN_ID generation.
>   * Make it possible to open transactions in parallel
>   * drop Oracle 11g support
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java 
> 37a5862791 
>   
> 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/ql/TestTxnCommands2.java 2c13e8dd03 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands3.java 51b0fa336f 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommandsForMmTable.java 
> 6525ffc00a 
>   ql/src/test/org/apache/hadoop/hive/ql/TxnCommandsBaseForTests.java 
> 1435269ed3 
>   
> ql/src/test/org/apache/hadoop/hive/ql/lockmgr/DbTxnManagerEndToEndTestBase.java
>  PRE-CREATION 
>   ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java 
> 73d3b91585 
>   
> ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManagerIsolationProperties.java
>  PRE-CREATION 
>   ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestInitiator.java 
> 1151466f8c 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  a874121e12 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/SQLGenerator.java
>  49b737ecf9 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java
>  2344c2d5f6 
>   
> 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/java/org/apache/hadoop/hive/metastore/txn/TxnStore.java
>  87130a519d 
>   
> standalone-metastore/metastore-server/src/main/sql/derby/hive-schema-4.0.0.derby.sql
>  1ace9d3ef0 
>   
> standalone-metastore/metastore-server/src/main/sql/derby/upgrade-3.2.0-to-4.0.0.derby.sql
>  8a3cd56658 
>   
> standalone-metastore/metastore-server/src/main/sql/mssql/hive-schema-4.0.0.mssql.sql
>  2e0177723d 
>   
> standalone-metastore/metastore-server/src/main/sql/mssql/upgrade-3.2.0-to-4.0.0.mssql.sql
>  9f3951575b 
>   
> standalone-metastore/metastore-server/src/main/sql/mysql/hive-schema-4.0.0.mysql.sql
>  0512a45cad 
>   
> standalone-metastore/metastore-server/src/main/sql/mysql/upgrade-3.2.0-to-4.0.0.mysql.sql
>  4b82e36ab4 
>   
> standalone-metastore/metastore-server/src/main/sql/oracle/hive-schema-4.0.0.oracle.sql
>  db398e5f66 
>   
> standalone-metastore/metastore-server/src/main/sql/oracle/upgrade-3.2.0-to-4.0.0.oracle.sql
>  1be83fc4a9 
>   
> standalone-metastore/metastore-server/src/main/sql/postgres/hive-schema-4.0.0.postgres.sql
>  e6e30160af 
>   
> standalone-metastore/metastore-server/src/main/sql/postgres/upgrade-3.2.0-to-4.0.0.postgres.sql
>  b90cecb173 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/DbInstallBase.java
>  c1a1629548 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/rules/DatabaseRule.java
>  a6d22d19ef 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/rules/Oracle.java
>  0b070e19ac 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/txn/TestOpenTxn.java
>  PRE-CREATION 
> 
> 
> Diff:

Re: Review Request 72388: HIVE-23048 Use sequences for TXN_ID generation

2020-04-21 Thread Denys Kuzmenko via Review Board



> On April 20, 2020, 10:40 a.m., Denys Kuzmenko wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
> > Line 172 (original), 225 (patched)
> > 
> >
> > Please rebase, this code is no longer here
> 
> Peter Varga wrote:
> which commit removed this? I don't see it.

oh, sorry, I confused it with the lock type. Not an issue ignore.


> On April 20, 2020, 10:40 a.m., Denys Kuzmenko wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
> > Lines 902 (patched)
> > 
> >
> > Can it be null? Could we add constraint on db level instead of multiple 
> > checks in backend code?
> 
> Peter Varga wrote:
> It should not be null, but the constraint is that there must be always 
> one record in the table, not a nonnull column.

ok, don't you cover this with if (!maxTxnIdRs.next()) { .. } ?


- Denys


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


On April 21, 2020, 7:53 a.m., Peter Varga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72388/
> ---
> 
> (Updated April 21, 2020, 7:53 a.m.)
> 
> 
> Review request for hive and Peter Vary.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> * Use sequences for TXN_ID generation.
>   * Make it possible to open transactions in parallel
>   * drop Oracle 11g support
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java 
> 37a5862791 
>   
> 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/ql/TestTxnCommands2.java 2c13e8dd03 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands3.java 51b0fa336f 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommandsForMmTable.java 
> 6525ffc00a 
>   ql/src/test/org/apache/hadoop/hive/ql/TxnCommandsBaseForTests.java 
> 1435269ed3 
>   
> ql/src/test/org/apache/hadoop/hive/ql/lockmgr/DbTxnManagerEndToEndTestBase.java
>  PRE-CREATION 
>   ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java 
> 73d3b91585 
>   
> ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManagerIsolationProperties.java
>  PRE-CREATION 
>   ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestInitiator.java 
> 1151466f8c 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  a874121e12 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/SQLGenerator.java
>  49b737ecf9 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java
>  2344c2d5f6 
>   
> 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/java/org/apache/hadoop/hive/metastore/txn/TxnStore.java
>  87130a519d 
>   
> standalone-metastore/metastore-server/src/main/sql/derby/hive-schema-4.0.0.derby.sql
>  1ace9d3ef0 
>   
> standalone-metastore/metastore-server/src/main/sql/derby/upgrade-3.2.0-to-4.0.0.derby.sql
>  8a3cd56658 
>   
> standalone-metastore/metastore-server/src/main/sql/mssql/hive-schema-4.0.0.mssql.sql
>  2e0177723d 
>   
> standalone-metastore/metastore-server/src/main/sql/mssql/upgrade-3.2.0-to-4.0.0.mssql.sql
>  9f3951575b 
>   
> standalone-metastore/metastore-server/src/main/sql/mysql/hive-schema-4.0.0.mysql.sql
>  0512a45cad 
>   
> standalone-metastore/metastore-server/src/main/sql/mysql/upgrade-3.2.0-to-4.0.0.mysql.sql
>  4b82e36ab4 
>   
> standalone-metastore/metastore-server/src/main/sql/oracle/hive-schema-4.0.0.oracle.sql
>  db398e5f66 
>   
> standalone-metastore/metastore-server/src/main/sql/oracle/upgrade-3.2.0-to-4.0.0.oracle.sql
>  1be83fc4a9 
>   
> standalone-metastore/metastore-server/src/main/sql/postgres/hive-schema-4.0.0.postgres.sql
>  e6e30160af 
>   
> standalone-metastore/metastore-server/src/main/sql/postgres/upgrade-3.2.0-to-4.0.0.postgres.sql
>  b90cecb173 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/DbInstallBase.java
>  c1a1629548 
>   
>

Re: Review Request 72388: HIVE-23048 Use sequences for TXN_ID generation

2020-04-21 Thread Peter Varga via Review Board


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

(Updated April 21, 2020, 7:53 a.m.)


Review request for hive and Peter Vary.


Repository: hive-git


Description
---

* Use sequences for TXN_ID generation.
  * Make it possible to open transactions in parallel
  * drop Oracle 11g support


Diffs (updated)
-

  ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java 37a5862791 
  
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/ql/TestTxnCommands2.java 2c13e8dd03 
  ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands3.java 51b0fa336f 
  ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommandsForMmTable.java 
6525ffc00a 
  ql/src/test/org/apache/hadoop/hive/ql/TxnCommandsBaseForTests.java 1435269ed3 
  
ql/src/test/org/apache/hadoop/hive/ql/lockmgr/DbTxnManagerEndToEndTestBase.java 
PRE-CREATION 
  ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java 
73d3b91585 
  
ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManagerIsolationProperties.java
 PRE-CREATION 
  ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestInitiator.java 
1151466f8c 
  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
 a874121e12 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/SQLGenerator.java
 49b737ecf9 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java
 2344c2d5f6 
  
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/java/org/apache/hadoop/hive/metastore/txn/TxnStore.java
 87130a519d 
  
standalone-metastore/metastore-server/src/main/sql/derby/hive-schema-4.0.0.derby.sql
 1ace9d3ef0 
  
standalone-metastore/metastore-server/src/main/sql/derby/upgrade-3.2.0-to-4.0.0.derby.sql
 8a3cd56658 
  
standalone-metastore/metastore-server/src/main/sql/mssql/hive-schema-4.0.0.mssql.sql
 2e0177723d 
  
standalone-metastore/metastore-server/src/main/sql/mssql/upgrade-3.2.0-to-4.0.0.mssql.sql
 9f3951575b 
  
standalone-metastore/metastore-server/src/main/sql/mysql/hive-schema-4.0.0.mysql.sql
 0512a45cad 
  
standalone-metastore/metastore-server/src/main/sql/mysql/upgrade-3.2.0-to-4.0.0.mysql.sql
 4b82e36ab4 
  
standalone-metastore/metastore-server/src/main/sql/oracle/hive-schema-4.0.0.oracle.sql
 db398e5f66 
  
standalone-metastore/metastore-server/src/main/sql/oracle/upgrade-3.2.0-to-4.0.0.oracle.sql
 1be83fc4a9 
  
standalone-metastore/metastore-server/src/main/sql/postgres/hive-schema-4.0.0.postgres.sql
 e6e30160af 
  
standalone-metastore/metastore-server/src/main/sql/postgres/upgrade-3.2.0-to-4.0.0.postgres.sql
 b90cecb173 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/DbInstallBase.java
 c1a1629548 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/rules/DatabaseRule.java
 a6d22d19ef 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/rules/Oracle.java
 0b070e19ac 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/txn/TestOpenTxn.java
 PRE-CREATION 


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

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


Testing
---


Thanks,

Peter Varga

Re: Review Request 72388: HIVE-23048 Use sequences for TXN_ID generation

2020-04-21 Thread Peter Varga via Review Board



> On April 20, 2020, 12:19 p.m., Peter Vary wrote:
> > This is a really big/scary change. I am really interested in the 
> > performance results! :)
> > Thanks for all the effort! Some questions below

Thank you for the review, i fixed most of them, but have a few questions.


> On April 20, 2020, 12:19 p.m., Peter Vary wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java
> > Lines 563 (patched)
> > 
> >
> > Maybe insert the query instead of getting id and using again. Not very 
> > important, just asking...

I tried it at first, but it become really slow, when there are 5 txns in 
the table. Probably there is a way to do it properly, but this is running in a 
scheduled thread so it is not that essential I think.


> On April 20, 2020, 12:19 p.m., Peter Vary wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
> > Line 448 (original), 528 (patched)
> > 
> >
> > Just a question: This is so similar to getOpenTxnsInfo... Any way to 
> > use the same code with different query and different response handling?

I started it but I think we should do it in a next change, we have to create 
some inner datastructure, because both of responses now missing something from 
the other


> On April 20, 2020, 12:19 p.m., Peter Vary wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
> > Lines 699 (patched)
> > 
> >
> > openTxns(dbConn, stmt, rqst) is used in replTableWriteIdState as well. 
> > Do we have to check there for timeout as well?

This is an alarming catch. What kind of queries run on a replication. I put 
there the table lock, but it will be a rather long synchronized block. On the 
other hand aren't the replication tasks somehow serialized? If there are no 
select queries on replication the timeout is not needed, if there are, we might 
have a problem, because we do a lot of think before the commit, where we can 
measure.
And the most seriuos problem, if I see correctly, the replication will use this 
newly opened transactionId-s to write into the TXN_TO_WRITE_ID, wouldn't be a 
problem if this will be different than the original value?


> On April 20, 2020, 12:19 p.m., Peter Vary wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
> > Lines 780 (patched)
> > 
> >
> > TXN_META_INFO? What is it used for before? Or is it new? Could we use a 
> > specific "state" for example?
> 
> Denys Kuzmenko wrote:
> My understanding was, it's kinda marker to flag the new txn entries and 
> get their ids.

It is a marker, I just used an existing field. It is updated to null later.


> On April 20, 2020, 12:19 p.m., Peter Vary wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
> > Line 641 (original), 783 (patched)
> > 
> >
> > Could we use a correct PreparedStatement, with the values set without 
> > the "hacky" quoting?
> > 
> > Like: pstmt.setXX for state/user/host/type/metainfo?
> > 
> > Or does this have a noticable performance gain?

Changed the metainfo select back and update to preparedstatements. The original 
insert will entirely change in your batch insert patch if I know correctly, so 
I will leave that.


> On April 20, 2020, 12:19 p.m., Peter Vary wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
> > Line 1168 (original), 1418 (patched)
> > 
> >
> > If we do not make such a fuss about the checking, just a simple assert 
> > instead, we might can inline the method here, since it is not used anywhere 
> > else? What do you think?

The commitTxns is already 200 lines long, if it's ok for you, i woudn't make it 
longer


> On April 20, 2020, 12:19 p.m., Peter Vary wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
> > Lines 2262 (patched)
> > 
> >
> > This is again a single query implemented with 3 SQL query and a java 
> > code. Am I right?

Yes, however it is a backgroung method. Does it worth to sacrifice the easier 
logging and debugging for some performance gain?


- Peter


---
This is an automatically generated e-mail. To

Re: Review Request 72388: HIVE-23048 Use sequences for TXN_ID generation

2020-04-21 Thread Peter Varga via Review Board



> On April 20, 2020, 10:40 a.m., Denys Kuzmenko wrote:
> > LGTM, just a few comments

Thank you for the review, I fixed most of the issues, but have a few question.


> On April 20, 2020, 10:40 a.m., Denys Kuzmenko wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java
> > Line 552 (original), 560 (patched)
> > 
> >
> > Could we use this query as condition for delete? selete where exists 
> > (..)

I would rather not do that, this method is called from a scheduled backgroung 
thread, it don't have to be incredible fast, on the other hand, if we run it 
this way we log the txnid-s between the select and the delete. I would like to 
keep that.


> On April 20, 2020, 10:40 a.m., Denys Kuzmenko wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
> > Line 172 (original), 225 (patched)
> > 
> >
> > Please rebase, this code is no longer here

which commit removed this? I don't see it.


> On April 20, 2020, 10:40 a.m., Denys Kuzmenko wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
> > Lines 807 (patched)
> > 
> >
> > could we use stream instead of for loop: 
> > first = 
> > txnIds.stream().max(Comparator.comparing(Long::valueOf)).orElse(LONG_MAX)

actually turned out this is a dead code since the min history rebase, removed 
it alltogether


> On April 20, 2020, 10:40 a.m., Denys Kuzmenko wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
> > Lines 902 (patched)
> > 
> >
> > Can it be null? Could we add constraint on db level instead of multiple 
> > checks in backend code?

It should not be null, but the constraint is that there must be always one 
record in the table, not a nonnull column.


- Peter


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


On April 20, 2020, 7:25 a.m., Peter Varga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72388/
> ---
> 
> (Updated April 20, 2020, 7:25 a.m.)
> 
> 
> Review request for hive and Peter Vary.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> * Use sequences for TXN_ID generation.
>   * Make it possible to open transactions in parallel
>   * drop Oracle 11g support
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java 
> 37a5862791 
>   
> 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/ql/TestTxnCommands2.java 2c13e8dd03 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands3.java 51b0fa336f 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommandsForMmTable.java 
> 6525ffc00a 
>   ql/src/test/org/apache/hadoop/hive/ql/TxnCommandsBaseForTests.java 
> 1435269ed3 
>   ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java 
> 73d3b91585 
>   ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManagerAcid.java 
> PRE-CREATION 
>   ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestInitiator.java 
> 1151466f8c 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  f6097f7520 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/SQLGenerator.java
>  49b737ecf9 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java
>  2344c2d5f6 
>   
> 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
>  d080df417b 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnStore.java
>  87130a519d 
>   
> standalone-metastore/metastore-server/src/main/sql/derby/hive-schema-4.0.0.derby.sql
>  1ace9d3ef0 
>   
> standalone-metastore/metastore-server/src/main/sql/derby/upgrade-3.2.0-to-4.0.0.derby.sql
>  8a3cd56658 
>   
> standalone-metastore/metastore-server/src/main/sql/mssql/hive-schema-4.0.0.mssql.sql
>

Re: Review Request 72388: HIVE-23048 Use sequences for TXN_ID generation

2020-04-20 Thread Denys Kuzmenko via Review Board



> On April 20, 2020, 12:19 p.m., Peter Vary wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
> > Lines 780 (patched)
> > 
> >
> > TXN_META_INFO? What is it used for before? Or is it new? Could we use a 
> > specific "state" for example?

My understanding was, it's kinda marker to flag the new txn entries and get 
their ids.


- Denys


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


On April 20, 2020, 7:25 a.m., Peter Varga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72388/
> ---
> 
> (Updated April 20, 2020, 7:25 a.m.)
> 
> 
> Review request for hive and Peter Vary.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> * Use sequences for TXN_ID generation.
>   * Make it possible to open transactions in parallel
>   * drop Oracle 11g support
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java 
> 37a5862791 
>   
> 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/ql/TestTxnCommands2.java 2c13e8dd03 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands3.java 51b0fa336f 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommandsForMmTable.java 
> 6525ffc00a 
>   ql/src/test/org/apache/hadoop/hive/ql/TxnCommandsBaseForTests.java 
> 1435269ed3 
>   ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java 
> 73d3b91585 
>   ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManagerAcid.java 
> PRE-CREATION 
>   ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestInitiator.java 
> 1151466f8c 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  f6097f7520 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/SQLGenerator.java
>  49b737ecf9 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java
>  2344c2d5f6 
>   
> 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
>  d080df417b 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnStore.java
>  87130a519d 
>   
> standalone-metastore/metastore-server/src/main/sql/derby/hive-schema-4.0.0.derby.sql
>  1ace9d3ef0 
>   
> standalone-metastore/metastore-server/src/main/sql/derby/upgrade-3.2.0-to-4.0.0.derby.sql
>  8a3cd56658 
>   
> standalone-metastore/metastore-server/src/main/sql/mssql/hive-schema-4.0.0.mssql.sql
>  2e0177723d 
>   
> standalone-metastore/metastore-server/src/main/sql/mssql/upgrade-3.2.0-to-4.0.0.mssql.sql
>  9f3951575b 
>   
> standalone-metastore/metastore-server/src/main/sql/mysql/hive-schema-4.0.0.mysql.sql
>  0512a45cad 
>   
> standalone-metastore/metastore-server/src/main/sql/mysql/upgrade-3.2.0-to-4.0.0.mysql.sql
>  4b82e36ab4 
>   
> standalone-metastore/metastore-server/src/main/sql/oracle/hive-schema-4.0.0.oracle.sql
>  db398e5f66 
>   
> standalone-metastore/metastore-server/src/main/sql/oracle/upgrade-3.2.0-to-4.0.0.oracle.sql
>  1be83fc4a9 
>   
> standalone-metastore/metastore-server/src/main/sql/postgres/hive-schema-4.0.0.postgres.sql
>  e6e30160af 
>   
> standalone-metastore/metastore-server/src/main/sql/postgres/upgrade-3.2.0-to-4.0.0.postgres.sql
>  b90cecb173 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/DbInstallBase.java
>  c1a1629548 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/rules/DatabaseRule.java
>  a6d22d19ef 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/rules/Oracle.java
>  0b070e19ac 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/txn/TestOpenTxn.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/72388/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Peter Varga
> 
>

Re: Review Request 72388: HIVE-23048 Use sequences for TXN_ID generation

2020-04-20 Thread Peter Vary via Review Board


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



This is a really big/scary change. I am really interested in the performance 
results! :)
Thanks for all the effort! Some questions below


ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManagerAcid.java
Lines 119 (patched)


nit: space after //



ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManagerAcid.java
Lines 154 (patched)


Could we create a test for multigap case (gap with more then 1?)



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java
Lines 563 (patched)


Maybe insert the query instead of getting id and using again. Not very 
important, just asking...



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Lines 424 (patched)


nit: missing line break



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Line 448 (original), 528 (patched)


Just a question: This is so similar to getOpenTxnsInfo... Any way to use 
the same code with different query and different response handling?



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Lines 699 (patched)


openTxns(dbConn, stmt, rqst) is used in replTableWriteIdState as well. Do 
we have to check there for timeout as well?



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Lines 747 (patched)


nit: typo? "support every"?



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Lines 780 (patched)


TXN_META_INFO? What is it used for before? Or is it new? Could we use a 
specific "state" for example?



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Line 641 (original), 783 (patched)


Could we use a correct PreparedStatement, with the values set without the 
"hacky" quoting?

Like: pstmt.setXX for state/user/host/type/metainfo?

Or does this have a noticable performance gain?



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Lines 891 (patched)


If we implement the stuff in a single query in both cases when it is used, 
we can get rid of this method



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Line 1168 (original), 1418 (patched)


If we do not make such a fuss about the checking, just a simple assert 
instead, we might can inline the method here, since it is not used anywhere 
else? What do you think?



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Line 1995 (original), 2245 (patched)


nit: space "MetaException{"



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Lines 2262 (patched)


This is again a single query implemented with 3 SQL query and a java code. 
Am I right?



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Line 4539 (original), 4775 (patched)


nit: I do not see what is changed, but if we change please remove double 
spaces too :)



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Line 5078 (original), 5321 (patched)


nit: Missing space: "MetaException{"?



standalone-metastore/metastore-server/src/main/sql/mssql/upgrade-3.2.0-to-4.0.0.mssql.sql
Lines 117 (patched)


nit: extra space? "TXN_COMPONENTS  WITH"



standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/txn/TestOpenTxn.java
Lines 78 (patched)


Please test for multiple gaps too


- Peter Vary


On ápr. 20, 2020, 7:25 de,

Re: Review Request 72388: HIVE-23048 Use sequences for TXN_ID generation

2020-04-20 Thread Denys Kuzmenko via Review Board


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



LGTM, just a few comments


ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManagerAcid.java
Lines 47 (patched)


Looks similar to TestDbTxnManager2. Could we extract common parts (setUp, 
tearDown) into the base class. Also name is misleading, TestDbTxnManager2 also 
covers ACID



standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
Lines 1202 (patched)


I would replace commit to persist, as it's a bit misleading and could be 
confused with txn.commit



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/SQLGenerator.java
Lines 351 (patched)


Could we use Optional here? Returning null is not a good practice.



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java
Lines 554 (patched)


Extend commend with commited, as it deletes both aborted & commited



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java
Line 552 (original), 560 (patched)


Could we use this query as condition for delete? selete where exists (..)



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Line 172 (original), 225 (patched)


Please rebase, this code is no longer here



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Line 549 (original), 635 (patched)


not needed, could we remove this



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Lines 694 (patched)


Could we change "slower than that" -> "slower than openTxnTimeout" for 
clarity



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Lines 716 (patched)


not needed, could we remove this



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Lines 807 (patched)


could we use stream instead of for loop: 
first = 
txnIds.stream().max(Comparator.comparing(Long::valueOf)).orElse(LONG_MAX)



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Lines 902 (patched)


Can it be null? Could we add constraint on db level instead of multiple 
checks in backend code?



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Line 1332 (original), 1579 (patched)


Could we rename to updateWSCommitIdAndCleanUpMetadata for readability



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Lines 5480 (patched)


As mentioned above, I would use optional here.


- Denys Kuzmenko


On April 20, 2020, 7:25 a.m., Peter Varga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72388/
> ---
> 
> (Updated April 20, 2020, 7:25 a.m.)
> 
> 
> Review request for hive and Peter Vary.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> * Use sequences for TXN_ID generation.
>   * Make it possible to open transactions in parallel
>   * drop Oracle 11g support
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java 
> 37a5862791 
>   
> 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/ql/TestTxnCommands2.java 2c13e8dd03 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands3.java 51b0fa336f 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommandsForMmTable.java 
> 6525ffc00a 
>   ql/src/test/org/apache/hadoop/hive/ql/TxnCommandsBaseForTests.java 
> 1435269ed3 
>

Review Request 72388: HIVE-23048 Use sequences for TXN_ID generation

2020-04-20 Thread Peter Varga via Review Board


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

Review request for hive and Peter Vary.


Repository: hive-git


Description
---

* Use sequences for TXN_ID generation.
  * Make it possible to open transactions in parallel
  * drop Oracle 11g support


Diffs
-

  ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java 37a5862791 
  
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/ql/TestTxnCommands2.java 2c13e8dd03 
  ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands3.java 51b0fa336f 
  ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommandsForMmTable.java 
6525ffc00a 
  ql/src/test/org/apache/hadoop/hive/ql/TxnCommandsBaseForTests.java 1435269ed3 
  ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java 
73d3b91585 
  ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManagerAcid.java 
PRE-CREATION 
  ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestInitiator.java 
1151466f8c 
  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
 f6097f7520 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/SQLGenerator.java
 49b737ecf9 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java
 2344c2d5f6 
  
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
 d080df417b 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnStore.java
 87130a519d 
  
standalone-metastore/metastore-server/src/main/sql/derby/hive-schema-4.0.0.derby.sql
 1ace9d3ef0 
  
standalone-metastore/metastore-server/src/main/sql/derby/upgrade-3.2.0-to-4.0.0.derby.sql
 8a3cd56658 
  
standalone-metastore/metastore-server/src/main/sql/mssql/hive-schema-4.0.0.mssql.sql
 2e0177723d 
  
standalone-metastore/metastore-server/src/main/sql/mssql/upgrade-3.2.0-to-4.0.0.mssql.sql
 9f3951575b 
  
standalone-metastore/metastore-server/src/main/sql/mysql/hive-schema-4.0.0.mysql.sql
 0512a45cad 
  
standalone-metastore/metastore-server/src/main/sql/mysql/upgrade-3.2.0-to-4.0.0.mysql.sql
 4b82e36ab4 
  
standalone-metastore/metastore-server/src/main/sql/oracle/hive-schema-4.0.0.oracle.sql
 db398e5f66 
  
standalone-metastore/metastore-server/src/main/sql/oracle/upgrade-3.2.0-to-4.0.0.oracle.sql
 1be83fc4a9 
  
standalone-metastore/metastore-server/src/main/sql/postgres/hive-schema-4.0.0.postgres.sql
 e6e30160af 
  
standalone-metastore/metastore-server/src/main/sql/postgres/upgrade-3.2.0-to-4.0.0.postgres.sql
 b90cecb173 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/DbInstallBase.java
 c1a1629548 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/rules/DatabaseRule.java
 a6d22d19ef 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/rules/Oracle.java
 0b070e19ac 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/txn/TestOpenTxn.java
 PRE-CREATION 


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


Testing
---


Thanks,

Peter Varga

Re: Review Request 72388: HIVE-23048 Use sequences for TXN_ID generation

Re: Review Request 72388: HIVE-23048 Use sequences for TXN_ID generation

Re: Review Request 72388: HIVE-23048 Use sequences for TXN_ID generation

Re: Review Request 72388: HIVE-23048 Use sequences for TXN_ID generation

Re: Review Request 72388: HIVE-23048 Use sequences for TXN_ID generation

Re: Review Request 72388: HIVE-23048 Use sequences for TXN_ID generation

Re: Review Request 72388: HIVE-23048 Use sequences for TXN_ID generation

Re: Review Request 72388: HIVE-23048 Use sequences for TXN_ID generation

Re: Review Request 72388: HIVE-23048 Use sequences for TXN_ID generation

Re: Review Request 72388: HIVE-23048 Use sequences for TXN_ID generation

Re: Review Request 72388: HIVE-23048 Use sequences for TXN_ID generation

Re: Review Request 72388: HIVE-23048 Use sequences for TXN_ID generation

Re: Review Request 72388: HIVE-23048 Use sequences for TXN_ID generation

Re: Review Request 72388: HIVE-23048 Use sequences for TXN_ID generation

Re: Review Request 72388: HIVE-23048 Use sequences for TXN_ID generation

Re: Review Request 72388: HIVE-23048 Use sequences for TXN_ID generation

Re: Review Request 72388: HIVE-23048 Use sequences for TXN_ID generation

Re: Review Request 72388: HIVE-23048 Use sequences for TXN_ID generation

Re: Review Request 72388: HIVE-23048 Use sequences for TXN_ID generation

Re: Review Request 72388: HIVE-23048 Use sequences for TXN_ID generation

Re: Review Request 72388: HIVE-23048 Use sequences for TXN_ID generation

Review Request 72388: HIVE-23048 Use sequences for TXN_ID generation

22 matches

Site Navigation

Mail list logo

Footer information