difin commented on code in PR #6457: URL: https://github.com/apache/hive/pull/6457#discussion_r3641035054
########## ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnDeadlockDetector.java: ########## @@ -0,0 +1,565 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hive.ql.lockmgr; + +import org.apache.hadoop.hive.conf.HiveConf; +import org.apache.hadoop.hive.metastore.api.LockState; +import org.apache.hadoop.hive.metastore.api.ShowLocksResponse; +import org.apache.hadoop.hive.metastore.api.ShowLocksResponseElement; +import org.apache.hadoop.hive.metastore.conf.MetastoreConf; +import org.apache.hadoop.hive.metastore.metrics.Metrics; +import org.apache.hadoop.hive.metastore.metrics.MetricsConstants; +import org.apache.hadoop.hive.metastore.txn.service.DeadlockDetectorService; +import org.apache.hadoop.hive.ql.session.SessionState; +import org.junit.Assert; +import org.junit.Test; + +import java.util.List; + +/** + * End-to-end tests for the metastore transaction deadlock detector + * ({@code DeadlockDetectorService} / {@code DeadlockDetectionFunction}). + */ +public class TestDbTxnDeadlockDetector extends DbTxnManagerEndToEndTestBase { + + /** + * N-party deadlock cycle: txn_i holds table_i, then requests table_{(i+1) % n}, forming + * a complete wait-for cycle. The detector must abort the youngest member (txn_{n-1}) and + * leave the rest live; the predecessor's waiting lock must become acquirable. Tarjan's + * SCC handles cycles of any size uniformly, so {n=2, 3} cover the canonical case and a + * non-trivial SCC respectively. + */ + @Test + public void testDeadlockDetectionMultiParty() throws Exception { + for (int n : new int[]{2, 3}) { + assertCycleAbortsYoungest(n); + } + } + + private void assertCycleAbortsYoungest(int n) throws Exception { + String[] tables = new String[n]; + for (int i = 0; i < n; i++) { + tables[i] = "DLM" + n + "_T" + i; + } + setupDeadlockTestTables(tables); + + HiveTxnManager[] mgrs = new HiveTxnManager[n]; + long[] ids = new long[n]; + for (int i = 0; i < n; i++) { + mgrs[i] = TxnManagerFactory.getTxnManagerFactory().getTxnManager(conf); + swapTxnManager(mgrs[i]); + mgrs[i].openTxn(ctx, "Txn" + i); + ids[i] = mgrs[i].getCurrentTxnId(); + driver.compileAndRespond("update " + tables[i] + " set a = 1 where b = 1", true); + mgrs[i].acquireLocks(driver.getPlan(), ctx, "Txn" + i); + } + for (int i = 1; i < n; i++) { + Assert.assertTrue("Txn IDs must be monotonically increasing", ids[i] > ids[i - 1]); + } + + for (int i = 0; i < n; i++) { + swapTxnManager(mgrs[i]); + driver.compileAndRespond("update " + tables[(i + 1) % n] + " set a = 2 where b = 2", true); + ((DbTxnManager) mgrs[i]).acquireLocks(driver.getPlan(), ctx, "Txn" + i, false); + } + + runDeadlockDetector(); + + int victim = n - 1; + LockException ex = null; + try { + mgrs[victim].heartbeat(); + } catch (LockException e) { + ex = e; + } + Assert.assertNotNull(n + "-party: heartbeat on victim Txn" + victim + " should fail", ex); + Assert.assertTrue(n + "-party: cause should be TxnAbortedException, was: " + ex, + ex.getCause() instanceof org.apache.hadoop.hive.metastore.api.TxnAbortedException); + for (int i = 0; i < victim; i++) { + mgrs[i].heartbeat(); + } + + // Predecessor's waiting lock (on the victim's table) is now acquirable. + int predecessor = victim - 1; + swapTxnManager(mgrs[predecessor]); + List<ShowLocksResponseElement> locks = getLocks(mgrs[predecessor]); + long waitingLockId = findWaitingLockId(locks, tables[victim], ids[predecessor]); + Assert.assertTrue(n + "-party: predecessor's waiting lock should exist", waitingLockId > 0); + LockState state = ((DbLockManager) mgrs[predecessor].getLockManager()).checkLock(waitingLockId); + Assert.assertEquals(n + "-party: predecessor's lock should now be acquired", + LockState.ACQUIRED, state); + + for (int i = 0; i < victim; i++) { + mgrs[i].rollbackTxn(); + } + for (HiveTxnManager m : mgrs) { + m.closeTxnManager(); + } + swapTxnManager(txnMgr); + } + + /** + * Linear wait chain (no cycle): txn1 holds lock, txn2 waits for txn1. + * The detector must NOT abort either transaction. + */ + @Test + public void testNoFalsePositiveLinearChain() throws Exception { + setupDeadlockTestTables("NF_T1"); + HiveTxnManager txnMgr1 = openTxnWithStatement("Txn1", "update NF_T1 set a = 1 where b = 1"); + HiveTxnManager txnMgr2 = openTxn("Txn2"); + runStatementNonBlocking(txnMgr2, "Txn2", "update NF_T1 set a = 2 where b = 2"); + + long waitingLockId = findWaitingLockId(getLocks(txnMgr2), "NF_T1", txnMgr2.getCurrentTxnId()); + Assert.assertTrue("Should have found txn2's waiting lock", waitingLockId > 0); + + long deadlockedBefore = getDeadlockedCounter(); + runDeadlockDetector(); + Assert.assertEquals("Linear chain must not increment deadlock counter", + deadlockedBefore, getDeadlockedCounter()); + + // Neither txn should have been aborted; txn2 remains WAITING. + txnMgr1.heartbeat(); + txnMgr2.heartbeat(); + LockState state = ((DbLockManager) txnMgr2.getLockManager()).checkLock(waitingLockId); + Assert.assertEquals("Txn2 should still be WAITING (no deadlock)", LockState.WAITING, state); + + txnMgr1.rollbackTxn(); + txnMgr2.rollbackTxn(); + txnMgr1.closeTxnManager(); + txnMgr2.closeTxnManager(); + swapTxnManager(txnMgr); + } + + /** + * Victim selection is confined to the cycle: txn3 — the youngest txn and itself a hard + * blocker (holds IW_T3) — waits on a cycle member but is not in the cycle, and must not Review Comment: "the youngest txn and itself a hard blocker" - this is a little bit unclear, is it a typo? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
