jfsii commented on code in PR #3887:
URL: https://github.com/apache/hive/pull/3887#discussion_r1053821695
##########
ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java:
##########
@@ -42,6 +42,7 @@
import org.apache.hadoop.hive.ql.Driver;
import org.apache.hadoop.hive.ql.QueryState;
import org.apache.hadoop.hive.ql.io.AcidUtils;
+import org.apache.hadoop.hive.ql.metadata.HiveException;
Review Comment:
I don't think this is used, remove it.
##########
ql/src/java/org/apache/hadoop/hive/ql/Driver.java:
##########
@@ -555,7 +551,14 @@ private void prepareContext() throws
CommandProcessorException {
String originalCboInfo = context != null ? context.cboInfo : null;
if (context != null && context.getExplainAnalyze() !=
AnalyzeState.RUNNING) {
// close the existing ctx etc before compiling a new query, but does not
destroy driver
- closeInProcess(false);
+ if (!driverContext.isRetrial()) {
+ closeInProcess(false);
+ } else {
+ // On retrail we need to maintain information from the prior context.
Such
Review Comment:
fix typo
##########
ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java:
##########
@@ -2465,6 +2468,113 @@ private void
testConcurrent2MergeUpdatesConflict(boolean slowCompile) throws Exc
Assert.assertTrue("Lost Update", isEqualCollection(res, asList("earl\t10",
"amy\t10")));
}
+ // The intent of this test is to cause multiple conflicts to the same query
to test the conflict retry functionality.
+ @Test
+ public void testConcurrentConflictRetry() throws Exception {
+ dropTable(new String[]{"target"});
+
+ driver2.getConf().setBoolVar(HiveConf.ConfVars.TXN_WRITE_X_LOCK, true);
+ driver.run("create table target(i int) stored as orc tblproperties
('transactional'='true')");
+ driver.run("insert into target values (1),(1)");
+
+ DbTxnManager txnMgr2 = (DbTxnManager)
TxnManagerFactory.getTxnManagerFactory().getTxnManager(conf);
+ swapTxnManager(txnMgr2);
+ // Start a query on driver2
+ driver2.compileAndRespond("update target set i = 1 where i = 1");
+
+ swapTxnManager(txnMgr);
+ // this should cause a conflict which should cause driver2.run to invoke
its conflict lambda
+ driver.run("update target set i = 1 where i = 1");
+ swapTxnManager(txnMgr2);
+
+ // This run should conflict with the above query and cause the "conflict
lambda" to be execute,
+ // which will then also conflict with the driver2 query and cause it to
retry.
+ AtomicInteger conflictCount = new AtomicInteger();
+ driver2.run(() -> {
+ conflictCount.getAndIncrement();
+ // on first conflict,
+ // we execute another query to cause an additional conflict
+ if (conflictCount.get() == 1) {
+ swapTxnManager(txnMgr);
+ // this should cause a conflict
+ try {
+ driver.run("update target set i = 1 where i = 1");
+ } catch (Exception e) {
+ // do nothing
+ }
+ swapTxnManager(txnMgr2);
+ }
+ });
+
+ // we expected two conflicts
+ Assert.assertEquals(2, conflictCount.get());
+ swapTxnManager(txnMgr);
+ // we expect two rows
+ driver.run("select * from target");
+ List<String> res = new ArrayList<>();
+ driver.getFetchTask().fetch(res);
+ Assert.assertEquals(2, res.size());
+ }
+
+ @Rule
+ public ExpectedException exceptionRule = ExpectedException.none();
+
+ @Test
+ public void testConcurrentConflictMaxRetryCount() throws Exception {
+ dropTable(new String[]{"target"});
+
+ driver2.getConf().setBoolVar(HiveConf.ConfVars.TXN_WRITE_X_LOCK, true);
+
+ final int maxRetries = 4;
+
driver2.getConf().setIntVar(HiveConf.ConfVars.HIVE_TXN_MAX_RETRYSNAPSHOT_COUNT,
maxRetries);
+
+ driver.run("create table target(i int) stored as orc tblproperties
('transactional'='true')");
+ driver.run("insert into target values (1),(1)");
+
+ DbTxnManager txnMgr2 = (DbTxnManager)
TxnManagerFactory.getTxnManagerFactory().getTxnManager(conf);
+ swapTxnManager(txnMgr2);
+ // Start a query on driver2, we expect this query to never execute because
of HIVE_TXN_MAX_RETRYSNAPSHOT_COUNT.
+ // We verify that it is never executed by counting the number of rows
returned that have i = 1.
+ driver2.compileAndRespond("update target set i = 2 where i = 1");
+
+ swapTxnManager(txnMgr);
+ // this should cause a conflict which should cause driver2.run to invoke
its conflict lambda
+ driver.run("update target set i = 1 where i = 1");
+ swapTxnManager(txnMgr2);
+
+ // This run should conflict with the above query and cause the "conflict
lambda" to be execute,
+ // which will then also conflict with the driver2 query and cause it to
retry. The intent here is
+ // to cause driver2's query to exceed HIVE_TXN_MAX_RETRYSNAPSHOT_COUNT and
throw exception.
+ AtomicInteger conflictCount = new AtomicInteger();
+ exceptionRule.expect(CommandProcessorException.class);
+ exceptionRule.expectMessage("Operation could not be executed, snapshot was
outdated when locks were acquired.");
+ driver2.run(() -> {
+ conflictCount.getAndIncrement();
+ // on first conflict,
+ // we execute another query to cause an additional conflict
+ if (conflictCount.get() <= maxRetries) {
+ swapTxnManager(txnMgr);
+ // this should cause a conflict
+ try {
+ // we use update here, to force a conflict but not create additional
rows
+ driver.run("update target set i = 1 where i = 1");
+ } catch (Exception e) {
+ // do nothing
+ }
+ swapTxnManager(txnMgr2);
+ }
+ });
+
+ // We expect maxRetries+1 conflicts
Review Comment:
I don't think this gets executed because of the exception rule. I'm going to
rewrite this to not use exception rule and use a try / catch and some asserts.
--
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]
