> 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) > > <https://reviews.apache.org/r/72388/diff/4/?file=2227458#file2227458line1112> > > > > 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) > > <https://reviews.apache.org/r/72388/diff/4/?file=2227463#file2227463line589> > > > > 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) > > <https://reviews.apache.org/r/72388/diff/4/?file=2227464#file2227464line86> > > > > 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) > > <https://reviews.apache.org/r/72388/diff/4/?file=2227465#file2227465line228> > > > > 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) > > <https://reviews.apache.org/r/72388/diff/4/?file=2227465#file2227465line534> > > > > 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) > > <https://reviews.apache.org/r/72388/diff/4/?file=2227465#file2227465line591> > > > > 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) > > <https://reviews.apache.org/r/72388/diff/4/?file=2227465#file2227465line718> > > > > 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) > > <https://reviews.apache.org/r/72388/diff/4/?file=2227465#file2227465line733> > > > > 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) > > <https://reviews.apache.org/r/72388/diff/4/?file=2227465#file2227465line881> > > > > 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) > > <https://reviews.apache.org/r/72388/diff/4/?file=2227465#file2227465line957> > > > > 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. > > > 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: https://reviews.apache.org/r/72388/diff/5/ > > > Testing > ------- > > > Thanks, > > Peter Varga > >
