----------------------------------------------------------- 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) <https://reviews.apache.org/r/72388/#comment308705> nit: space after // ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManagerAcid.java Lines 154 (patched) <https://reviews.apache.org/r/72388/#comment308707> 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) <https://reviews.apache.org/r/72388/#comment308713> 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) <https://reviews.apache.org/r/72388/#comment308706> nit: missing line break standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java Line 448 (original), 528 (patched) <https://reviews.apache.org/r/72388/#comment308708> 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) <https://reviews.apache.org/r/72388/#comment308709> 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) <https://reviews.apache.org/r/72388/#comment308710> nit: typo? "support every"? standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java Lines 780 (patched) <https://reviews.apache.org/r/72388/#comment308712> 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) <https://reviews.apache.org/r/72388/#comment308711> 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) <https://reviews.apache.org/r/72388/#comment308715> 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) <https://reviews.apache.org/r/72388/#comment308716> 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) <https://reviews.apache.org/r/72388/#comment308717> nit: space "MetaException{" standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java Lines 2262 (patched) <https://reviews.apache.org/r/72388/#comment308714> 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) <https://reviews.apache.org/r/72388/#comment308718> 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) <https://reviews.apache.org/r/72388/#comment308719> 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) <https://reviews.apache.org/r/72388/#comment308720> nit: extra space? "TXN_COMPONENTS WITH" standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/txn/TestOpenTxn.java Lines 78 (patched) <https://reviews.apache.org/r/72388/#comment308721> Please test for multiple gaps too - Peter Vary On ápr. 20, 2020, 7:25 de, Peter Varga wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/72388/ > ----------------------------------------------------------- > > (Updated ápr. 20, 2020, 7:25 de) > > > 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 > >
