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

Wei Zheng commented on HIVE-12636:
----------------------------------

It looks good in general. A few minor comments:

Missed "break;" in Driver.startImplicitTxn() after case COMMIT and ROLLBACK?

Comment for "return 10;" in Driver.compile() ? Same block, "long txnid = 
txnManager.openTxn(ctx, userFromUGI);", txnid is not used.

Not too sure about this in Driver.recordValidTxns()
{code}
    if(oldList != null) {
      throw new IllegalStateException("calling recordValidTxn() more than once 
in the same " +
        JavaUtils.txnIdToString(txnMgr.getCurrentTxnId()));
    }
{code}

"userFromUGI" in Driver.getUserFromUGI() is no longer used.

In SemanticAnalyzerFactory, should they stay?
//    commandType.put(HiveParser.TOK_UPDATE_TABLE, 
HiveOperation.SQLUPDATE);//HIVE-16443
//    commandType.put(HiveParser.TOK_DELETE_FROM, HiveOperation.SQLDELETE);
//    commandType.put(HiveParser.TOK_MERGE, HiveOperation.SQLMERGE);
//   INSERT, INSERT OVERWRITE, 

Why would some stats estimate in q.out files change?

> Ensure that all queries (with DbTxnManager) run in a transaction
> ----------------------------------------------------------------
>
>                 Key: HIVE-12636
>                 URL: https://issues.apache.org/jira/browse/HIVE-12636
>             Project: Hive
>          Issue Type: Improvement
>          Components: Transactions
>    Affects Versions: 1.0.0
>            Reporter: Eugene Koifman
>            Assignee: Eugene Koifman
>            Priority: Critical
>         Attachments: HIVE-12636.01.patch, HIVE-12636.02.patch, 
> HIVE-12636.03.patch, HIVE-12636.04.patch, HIVE-12636.05.patch, 
> HIVE-12636.06.patch, HIVE-12636.07.patch, HIVE-12636.09.patch, 
> HIVE-12636.10.patch, HIVE-12636.12.patch, HIVE-12636.13.patch, 
> HIVE-12636.17.patch
>
>
> Assuming Hive is using DbTxnManager
> Currently (as of this writing only auto commit mode is supported), only 
> queries that write to an Acid table start a transaction.
> Read-only queries don't open a txn but still acquire locks.
> This makes internal structures confusing/odd.
> The are constantly 2 code paths to deal with which is inconvenient and error 
> prone.
> Also, a txn id is convenient "handle" for all locks/resources within a txn.
> Doing thing would mean the client no longer needs to track locks that it 
> acquired.  This enables further improvements to metastore side of Acid.
> # add metastore call to openTxn() and acquireLocks() in a single call.  this 
> it to make sure perf doesn't degrade for read-only query.  (Would also be 
> useful for auto commit write queries)
> # Should RO queries generate txn ids from the same sequence?  (they could for 
> example use negative values of a different sequence).  Txnid is part of the 
> delta/base file name.  Currently it's 7 digits.  If we use the same sequence, 
> we'll exceed 7 digits faster. (possible upgrade issue).  On the other hand 
> there is value in being able to pick txn id and commit timestamp out of the 
> same logical sequence.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

Reply via email to