[ 
https://issues.apache.org/jira/browse/HIVE-15077?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16373307#comment-16373307
 ] 

Eugene Koifman commented on HIVE-15077:
---------------------------------------

I did make it unfair in HIVE-10242 but that was not intentional.  (To ensure 
fairness the requested lock is only checked against locks with smaller 
extLockId.  The problem was that HIVE-10242 made it such that not all locks 
with smaller extLockId that should have been checked, were checked.)  I had to 
sort locks by weight in HIVE-10242 because the jumpTable doesn't have any info 
about the resource and so the logic that says "if looking to get S lock and see 
acquired S lock in front, acquire" doesn't work because the S lock in front may 
be on a different resource.  The issue this caused is demonstrated in 
\{{testFairness2()}} in the Description of this ticket.

So I'm aiming for a lock manager that is fair, correct and not more strict than 
necessary.  The last part is a work in progress.

What I think we really need is the use of Intention locks.  That way what you 
are suggesting is possible.  Right now we just "infer" a lock (which is not 
physically there) up/down the resource hierarchy based on the lock that is 
actually asked for (and of the same type).  This way you'd only have to compare 
locks on resources with the same path.  This is a bigger change.

 

 

> Acid LockManager is unfair
> --------------------------
>
>                 Key: HIVE-15077
>                 URL: https://issues.apache.org/jira/browse/HIVE-15077
>             Project: Hive
>          Issue Type: Bug
>          Components: Transactions
>    Affects Versions: 2.3.0
>            Reporter: Eugene Koifman
>            Assignee: Eugene Koifman
>            Priority: Blocker
>         Attachments: HIVE-15077.02.patch
>
>
> HIVE-10242 made the acid LM unfair.
> In TxnHandler.checkLock(), suppose we are trying to acquire SR5  (the number 
> is extLockId).  
> Then 
> LockInfo[] locks = lockSet.toArray(new LockInfo[lockSet.size()]);
> may look like this (all explicitly listed locks are in Waiting state)
> {...., SR5 SW3 X4}
> So the algorithm will find SR5 in the list and start looking backwards (to 
> the left).
> According to IDs, SR5 should wait for X4 to be granted but X4 won't even be 
> examined and so SR5 may be granted.
> Theoretically, this could cause starvation.
> The query that generates the list already has
> query.append(" and hl_lock_ext_id <= ").append(extLockId);
> but it should use "<" rather than "<=" to exclude the locks being checked 
> from "locks" list which will make the algorithm look at all locks "in front" 
> of a given lock.
> Here is an example (add to TestDbTxnManager2)
> {noformat}
>   @Test
>   public void testFairness2() throws Exception {
>     dropTable(new String[]{"T7"});
>     CommandProcessorResponse cpr = driver.run("create table if not exists T7 
> (a int) partitioned by (p int) stored as orc TBLPROPERTIES 
> ('transactional'='true')");
>     checkCmdOnDriver(cpr);
>     checkCmdOnDriver(driver.run("insert into T7 partition(p) 
> values(1,1),(1,2)"));//create 2 partitions
>     cpr = driver.compileAndRespond("select a from T7 ");
>     checkCmdOnDriver(cpr);
>     txnMgr.acquireLocks(driver.getPlan(), ctx, "Fifer");//gets S lock on T7
>     HiveTxnManager txnMgr2 = 
> TxnManagerFactory.getTxnManagerFactory().getTxnManager(conf);
>     swapTxnManager(txnMgr2);
>     cpr = driver.compileAndRespond("alter table T7 drop partition (p=1)");
>     checkCmdOnDriver(cpr);
>     //tries to get X lock on T7.p=1 and gets Waiting state
>     LockState lockState = ((DbTxnManager) 
> txnMgr2).acquireLocks(driver.getPlan(), ctx, "Fiddler", false);
>     List<ShowLocksResponseElement> locks = getLocks();
>     Assert.assertEquals("Unexpected lock count", 4, locks.size());
>     checkLock(LockType.SHARED_READ, LockState.ACQUIRED, "default", "T7", 
> null, locks);
>     checkLock(LockType.SHARED_READ, LockState.ACQUIRED, "default", "T7", 
> "p=1", locks);
>     checkLock(LockType.SHARED_READ, LockState.ACQUIRED, "default", "T7", 
> "p=2", locks);
>     checkLock(LockType.EXCLUSIVE, LockState.WAITING, "default", "T7", "p=1", 
> locks);
>     HiveTxnManager txnMgr3 = 
> TxnManagerFactory.getTxnManagerFactory().getTxnManager(conf);
>     swapTxnManager(txnMgr3);
>     //this should block behind the X lock on  T7.p=1
>     cpr = driver.compileAndRespond("select a from T7");
>     checkCmdOnDriver(cpr);
>     txnMgr3.acquireLocks(driver.getPlan(), ctx, "Fifer");//gets S lock on T6
>     locks = getLocks();
>     Assert.assertEquals("Unexpected lock count", 7, locks.size());
>     checkLock(LockType.SHARED_READ, LockState.ACQUIRED, "default", "T7", 
> null, locks);
>     checkLock(LockType.SHARED_READ, LockState.ACQUIRED, "default", "T7", 
> "p=1", locks);
>     checkLock(LockType.SHARED_READ, LockState.ACQUIRED, "default", "T7", 
> "p=2", locks);
>     checkLock(LockType.SHARED_READ, LockState.ACQUIRED, "default", "T7", 
> null, locks);
>     checkLock(LockType.SHARED_READ, LockState.ACQUIRED, "default", "T7", 
> "p=1", locks);
>     checkLock(LockType.SHARED_READ, LockState.ACQUIRED, "default", "T7", 
> "p=2", locks);
>     checkLock(LockType.EXCLUSIVE, LockState.WAITING, "default", "T7", "p=1", 
> locks);
>   }
> {noformat}
> The 2nd {{locks = getLocks();}} output shows that all locks for the 2nd 
> {{select * from T7}} are all acquired while they should block behind the X 
> lock to be fair.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to