You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hive.apache.org by "ASF GitHub Bot (Jira)" <ji...@apache.org> on 2022/05/02 12:04:00 UTC
[jira] [Work logged] (HIVE-26023) Non blocking REPLACE, RENAME COLUMNS implementation
[ https://issues.apache.org/jira/browse/HIVE-26023?focusedWorklogId=764910&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-764910 ]
ASF GitHub Bot logged work on HIVE-26023:
-----------------------------------------
Author: ASF GitHub Bot
Created on: 02/May/22 12:03
Start Date: 02/May/22 12:03
Worklog Time Spent: 10m
Work Description: veghlaci05 commented on code in PR #3089:
URL: https://github.com/apache/hive/pull/3089#discussion_r862778203
##########
ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java:
##########
@@ -3592,4 +3592,94 @@ public void testDropTableNonBlocking2() throws Exception {
driver.getFetchTask().fetch(res);
Assert.assertEquals("No records found", 2, res.size());
}
-}
+
+ @Test
+ public void testReplaceColumnsNonBlocking() throws Exception {
+ //TODO: incorporate add new column via `REPLACE` inside of `ADD COLUMNS` test case
+ testReplaceColumns(false);
+ }
+ @Test
+ public void testReplaceColumnsBlocking() throws Exception {
+ testReplaceColumns(true);
+ }
+ private void testReplaceColumns(boolean blocking) throws Exception {
+ testReplaceRenameColumns(blocking, "replace columns (c string, a bigint)");
+ }
+
+ @Test
+ public void testRenameColumnsNonBlocking() throws Exception {
+ testRenameColumns(false);
+ }
+ @Test
+ public void testRenameColumnsBlocking() throws Exception {
+ testRenameColumns(true);
+ }
+ private void testRenameColumns(boolean blocking) throws Exception {
+ testReplaceRenameColumns(blocking, "change column a c string");
+ }
+
+ private void testReplaceRenameColumns(boolean blocking, String alterSubQuery) throws Exception {
+ dropTable(new String[] {"tab_acid"});
+
+ HiveConf.setBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_LOCKLESS_READS_ENABLED, !blocking);
+ driver = Mockito.spy(driver);
+
+ HiveConf.setBoolVar(driver2.getConf(), HiveConf.ConfVars.HIVE_ACID_LOCKLESS_READS_ENABLED, !blocking);
+ driver2 = Mockito.spy(driver2);
+
+ driver.run("create table if not exists tab_acid (a int, b int) " +
+ "stored as orc TBLPROPERTIES ('transactional'='true')");
+ driver.run("insert into tab_acid (a,b) values(1,2),(3,4)");
+
+ driver.compileAndRespond("select * from tab_acid");
+ List<String> res = new ArrayList<>();
+
+ driver.lockAndRespond();
+ List<ShowLocksResponseElement> locks = getLocks();
+ Assert.assertEquals("Unexpected lock count", 1, locks.size());
+
+ checkLock(LockType.SHARED_READ,
+ LockState.ACQUIRED, "default", "tab_acid", null, locks);
+
+ DbTxnManager txnMgr2 = (DbTxnManager) TxnManagerFactory.getTxnManagerFactory().getTxnManager(conf);
+ swapTxnManager(txnMgr2);
+ driver2.compileAndRespond("alter table tab_acid "+ alterSubQuery);
+
+ if (blocking) {
+ txnMgr2.acquireLocks(driver2.getPlan(), ctx, null, false);
+ locks = getLocks();
+
+ ShowLocksResponseElement checkLock = checkLock(LockType.EXCLUSIVE,
+ LockState.WAITING, "default", "tab_acid", null, locks);
+
+ swapTxnManager(txnMgr);
+ Mockito.doNothing().when(driver).lockAndRespond();
+ driver.run();
+
+ driver.getFetchTask().fetch(res);
+ swapTxnManager(txnMgr2);
+
+ FieldSetter.setField(txnMgr2, txnMgr2.getClass().getDeclaredField("numStatements"), 0);
Review Comment:
Why is this set needed?
##########
ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java:
##########
@@ -3592,4 +3592,94 @@ public void testDropTableNonBlocking2() throws Exception {
driver.getFetchTask().fetch(res);
Assert.assertEquals("No records found", 2, res.size());
}
-}
+
+ @Test
+ public void testReplaceColumnsNonBlocking() throws Exception {
+ //TODO: incorporate add new column via `REPLACE` inside of `ADD COLUMNS` test case
+ testReplaceColumns(false);
+ }
+ @Test
+ public void testReplaceColumnsBlocking() throws Exception {
+ testReplaceColumns(true);
+ }
+ private void testReplaceColumns(boolean blocking) throws Exception {
+ testReplaceRenameColumns(blocking, "replace columns (c string, a bigint)");
+ }
+
+ @Test
+ public void testRenameColumnsNonBlocking() throws Exception {
+ testRenameColumns(false);
+ }
+ @Test
+ public void testRenameColumnsBlocking() throws Exception {
+ testRenameColumns(true);
+ }
+ private void testRenameColumns(boolean blocking) throws Exception {
+ testReplaceRenameColumns(blocking, "change column a c string");
+ }
+
+ private void testReplaceRenameColumns(boolean blocking, String alterSubQuery) throws Exception {
+ dropTable(new String[] {"tab_acid"});
+
+ HiveConf.setBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_LOCKLESS_READS_ENABLED, !blocking);
+ driver = Mockito.spy(driver);
+
+ HiveConf.setBoolVar(driver2.getConf(), HiveConf.ConfVars.HIVE_ACID_LOCKLESS_READS_ENABLED, !blocking);
+ driver2 = Mockito.spy(driver2);
+
+ driver.run("create table if not exists tab_acid (a int, b int) " +
+ "stored as orc TBLPROPERTIES ('transactional'='true')");
+ driver.run("insert into tab_acid (a,b) values(1,2),(3,4)");
+
+ driver.compileAndRespond("select * from tab_acid");
+ List<String> res = new ArrayList<>();
+
+ driver.lockAndRespond();
+ List<ShowLocksResponseElement> locks = getLocks();
+ Assert.assertEquals("Unexpected lock count", 1, locks.size());
+
+ checkLock(LockType.SHARED_READ,
+ LockState.ACQUIRED, "default", "tab_acid", null, locks);
+
+ DbTxnManager txnMgr2 = (DbTxnManager) TxnManagerFactory.getTxnManagerFactory().getTxnManager(conf);
+ swapTxnManager(txnMgr2);
+ driver2.compileAndRespond("alter table tab_acid "+ alterSubQuery);
+
+ if (blocking) {
+ txnMgr2.acquireLocks(driver2.getPlan(), ctx, null, false);
+ locks = getLocks();
+
+ ShowLocksResponseElement checkLock = checkLock(LockType.EXCLUSIVE,
+ LockState.WAITING, "default", "tab_acid", null, locks);
+
+ swapTxnManager(txnMgr);
+ Mockito.doNothing().when(driver).lockAndRespond();
+ driver.run();
+
+ driver.getFetchTask().fetch(res);
+ swapTxnManager(txnMgr2);
+
+ FieldSetter.setField(txnMgr2, txnMgr2.getClass().getDeclaredField("numStatements"), 0);
+ txnMgr2.getMS().unlock(checkLock.getLockid());
+ }
+ driver2.lockAndRespond();
+ locks = getLocks();
+ Assert.assertEquals("Unexpected lock count", blocking ? 1 : 2, locks.size());
+
+ checkLock(blocking ? LockType.EXCLUSIVE : LockType.EXCL_WRITE,
+ LockState.ACQUIRED, "default", "tab_acid", null, locks);
Review Comment:
This could be changed to sth like
` checkLock(blocking ? LockType.EXCLUSIVE : LockType.EXCL_WRITE,
blocking ? LockState.WAITING : LockState.ACQUIRED, "default", "tab_acid", null, locks);`
In this case the code block inside `if (blocking)` would not be necessary.
Issue Time Tracking
-------------------
Worklog Id: (was: 764910)
Time Spent: 20m (was: 10m)
> Non blocking REPLACE, RENAME COLUMNS implementation
> ---------------------------------------------------
>
> Key: HIVE-26023
> URL: https://issues.apache.org/jira/browse/HIVE-26023
> Project: Hive
> Issue Type: Task
> Reporter: Denys Kuzmenko
> Priority: Major
> Labels: pull-request-available
> Time Spent: 20m
> Remaining Estimate: 0h
>
> Modify the REPLACE/RENAME COLUMNS operation to not acquire EXCLUSIVE lock limiting the system concurrency.
--
This message was sent by Atlassian Jira
(v8.20.7#820007)