[ 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)