[jira] [Comment Edited] (HIVE-17647) DDLTask.generateAddMmTasks(Table tbl) and other random code should not start transactions

["Eugene Koifman (JIRA)" <ji...@apache.org>]

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

Eugene Koifman edited comment on HIVE-17647 at 4/5/18 6:29 PM:
---------------------------------------------------------------

In DDLTask
{noformat}
if (writeId == null) {
	      throw new HiveException("Internal error - write ID not set for MM conversion"); 
 }
 could this include table name or path
{noformat}

Could you add a test to check that X lock is actually acquired?  (TestDbTxnManager2) I'm not sure {{ddlWork.setNeedLock(true);}} will do anything.  LM relies on WriteEntity with WriteType.DD_EXCLUSIVE to get X lock...

Also, I don't any .q tests check that the data lands in the right place, i.e. into a delta.  that would be useful.


was (Author: ekoifman):
In DDLTask
{noformat}
if (writeId == null) {
	      throw new HiveException("Internal error - write ID not set for MM conversion"); 
 }
 could this include table name or path
{noformat}

Could you add a test to check that X lock is actually acquired?  (TestDbTxnManager2) I'm not sure {{ddlWork.setNeedLock(true);}} will do anything.  LM relies on WriteEntity with WriteType.DD_EXCLUSIVE to get X lock...


> DDLTask.generateAddMmTasks(Table tbl) and other random code should not start transactions
> -----------------------------------------------------------------------------------------
>
>                 Key: HIVE-17647
>                 URL: https://issues.apache.org/jira/browse/HIVE-17647
>             Project: Hive
>          Issue Type: Sub-task
>          Components: Transactions
>            Reporter: Eugene Koifman
>            Assignee: Sergey Shelukhin
>            Priority: Major
>              Labels: mm-gap-2
>         Attachments: HIVE-17647.patch
>
>
> This method (and other places) have 
> {noformat}
>       if (txnManager.isTxnOpen()) {
>         mmWriteId = txnManager.getCurrentTxnId();
>       } else {
>         mmWriteId = txnManager.openTxn(new Context(conf), conf.getUser());
>         txnManager.commitTxn();
>       }
> {noformat}
> this should throw if there is no open transaction.  It should never open one.
> In general the logic seems suspect.  Looks like the intent is to move all existing files into a delta_x_x/ when a plain table is converted to MM table.  This seems like something that needs to be done from under an Exclusive lock to prevent concurrent Insert operations writing data under table/partition root.  But this is too late to acquire locks which should be done from the Driver.acquireLocks()  (or else have deadlock detector since acquiring them here would bread all-or-nothing lock acquisition semantics currently required w/o deadlock detector)



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