[
https://issues.apache.org/jira/browse/HIVE-24853?focusedWorklogId=562934&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-562934
]
ASF GitHub Bot logged work on HIVE-24853:
-----------------------------------------
Author: ASF GitHub Bot
Created on: 09/Mar/21 09:24
Start Date: 09/Mar/21 09:24
Worklog Time Spent: 10m
Work Description: kgyrtkirk commented on a change in pull request #2044:
URL: https://github.com/apache/hive/pull/2044#discussion_r589355466
##########
File path:
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
##########
@@ -390,13 +394,16 @@ public void alterTable(RawStore msdb, Warehouse wh,
String catName, String dbnam
part.getValues(), oldCols, oldt, part, null, null);
assert (colStats.isEmpty());
if (cascade) {
Review comment:
I think it will enough to have 1 call in this method - either here above
the if or keep the one after the block
##########
File path:
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetastoreDirectSqlUtils.java
##########
@@ -138,23 +138,30 @@ static void timingTrace(boolean doTrace, String
queryText, long start, long quer
List<Object[]> list = ensureList(result);
Iterator<Object[]> iter = list.iterator();
Object[] fields = null;
- for (Map.Entry<Long, T> entry : tree.entrySet()) {
- if (fields == null && !iter.hasNext()) break;
- long id = entry.getKey();
- while (fields != null || iter.hasNext()) {
- if (fields == null) {
- fields = iter.next();
+ int rv = 0;
+ try {
+ for (Map.Entry<Long, T> entry : tree.entrySet()) {
+ if (fields == null && !iter.hasNext())
+ break;
+ long id = entry.getKey();
+ while (fields != null || iter.hasNext()) {
+ if (fields == null) {
+ fields = iter.next();
+ }
+ long nestedId = extractSqlLong(fields[keyIndex]);
+ if (nestedId < id)
Review comment:
please follow the hive formatter braces (`{}`) are mandatory for even 1
line if blocks as well
##########
File path:
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
##########
@@ -355,8 +357,10 @@ public void alterTable(RawStore msdb, Warehouse wh, String
catName, String dbnam
writeIdList, newt.getWriteId());
}
} else {
+ Deadline.checkTimeout();
Review comment:
I think 1 check should be enough in every loop
a few lines above the `for (Entry<Partition, ColumnStatistics> partColStats
: columnStatsNeedUpdated.entries()) {`
doesnt have any checks
##########
File path:
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java
##########
@@ -1783,11 +1783,15 @@ private long partsFoundForPartitions(
MetastoreDirectSqlUtils.timingTrace(doTrace, queryText, start, end);
List<Object[]> list = MetastoreDirectSqlUtils.ensureList(qResult);
List<ColumnStatisticsObj> colStats = new
ArrayList<ColumnStatisticsObj>(list.size());
- for (Object[] row : list) {
- colStats.add(prepareCSObjWithAdjustedNDV(row, 0,
useDensityFunctionForNDVEstimation, ndvTuner));
- Deadline.checkTimeout();
+ try {
Review comment:
instead of try-finally ; can't we switch to try-with-resources for the
query object ?
note that for example `ensureList` may also throw an exception in some cases
- and in that particular case the query will not be closed...
##########
File path:
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
##########
@@ -559,11 +567,14 @@ public Partition alterPartition(RawStore msdb, Warehouse
wh, String catName, Str
// PartitionView does not have SD. We do not need update its column
stats
if (oldPart.getSd() != null) {
+ Deadline.checkTimeout();
Review comment:
I think we should be ok to only check this once in every for loop
##########
File path:
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java
##########
@@ -1814,22 +1818,26 @@ private long partsFoundForPartitions(
List<String> noExtraColumnNames = new ArrayList<String>();
Map<String, String[]> extraColumnNameTypeParts = new HashMap<String,
String[]>();
List<Object[]> list = MetastoreDirectSqlUtils.ensureList(qResult);
- for (Object[] row : list) {
- String colName = (String) row[0];
- String colType = (String) row[1];
- // Extrapolation is not needed for this column if
- // count(\"PARTITION_NAME\")==partNames.size()
- // Or, extrapolation is not possible for this column if
- // count(\"PARTITION_NAME\")<2
- Long count = MetastoreDirectSqlUtils.extractSqlLong(row[2]);
- if (count == partNames.size() || count < 2) {
- noExtraColumnNames.add(colName);
- } else {
- extraColumnNameTypeParts.put(colName, new String[] { colType,
String.valueOf(count) });
+ try {
+ for (Object[] row : list) {
+ String colName = (String) row[0];
Review comment:
try-with-resources for `query`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Issue Time Tracking
-------------------
Worklog Id: (was: 562934)
Time Spent: 50m (was: 40m)
> HMS leaks queries in case of timeout
> ------------------------------------
>
> Key: HIVE-24853
> URL: https://issues.apache.org/jira/browse/HIVE-24853
> Project: Hive
> Issue Type: Bug
> Reporter: Ayush Saxena
> Assignee: Ayush Saxena
> Priority: Major
> Labels: pull-request-available
> Time Spent: 50m
> Remaining Estimate: 0h
>
> The queries aren't closed in case of timeout.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
