[
https://issues.apache.org/jira/browse/HIVE-14263?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15389034#comment-15389034
]
Thejas M Nair commented on HIVE-14263:
--------------------------------------
Thanks for the patch [~taoli-hwx]!
Some comments on 1.patch -
* The 'final string' can be in the tryAcquireCompileLock method itself as they
are not being referenced from anywhere else. Maybe we don't even need those
variables, as they are just used once within the functions. Thats easier to
read.
* "a call to tryLock() will immediately acquire the lock if it is available,
whether or not other threads are currently waiting for the lock. "
https://docs.oracle.com/javase/7/docs/api/java/util/concurrent/locks/ReentrantLock.html#tryLock().
Looks like tryLock(0, TimeUnit.SECONDS) is the better call to make
* Use SessionState.getConsole().logInfo instead of ol.writeOperationLog. I see
that ol.writeOperationLog is being used at few other places, that should be
cleaned up. SessionState.getConsole().* methods are used in almost all places
for information . We can cleanup the usage of ol.writeOperationLog in a
separate patch.
* LOG.debug(waitingForLockMsg + ": " + command) can be removed once
SessionState.getConsole().logInfo is used. It will print to the HS2 log as well.
* If the 'waiting for lock' is printed, its useful to also have the 'acquired
lock' message printed. How about replacing LOG.debug(lockAcquiredMsg); with
SessionState.getConsole().logInfo(lockAcquiredMsg) ?
> Log message when HS2 query is waiting on compile lock
> -----------------------------------------------------
>
> Key: HIVE-14263
> URL: https://issues.apache.org/jira/browse/HIVE-14263
> Project: Hive
> Issue Type: Bug
> Components: HiveServer2
> Reporter: Thejas M Nair
> Assignee: Tao Li
> Attachments: HIVE-14263.1.patch
>
>
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
