> On May 4, 2020, 3:21 p.m., Denys Kuzmenko wrote: > > LGTM , just few questions
Thanks for the review! > On May 4, 2020, 3:21 p.m., Denys Kuzmenko wrote: > > ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java > > Line 153 (original) > > <https://reviews.apache.org/r/72469/diff/2/?file=2230142#file2230142line155> > > > > Cleanup functionality was moved out of the Initiator and is covered by > > HousekeeperThreadnow. Is it right? Yes, that's correct. > On May 4, 2020, 3:21 p.m., Denys Kuzmenko wrote: > > ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestInitiator.java > > Line 205 (original) > > <https://reviews.apache.org/r/72469/diff/2/?file=2230147#file2230147line205> > > > > Do we have similar test for Housekeeper? Yes. The various TxnStore operations (e.g. writeset pruning) that AcidHouseKeeper executes in sequence are already covered by tests in various places (e.g. TestTxnHandler, TestDbTxnManager2). > On May 4, 2020, 3:21 p.m., Denys Kuzmenko wrote: > > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/AcidCompactionHistoryService.java > > Line 31 (original), 32 (patched) > > <https://reviews.apache.org/r/72469/diff/2/?file=2230151#file2230151line32> > > > > What does empty txn mean? Is it obsolete/abandoned/timed-out txns? It refers to aborted/committed TXNs that do not have any entries in TXN_COMPONENTS (hence the word empty). > On May 4, 2020, 3:21 p.m., Denys Kuzmenko wrote: > > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/AcidHouseKeeperService.java > > Lines 84 (patched) > > <https://reviews.apache.org/r/72469/diff/2/?file=2230152#file2230152line84> > > > > Old AcidCompactionHistoryService was doing same stuff > > (txnHandler.purgeCompactionHistory()). Do we need than > > AcidEmptyTxnCleanerService and if yes can we embed it here? Those operations that can run with the about same frequency (timedout txn cleaning, writeset cleaning, compaction history pruning) have been merged and consolidated into a single HouseKeeper for simplicity (along with the cleaning ops Initiator used to do). However, AcidEmptyTxnCleaner needs to be treated separately it has to run significantly more frequently than AcidHouseKeeper due to how the new open txn mechanism works. - Marton ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72469/#review220594 ----------------------------------------------------------- On May 4, 2020, 12:58 p.m., Marton Bod wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/72469/ > ----------------------------------------------------------- > > (Updated May 4, 2020, 12:58 p.m.) > > > Review request for hive, Denys Kuzmenko and Peter Vary. > > > Repository: hive-git > > > Description > ------- > > Consolidate different metastore housekeeper threads into one, move cleaner > methods out of compactor initiator. > Create separate txn cleaner service which will need to run more frequently. > > > Diffs > ----- > > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 829791e0a9 > ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java > 23512e252e > ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands2.java 48bf8529fa > ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands3.java 5b8c6701e1 > ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommandsForMmTable.java > eac2c6307f > ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java > 1687425bcb > ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestInitiator.java > e4ff14a140 > > standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java > 842b7fe53d > > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java > 7bba8d6ee6 > > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MaterializationsRebuildLockCleanerTask.java > d35c9602a6 > > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/AcidCompactionHistoryService.java > e96a7ba289 > > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/AcidHouseKeeperService.java > c4a488bac0 > > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/AcidOpenTxnsCounterService.java > 2ad5a89f03 > > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/AcidWriteSetService.java > 5ec513dfd4 > > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java > 8fded608d0 > > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnStore.java > e8ac71ae55 > > standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/conf/TestMetastoreConf.java > 9905a14983 > > standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/txn/TestAcidEmptyTxnCleanerService.java > PRE-CREATION > > > Diff: https://reviews.apache.org/r/72469/diff/2/ > > > Testing > ------- > > > Thanks, > > Marton Bod > >
