[jira] [Work logged] (HIVE-23266) Remove QueryWrapper from ObjectStore
[
https://issues.apache.org/jira/browse/HIVE-23266?focusedWorklogId=445244=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-445244
]
ASF GitHub Bot logged work on HIVE-23266:
-
Author: ASF GitHub Bot
Created on: 13/Jun/20 01:02
Start Date: 13/Jun/20 01:02
Worklog Time Spent: 10m
Work Description: belugabehr merged pull request #1078:
URL: https://github.com/apache/hive/pull/1078
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: 445244)
Time Spent: 2h 50m (was: 2h 40m)
> Remove QueryWrapper from ObjectStore
>
>
> Key: HIVE-23266
> URL: https://issues.apache.org/jira/browse/HIVE-23266
> Project: Hive
> Issue Type: Improvement
>Reporter: David Mollitor
>Assignee: David Mollitor
>Priority: Major
> Labels: pull-request-available
> Attachments: HIVE-23266.1.patch, HIVE-23266.10.patch,
> HIVE-23266.11.patch, HIVE-23266.2.patch, HIVE-23266.2.patch,
> HIVE-23266.3.patch, HIVE-23266.4.patch, HIVE-23266.5.patch,
> HIVE-23266.6.patch, HIVE-23266.7.patch, HIVE-23266.8.patch, HIVE-23266.9.patch
>
> Time Spent: 2h 50m
> Remaining Estimate: 0h
>
> There is currently a utility called {{QueryWrapper}} that makes a normal
> {{Query}} auto-closable. However, {{Query}} is now in fact already
> auto-closing, so there is no need for this class. In trying to remove it, I
> realized that this wrapper was being passed around in pretty convoluted ways
> and also it was sometimes being created in a {{try-with-resources}} block but
> then never actually used in any way.
> Remove the {{QueryWrapper}} from the class and simplify some of the DB
> interactions.
> https://github.com/apache/hive/blob/master/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java#L178
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Work logged] (HIVE-23266) Remove QueryWrapper from ObjectStore
[
https://issues.apache.org/jira/browse/HIVE-23266?focusedWorklogId=444928=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-444928
]
ASF GitHub Bot logged work on HIVE-23266:
-
Author: ASF GitHub Bot
Created on: 12/Jun/20 14:40
Start Date: 12/Jun/20 14:40
Worklog Time Spent: 10m
Work Description: belugabehr commented on pull request #1078:
URL: https://github.com/apache/hive/pull/1078#issuecomment-643307079
@dengzhhu653 Really, thanks so much for the review. Greatly appreciated.
Happy to get this class moving in the right direction.
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: 444928)
Time Spent: 2h 40m (was: 2.5h)
> Remove QueryWrapper from ObjectStore
>
>
> Key: HIVE-23266
> URL: https://issues.apache.org/jira/browse/HIVE-23266
> Project: Hive
> Issue Type: Improvement
>Reporter: David Mollitor
>Assignee: David Mollitor
>Priority: Major
> Labels: pull-request-available
> Attachments: HIVE-23266.1.patch, HIVE-23266.10.patch,
> HIVE-23266.11.patch, HIVE-23266.2.patch, HIVE-23266.2.patch,
> HIVE-23266.3.patch, HIVE-23266.4.patch, HIVE-23266.5.patch,
> HIVE-23266.6.patch, HIVE-23266.7.patch, HIVE-23266.8.patch, HIVE-23266.9.patch
>
> Time Spent: 2h 40m
> Remaining Estimate: 0h
>
> There is currently a utility called {{QueryWrapper}} that makes a normal
> {{Query}} auto-closable. However, {{Query}} is now in fact already
> auto-closing, so there is no need for this class. In trying to remove it, I
> realized that this wrapper was being passed around in pretty convoluted ways
> and also it was sometimes being created in a {{try-with-resources}} block but
> then never actually used in any way.
> Remove the {{QueryWrapper}} from the class and simplify some of the DB
> interactions.
> https://github.com/apache/hive/blob/master/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java#L178
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Work logged] (HIVE-23266) Remove QueryWrapper from ObjectStore
[
https://issues.apache.org/jira/browse/HIVE-23266?focusedWorklogId=444853=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-444853
]
ASF GitHub Bot logged work on HIVE-23266:
-
Author: ASF GitHub Bot
Created on: 12/Jun/20 12:46
Start Date: 12/Jun/20 12:46
Worklog Time Spent: 10m
Work Description: dengzhhu653 commented on pull request #1078:
URL: https://github.com/apache/hive/pull/1078#issuecomment-643251826
LGTM, +1. Thanks @belugabehr
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: 444853)
Time Spent: 2.5h (was: 2h 20m)
> Remove QueryWrapper from ObjectStore
>
>
> Key: HIVE-23266
> URL: https://issues.apache.org/jira/browse/HIVE-23266
> Project: Hive
> Issue Type: Improvement
>Reporter: David Mollitor
>Assignee: David Mollitor
>Priority: Major
> Labels: pull-request-available
> Attachments: HIVE-23266.1.patch, HIVE-23266.10.patch,
> HIVE-23266.11.patch, HIVE-23266.2.patch, HIVE-23266.2.patch,
> HIVE-23266.3.patch, HIVE-23266.4.patch, HIVE-23266.5.patch,
> HIVE-23266.6.patch, HIVE-23266.7.patch, HIVE-23266.8.patch, HIVE-23266.9.patch
>
> Time Spent: 2.5h
> Remaining Estimate: 0h
>
> There is currently a utility called {{QueryWrapper}} that makes a normal
> {{Query}} auto-closable. However, {{Query}} is now in fact already
> auto-closing, so there is no need for this class. In trying to remove it, I
> realized that this wrapper was being passed around in pretty convoluted ways
> and also it was sometimes being created in a {{try-with-resources}} block but
> then never actually used in any way.
> Remove the {{QueryWrapper}} from the class and simplify some of the DB
> interactions.
> https://github.com/apache/hive/blob/master/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java#L178
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Work logged] (HIVE-23266) Remove QueryWrapper from ObjectStore
[
https://issues.apache.org/jira/browse/HIVE-23266?focusedWorklogId=444618=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-444618
]
ASF GitHub Bot logged work on HIVE-23266:
-
Author: ASF GitHub Bot
Created on: 11/Jun/20 23:09
Start Date: 11/Jun/20 23:09
Worklog Time Spent: 10m
Work Description: dengzhhu653 commented on a change in pull request #1078:
URL: https://github.com/apache/hive/pull/1078#discussion_r439118943
##
File path:
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
##
@@ -7109,22 +7121,37 @@ protected String describeResult() {
}
@Override
- public List listPrincipalDBGrantsAll(
- String principalName, PrincipalType principalType) {
-try (QueryWrapper queryWrapper = new QueryWrapper()) {
- return convertDB(listPrincipalAllDBGrant(principalName, principalType,
queryWrapper));
+ public List listPrincipalDBGrantsAll(String
principalName, PrincipalType principalType) {
+List results = Collections.emptyList();
+try {
+ openTransaction();
+ results = convertDB(listPrincipalAllDBGrant(principalName,
principalType));
+ commitTransaction();
+} catch (Exception e) {
+ throw new RuntimeException(e);
+} finally {
+ rollbackAndCleanup(true, null);
Review comment:
I found that using ```try (QueryWrapper wrapper = new QueryWrapper())```
does not need to add a catch clause comparing to ```try(Query q =
pm.newQuery())```.
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: 444618)
Time Spent: 2h 20m (was: 2h 10m)
> Remove QueryWrapper from ObjectStore
>
>
> Key: HIVE-23266
> URL: https://issues.apache.org/jira/browse/HIVE-23266
> Project: Hive
> Issue Type: Improvement
>Reporter: David Mollitor
>Assignee: David Mollitor
>Priority: Major
> Labels: pull-request-available
> Attachments: HIVE-23266.1.patch, HIVE-23266.10.patch,
> HIVE-23266.11.patch, HIVE-23266.2.patch, HIVE-23266.2.patch,
> HIVE-23266.3.patch, HIVE-23266.4.patch, HIVE-23266.5.patch,
> HIVE-23266.6.patch, HIVE-23266.7.patch, HIVE-23266.8.patch, HIVE-23266.9.patch
>
> Time Spent: 2h 20m
> Remaining Estimate: 0h
>
> There is currently a utility called {{QueryWrapper}} that makes a normal
> {{Query}} auto-closable. However, {{Query}} is now in fact already
> auto-closing, so there is no need for this class. In trying to remove it, I
> realized that this wrapper was being passed around in pretty convoluted ways
> and also it was sometimes being created in a {{try-with-resources}} block but
> then never actually used in any way.
> Remove the {{QueryWrapper}} from the class and simplify some of the DB
> interactions.
> https://github.com/apache/hive/blob/master/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java#L178
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Work logged] (HIVE-23266) Remove QueryWrapper from ObjectStore
[
https://issues.apache.org/jira/browse/HIVE-23266?focusedWorklogId=444374=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-444374
]
ASF GitHub Bot logged work on HIVE-23266:
-
Author: ASF GitHub Bot
Created on: 11/Jun/20 16:18
Start Date: 11/Jun/20 16:18
Worklog Time Spent: 10m
Work Description: belugabehr commented on a change in pull request #1078:
URL: https://github.com/apache/hive/pull/1078#discussion_r438909147
##
File path:
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
##
@@ -7109,22 +7121,37 @@ protected String describeResult() {
}
@Override
- public List listPrincipalDBGrantsAll(
- String principalName, PrincipalType principalType) {
-try (QueryWrapper queryWrapper = new QueryWrapper()) {
- return convertDB(listPrincipalAllDBGrant(principalName, principalType,
queryWrapper));
+ public List listPrincipalDBGrantsAll(String
principalName, PrincipalType principalType) {
+List results = Collections.emptyList();
+try {
+ openTransaction();
+ results = convertDB(listPrincipalAllDBGrant(principalName,
principalType));
+ commitTransaction();
+} catch (Exception e) {
+ throw new RuntimeException(e);
+} finally {
+ rollbackAndCleanup(true, null);
Review comment:
@dengzhhu653 OK, I am following you now. I will address this is new
push. Thanks so much.
This whole setup is pretty wonky (hence my inspiration to simplify the Query
close stuff in this PR) and I'd like to address, but in another JIRA.
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: 444374)
Time Spent: 2h 10m (was: 2h)
> Remove QueryWrapper from ObjectStore
>
>
> Key: HIVE-23266
> URL: https://issues.apache.org/jira/browse/HIVE-23266
> Project: Hive
> Issue Type: Improvement
>Reporter: David Mollitor
>Assignee: David Mollitor
>Priority: Major
> Labels: pull-request-available
> Attachments: HIVE-23266.1.patch, HIVE-23266.10.patch,
> HIVE-23266.11.patch, HIVE-23266.2.patch, HIVE-23266.2.patch,
> HIVE-23266.3.patch, HIVE-23266.4.patch, HIVE-23266.5.patch,
> HIVE-23266.6.patch, HIVE-23266.7.patch, HIVE-23266.8.patch, HIVE-23266.9.patch
>
> Time Spent: 2h 10m
> Remaining Estimate: 0h
>
> There is currently a utility called {{QueryWrapper}} that makes a normal
> {{Query}} auto-closable. However, {{Query}} is now in fact already
> auto-closing, so there is no need for this class. In trying to remove it, I
> realized that this wrapper was being passed around in pretty convoluted ways
> and also it was sometimes being created in a {{try-with-resources}} block but
> then never actually used in any way.
> Remove the {{QueryWrapper}} from the class and simplify some of the DB
> interactions.
> https://github.com/apache/hive/blob/master/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java#L178
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Work logged] (HIVE-23266) Remove QueryWrapper from ObjectStore
[
https://issues.apache.org/jira/browse/HIVE-23266?focusedWorklogId=444370=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-444370
]
ASF GitHub Bot logged work on HIVE-23266:
-
Author: ASF GitHub Bot
Created on: 11/Jun/20 16:16
Start Date: 11/Jun/20 16:16
Worklog Time Spent: 10m
Work Description: belugabehr commented on a change in pull request #1078:
URL: https://github.com/apache/hive/pull/1078#discussion_r438909147
##
File path:
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
##
@@ -7109,22 +7121,37 @@ protected String describeResult() {
}
@Override
- public List listPrincipalDBGrantsAll(
- String principalName, PrincipalType principalType) {
-try (QueryWrapper queryWrapper = new QueryWrapper()) {
- return convertDB(listPrincipalAllDBGrant(principalName, principalType,
queryWrapper));
+ public List listPrincipalDBGrantsAll(String
principalName, PrincipalType principalType) {
+List results = Collections.emptyList();
+try {
+ openTransaction();
+ results = convertDB(listPrincipalAllDBGrant(principalName,
principalType));
+ commitTransaction();
+} catch (Exception e) {
+ throw new RuntimeException(e);
+} finally {
+ rollbackAndCleanup(true, null);
Review comment:
@dengzhhu653 OK, I am following you now. I will address this is new
push. Thanks so much.
This whole setup is pretty wonky and I'd like to address, but in another
JIRA.
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: 444370)
Time Spent: 2h (was: 1h 50m)
> Remove QueryWrapper from ObjectStore
>
>
> Key: HIVE-23266
> URL: https://issues.apache.org/jira/browse/HIVE-23266
> Project: Hive
> Issue Type: Improvement
>Reporter: David Mollitor
>Assignee: David Mollitor
>Priority: Major
> Labels: pull-request-available
> Attachments: HIVE-23266.1.patch, HIVE-23266.10.patch,
> HIVE-23266.11.patch, HIVE-23266.2.patch, HIVE-23266.2.patch,
> HIVE-23266.3.patch, HIVE-23266.4.patch, HIVE-23266.5.patch,
> HIVE-23266.6.patch, HIVE-23266.7.patch, HIVE-23266.8.patch, HIVE-23266.9.patch
>
> Time Spent: 2h
> Remaining Estimate: 0h
>
> There is currently a utility called {{QueryWrapper}} that makes a normal
> {{Query}} auto-closable. However, {{Query}} is now in fact already
> auto-closing, so there is no need for this class. In trying to remove it, I
> realized that this wrapper was being passed around in pretty convoluted ways
> and also it was sometimes being created in a {{try-with-resources}} block but
> then never actually used in any way.
> Remove the {{QueryWrapper}} from the class and simplify some of the DB
> interactions.
> https://github.com/apache/hive/blob/master/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java#L178
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Work logged] (HIVE-23266) Remove QueryWrapper from ObjectStore
[
https://issues.apache.org/jira/browse/HIVE-23266?focusedWorklogId=444340=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-444340
]
ASF GitHub Bot logged work on HIVE-23266:
-
Author: ASF GitHub Bot
Created on: 11/Jun/20 15:30
Start Date: 11/Jun/20 15:30
Worklog Time Spent: 10m
Work Description: dengzhhu653 commented on a change in pull request #1078:
URL: https://github.com/apache/hive/pull/1078#discussion_r438872802
##
File path:
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
##
@@ -7109,22 +7121,37 @@ protected String describeResult() {
}
@Override
- public List listPrincipalDBGrantsAll(
- String principalName, PrincipalType principalType) {
-try (QueryWrapper queryWrapper = new QueryWrapper()) {
- return convertDB(listPrincipalAllDBGrant(principalName, principalType,
queryWrapper));
+ public List listPrincipalDBGrantsAll(String
principalName, PrincipalType principalType) {
+List results = Collections.emptyList();
+try {
+ openTransaction();
+ results = convertDB(listPrincipalAllDBGrant(principalName,
principalType));
+ commitTransaction();
+} catch (Exception e) {
+ throw new RuntimeException(e);
+} finally {
+ rollbackAndCleanup(true, null);
Review comment:
When exception happens in this method, if the same ObjectStore tries to
create table afterwards, he may be unable to commit the transaction as
```openTrasactionCalls``` should be greater than 0 or rollback as the method
```commitTransaction``` returns true, the modification would be lost when
metastore client shutdown.
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: 444340)
Time Spent: 1h 50m (was: 1h 40m)
> Remove QueryWrapper from ObjectStore
>
>
> Key: HIVE-23266
> URL: https://issues.apache.org/jira/browse/HIVE-23266
> Project: Hive
> Issue Type: Improvement
>Reporter: David Mollitor
>Assignee: David Mollitor
>Priority: Major
> Labels: pull-request-available
> Attachments: HIVE-23266.1.patch, HIVE-23266.10.patch,
> HIVE-23266.11.patch, HIVE-23266.2.patch, HIVE-23266.2.patch,
> HIVE-23266.3.patch, HIVE-23266.4.patch, HIVE-23266.5.patch,
> HIVE-23266.6.patch, HIVE-23266.7.patch, HIVE-23266.8.patch, HIVE-23266.9.patch
>
> Time Spent: 1h 50m
> Remaining Estimate: 0h
>
> There is currently a utility called {{QueryWrapper}} that makes a normal
> {{Query}} auto-closable. However, {{Query}} is now in fact already
> auto-closing, so there is no need for this class. In trying to remove it, I
> realized that this wrapper was being passed around in pretty convoluted ways
> and also it was sometimes being created in a {{try-with-resources}} block but
> then never actually used in any way.
> Remove the {{QueryWrapper}} from the class and simplify some of the DB
> interactions.
> https://github.com/apache/hive/blob/master/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java#L178
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Work logged] (HIVE-23266) Remove QueryWrapper from ObjectStore
[
https://issues.apache.org/jira/browse/HIVE-23266?focusedWorklogId=444306=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-444306
]
ASF GitHub Bot logged work on HIVE-23266:
-
Author: ASF GitHub Bot
Created on: 11/Jun/20 14:52
Start Date: 11/Jun/20 14:52
Worklog Time Spent: 10m
Work Description: belugabehr commented on a change in pull request #1078:
URL: https://github.com/apache/hive/pull/1078#discussion_r438844956
##
File path:
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
##
@@ -7109,22 +7121,37 @@ protected String describeResult() {
}
@Override
- public List listPrincipalDBGrantsAll(
- String principalName, PrincipalType principalType) {
-try (QueryWrapper queryWrapper = new QueryWrapper()) {
- return convertDB(listPrincipalAllDBGrant(principalName, principalType,
queryWrapper));
+ public List listPrincipalDBGrantsAll(String
principalName, PrincipalType principalType) {
+List results = Collections.emptyList();
+try {
+ openTransaction();
+ results = convertDB(listPrincipalAllDBGrant(principalName,
principalType));
+ commitTransaction();
+} catch (Exception e) {
+ throw new RuntimeException(e);
+} finally {
+ rollbackAndCleanup(true, null);
Review comment:
@dengzhhu653
This particular method is a top-level method and it is read-only.
Therefore, there will never be anything to roll back.
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: 444306)
Time Spent: 1h 40m (was: 1.5h)
> Remove QueryWrapper from ObjectStore
>
>
> Key: HIVE-23266
> URL: https://issues.apache.org/jira/browse/HIVE-23266
> Project: Hive
> Issue Type: Improvement
>Reporter: David Mollitor
>Assignee: David Mollitor
>Priority: Major
> Labels: pull-request-available
> Attachments: HIVE-23266.1.patch, HIVE-23266.10.patch,
> HIVE-23266.11.patch, HIVE-23266.2.patch, HIVE-23266.2.patch,
> HIVE-23266.3.patch, HIVE-23266.4.patch, HIVE-23266.5.patch,
> HIVE-23266.6.patch, HIVE-23266.7.patch, HIVE-23266.8.patch, HIVE-23266.9.patch
>
> Time Spent: 1h 40m
> Remaining Estimate: 0h
>
> There is currently a utility called {{QueryWrapper}} that makes a normal
> {{Query}} auto-closable. However, {{Query}} is now in fact already
> auto-closing, so there is no need for this class. In trying to remove it, I
> realized that this wrapper was being passed around in pretty convoluted ways
> and also it was sometimes being created in a {{try-with-resources}} block but
> then never actually used in any way.
> Remove the {{QueryWrapper}} from the class and simplify some of the DB
> interactions.
> https://github.com/apache/hive/blob/master/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java#L178
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Work logged] (HIVE-23266) Remove QueryWrapper from ObjectStore
[
https://issues.apache.org/jira/browse/HIVE-23266?focusedWorklogId=444305=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-444305
]
ASF GitHub Bot logged work on HIVE-23266:
-
Author: ASF GitHub Bot
Created on: 11/Jun/20 14:51
Start Date: 11/Jun/20 14:51
Worklog Time Spent: 10m
Work Description: belugabehr commented on a change in pull request #1078:
URL: https://github.com/apache/hive/pull/1078#discussion_r438844525
##
File path:
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
##
@@ -7109,22 +7121,37 @@ protected String describeResult() {
}
@Override
- public List listPrincipalDBGrantsAll(
- String principalName, PrincipalType principalType) {
-try (QueryWrapper queryWrapper = new QueryWrapper()) {
- return convertDB(listPrincipalAllDBGrant(principalName, principalType,
queryWrapper));
+ public List listPrincipalDBGrantsAll(String
principalName, PrincipalType principalType) {
+List results = Collections.emptyList();
+try {
+ openTransaction();
+ results = convertDB(listPrincipalAllDBGrant(principalName,
principalType));
+ commitTransaction();
+} catch (Exception e) {
+ throw new RuntimeException(e);
+} finally {
+ rollbackAndCleanup(true, null);
}
+return results;
}
@Override
public List listDBGrantsAll(String catName, String
dbName) {
-return listDBGrantsAll(catName, dbName, null);
+List results = Collections.emptyList();
+try {
+ openTransaction();
+ results = listDBGrantsAll(catName, dbName, null);
+ commitTransaction();
+} catch (Exception e) {
+ throw new RuntimeException(e);
+} finally {
+ rollbackAndCleanup(true, null);
Review comment:
OK. I just took a look at it. 'true' here means 'do not roll back'.
This particular method is a top-level method and it is read-only. Therefore,
there will never be anything to roll back.
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: 444305)
Time Spent: 1.5h (was: 1h 20m)
> Remove QueryWrapper from ObjectStore
>
>
> Key: HIVE-23266
> URL: https://issues.apache.org/jira/browse/HIVE-23266
> Project: Hive
> Issue Type: Improvement
>Reporter: David Mollitor
>Assignee: David Mollitor
>Priority: Major
> Labels: pull-request-available
> Attachments: HIVE-23266.1.patch, HIVE-23266.10.patch,
> HIVE-23266.11.patch, HIVE-23266.2.patch, HIVE-23266.2.patch,
> HIVE-23266.3.patch, HIVE-23266.4.patch, HIVE-23266.5.patch,
> HIVE-23266.6.patch, HIVE-23266.7.patch, HIVE-23266.8.patch, HIVE-23266.9.patch
>
> Time Spent: 1.5h
> Remaining Estimate: 0h
>
> There is currently a utility called {{QueryWrapper}} that makes a normal
> {{Query}} auto-closable. However, {{Query}} is now in fact already
> auto-closing, so there is no need for this class. In trying to remove it, I
> realized that this wrapper was being passed around in pretty convoluted ways
> and also it was sometimes being created in a {{try-with-resources}} block but
> then never actually used in any way.
> Remove the {{QueryWrapper}} from the class and simplify some of the DB
> interactions.
> https://github.com/apache/hive/blob/master/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java#L178
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Work logged] (HIVE-23266) Remove QueryWrapper from ObjectStore
[
https://issues.apache.org/jira/browse/HIVE-23266?focusedWorklogId=443479=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-443479
]
ASF GitHub Bot logged work on HIVE-23266:
-
Author: ASF GitHub Bot
Created on: 10/Jun/20 00:01
Start Date: 10/Jun/20 00:01
Worklog Time Spent: 10m
Work Description: dengzhhu653 commented on a change in pull request #1078:
URL: https://github.com/apache/hive/pull/1078#discussion_r437786895
##
File path:
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/VerifyingObjectStore.java
##
@@ -93,9 +93,11 @@ public boolean getPartitionsByExpr(String catName, String
dbName, String tblName
@Override
public List getPartitions(
String catName, String dbName, String tableName, int maxParts) throws
MetaException, NoSuchObjectException {
+openTransaction();
List sqlResults = getPartitionsInternal(catName, dbName,
tableName, maxParts, true, false);
List ormResults = getPartitionsInternal(catName, dbName,
tableName, maxParts, false, true);
verifyLists(sqlResults, ormResults, Partition.class);
+commitTransaction();
Review comment:
Got it.
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: 443479)
Time Spent: 1h 10m (was: 1h)
> Remove QueryWrapper from ObjectStore
>
>
> Key: HIVE-23266
> URL: https://issues.apache.org/jira/browse/HIVE-23266
> Project: Hive
> Issue Type: Improvement
>Reporter: David Mollitor
>Assignee: David Mollitor
>Priority: Major
> Labels: pull-request-available
> Attachments: HIVE-23266.1.patch, HIVE-23266.10.patch,
> HIVE-23266.11.patch, HIVE-23266.2.patch, HIVE-23266.2.patch,
> HIVE-23266.3.patch, HIVE-23266.4.patch, HIVE-23266.5.patch,
> HIVE-23266.6.patch, HIVE-23266.7.patch, HIVE-23266.8.patch, HIVE-23266.9.patch
>
> Time Spent: 1h 10m
> Remaining Estimate: 0h
>
> There is currently a utility called {{QueryWrapper}} that makes a normal
> {{Query}} auto-closable. However, {{Query}} is now in fact already
> auto-closing, so there is no need for this class. In trying to remove it, I
> realized that this wrapper was being passed around in pretty convoluted ways
> and also it was sometimes being created in a {{try-with-resources}} block but
> then never actually used in any way.
> Remove the {{QueryWrapper}} from the class and simplify some of the DB
> interactions.
> https://github.com/apache/hive/blob/master/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java#L178
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Work logged] (HIVE-23266) Remove QueryWrapper from ObjectStore
[
https://issues.apache.org/jira/browse/HIVE-23266?focusedWorklogId=443480=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-443480
]
ASF GitHub Bot logged work on HIVE-23266:
-
Author: ASF GitHub Bot
Created on: 10/Jun/20 00:01
Start Date: 10/Jun/20 00:01
Worklog Time Spent: 10m
Work Description: dengzhhu653 commented on a change in pull request #1078:
URL: https://github.com/apache/hive/pull/1078#discussion_r437786895
##
File path:
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/VerifyingObjectStore.java
##
@@ -93,9 +93,11 @@ public boolean getPartitionsByExpr(String catName, String
dbName, String tblName
@Override
public List getPartitions(
String catName, String dbName, String tableName, int maxParts) throws
MetaException, NoSuchObjectException {
+openTransaction();
List sqlResults = getPartitionsInternal(catName, dbName,
tableName, maxParts, true, false);
List ormResults = getPartitionsInternal(catName, dbName,
tableName, maxParts, false, true);
verifyLists(sqlResults, ormResults, Partition.class);
+commitTransaction();
Review comment:
OK, got it.
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: 443480)
Time Spent: 1h 20m (was: 1h 10m)
> Remove QueryWrapper from ObjectStore
>
>
> Key: HIVE-23266
> URL: https://issues.apache.org/jira/browse/HIVE-23266
> Project: Hive
> Issue Type: Improvement
>Reporter: David Mollitor
>Assignee: David Mollitor
>Priority: Major
> Labels: pull-request-available
> Attachments: HIVE-23266.1.patch, HIVE-23266.10.patch,
> HIVE-23266.11.patch, HIVE-23266.2.patch, HIVE-23266.2.patch,
> HIVE-23266.3.patch, HIVE-23266.4.patch, HIVE-23266.5.patch,
> HIVE-23266.6.patch, HIVE-23266.7.patch, HIVE-23266.8.patch, HIVE-23266.9.patch
>
> Time Spent: 1h 20m
> Remaining Estimate: 0h
>
> There is currently a utility called {{QueryWrapper}} that makes a normal
> {{Query}} auto-closable. However, {{Query}} is now in fact already
> auto-closing, so there is no need for this class. In trying to remove it, I
> realized that this wrapper was being passed around in pretty convoluted ways
> and also it was sometimes being created in a {{try-with-resources}} block but
> then never actually used in any way.
> Remove the {{QueryWrapper}} from the class and simplify some of the DB
> interactions.
> https://github.com/apache/hive/blob/master/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java#L178
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Work logged] (HIVE-23266) Remove QueryWrapper from ObjectStore
[
https://issues.apache.org/jira/browse/HIVE-23266?focusedWorklogId=443478=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-443478
]
ASF GitHub Bot logged work on HIVE-23266:
-
Author: ASF GitHub Bot
Created on: 09/Jun/20 23:59
Start Date: 09/Jun/20 23:59
Worklog Time Spent: 10m
Work Description: dengzhhu653 commented on a change in pull request #1078:
URL: https://github.com/apache/hive/pull/1078#discussion_r437786344
##
File path:
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
##
@@ -7109,22 +7121,37 @@ protected String describeResult() {
}
@Override
- public List listPrincipalDBGrantsAll(
- String principalName, PrincipalType principalType) {
-try (QueryWrapper queryWrapper = new QueryWrapper()) {
- return convertDB(listPrincipalAllDBGrant(principalName, principalType,
queryWrapper));
+ public List listPrincipalDBGrantsAll(String
principalName, PrincipalType principalType) {
+List results = Collections.emptyList();
+try {
+ openTransaction();
+ results = convertDB(listPrincipalAllDBGrant(principalName,
principalType));
+ commitTransaction();
+} catch (Exception e) {
+ throw new RuntimeException(e);
+} finally {
+ rollbackAndCleanup(true, null);
}
+return results;
}
@Override
public List listDBGrantsAll(String catName, String
dbName) {
-return listDBGrantsAll(catName, dbName, null);
+List results = Collections.emptyList();
+try {
+ openTransaction();
+ results = listDBGrantsAll(catName, dbName, null);
+ commitTransaction();
+} catch (Exception e) {
+ throw new RuntimeException(e);
+} finally {
+ rollbackAndCleanup(true, null);
Review comment:
Sorry for the delay, my concern is that set success to true on
```rollbackAndCleanup``` here may make the transaction unable to rollback.
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: 443478)
Time Spent: 1h (was: 50m)
> Remove QueryWrapper from ObjectStore
>
>
> Key: HIVE-23266
> URL: https://issues.apache.org/jira/browse/HIVE-23266
> Project: Hive
> Issue Type: Improvement
>Reporter: David Mollitor
>Assignee: David Mollitor
>Priority: Major
> Labels: pull-request-available
> Attachments: HIVE-23266.1.patch, HIVE-23266.10.patch,
> HIVE-23266.11.patch, HIVE-23266.2.patch, HIVE-23266.2.patch,
> HIVE-23266.3.patch, HIVE-23266.4.patch, HIVE-23266.5.patch,
> HIVE-23266.6.patch, HIVE-23266.7.patch, HIVE-23266.8.patch, HIVE-23266.9.patch
>
> Time Spent: 1h
> Remaining Estimate: 0h
>
> There is currently a utility called {{QueryWrapper}} that makes a normal
> {{Query}} auto-closable. However, {{Query}} is now in fact already
> auto-closing, so there is no need for this class. In trying to remove it, I
> realized that this wrapper was being passed around in pretty convoluted ways
> and also it was sometimes being created in a {{try-with-resources}} block but
> then never actually used in any way.
> Remove the {{QueryWrapper}} from the class and simplify some of the DB
> interactions.
> https://github.com/apache/hive/blob/master/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java#L178
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Work logged] (HIVE-23266) Remove QueryWrapper from ObjectStore
[
https://issues.apache.org/jira/browse/HIVE-23266?focusedWorklogId=443474=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-443474
]
ASF GitHub Bot logged work on HIVE-23266:
-
Author: ASF GitHub Bot
Created on: 09/Jun/20 23:48
Start Date: 09/Jun/20 23:48
Worklog Time Spent: 10m
Work Description: dengzhhu653 commented on a change in pull request #1078:
URL: https://github.com/apache/hive/pull/1078#discussion_r437783154
##
File path:
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
##
@@ -7109,22 +7121,37 @@ protected String describeResult() {
}
@Override
- public List listPrincipalDBGrantsAll(
- String principalName, PrincipalType principalType) {
-try (QueryWrapper queryWrapper = new QueryWrapper()) {
- return convertDB(listPrincipalAllDBGrant(principalName, principalType,
queryWrapper));
+ public List listPrincipalDBGrantsAll(String
principalName, PrincipalType principalType) {
+List results = Collections.emptyList();
+try {
+ openTransaction();
+ results = convertDB(listPrincipalAllDBGrant(principalName,
principalType));
+ commitTransaction();
+} catch (Exception e) {
+ throw new RuntimeException(e);
+} finally {
+ rollbackAndCleanup(true, null);
Review comment:
https://github.com/apache/hive/blob/871ee8009380e1bab160b58dc378a7f668c64584/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java#L11729-L11739
I'm wondering that the opened transaction may be unable to be committed when
exception happens on ```listPrincipalAllDBGrant``` or rolled back as we set
the ```success``` to true on the method ```rollbackAndCleanup```. Could you
explain more about why set ```success``` to true here? Thanks @belugabehr
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: 443474)
Time Spent: 50m (was: 40m)
> Remove QueryWrapper from ObjectStore
>
>
> Key: HIVE-23266
> URL: https://issues.apache.org/jira/browse/HIVE-23266
> Project: Hive
> Issue Type: Improvement
>Reporter: David Mollitor
>Assignee: David Mollitor
>Priority: Major
> Labels: pull-request-available
> Attachments: HIVE-23266.1.patch, HIVE-23266.10.patch,
> HIVE-23266.11.patch, HIVE-23266.2.patch, HIVE-23266.2.patch,
> HIVE-23266.3.patch, HIVE-23266.4.patch, HIVE-23266.5.patch,
> HIVE-23266.6.patch, HIVE-23266.7.patch, HIVE-23266.8.patch, HIVE-23266.9.patch
>
> Time Spent: 50m
> Remaining Estimate: 0h
>
> There is currently a utility called {{QueryWrapper}} that makes a normal
> {{Query}} auto-closable. However, {{Query}} is now in fact already
> auto-closing, so there is no need for this class. In trying to remove it, I
> realized that this wrapper was being passed around in pretty convoluted ways
> and also it was sometimes being created in a {{try-with-resources}} block but
> then never actually used in any way.
> Remove the {{QueryWrapper}} from the class and simplify some of the DB
> interactions.
> https://github.com/apache/hive/blob/master/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java#L178
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Work logged] (HIVE-23266) Remove QueryWrapper from ObjectStore
[
https://issues.apache.org/jira/browse/HIVE-23266?focusedWorklogId=442960=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-442960
]
ASF GitHub Bot logged work on HIVE-23266:
-
Author: ASF GitHub Bot
Created on: 09/Jun/20 16:16
Start Date: 09/Jun/20 16:16
Worklog Time Spent: 10m
Work Description: belugabehr opened a new pull request #1078:
URL: https://github.com/apache/hive/pull/1078
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: 442960)
Time Spent: 40m (was: 0.5h)
> Remove QueryWrapper from ObjectStore
>
>
> Key: HIVE-23266
> URL: https://issues.apache.org/jira/browse/HIVE-23266
> Project: Hive
> Issue Type: Improvement
>Reporter: David Mollitor
>Assignee: David Mollitor
>Priority: Major
> Labels: pull-request-available
> Attachments: HIVE-23266.1.patch, HIVE-23266.10.patch,
> HIVE-23266.11.patch, HIVE-23266.2.patch, HIVE-23266.2.patch,
> HIVE-23266.3.patch, HIVE-23266.4.patch, HIVE-23266.5.patch,
> HIVE-23266.6.patch, HIVE-23266.7.patch, HIVE-23266.8.patch, HIVE-23266.9.patch
>
> Time Spent: 40m
> Remaining Estimate: 0h
>
> There is currently a utility called {{QueryWrapper}} that makes a normal
> {{Query}} auto-closable. However, {{Query}} is now in fact already
> auto-closing, so there is no need for this class. In trying to remove it, I
> realized that this wrapper was being passed around in pretty convoluted ways
> and also it was sometimes being created in a {{try-with-resources}} block but
> then never actually used in any way.
> Remove the {{QueryWrapper}} from the class and simplify some of the DB
> interactions.
> https://github.com/apache/hive/blob/master/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java#L178
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Work logged] (HIVE-23266) Remove QueryWrapper from ObjectStore
[
https://issues.apache.org/jira/browse/HIVE-23266?focusedWorklogId=442825=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-442825
]
ASF GitHub Bot logged work on HIVE-23266:
-
Author: ASF GitHub Bot
Created on: 09/Jun/20 16:02
Start Date: 09/Jun/20 16:02
Worklog Time Spent: 10m
Work Description: dengzhhu653 commented on a change in pull request #1078:
URL: https://github.com/apache/hive/pull/1078#discussion_r437147690
##
File path:
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/VerifyingObjectStore.java
##
@@ -93,9 +93,11 @@ public boolean getPartitionsByExpr(String catName, String
dbName, String tblName
@Override
public List getPartitions(
String catName, String dbName, String tableName, int maxParts) throws
MetaException, NoSuchObjectException {
+openTransaction();
List sqlResults = getPartitionsInternal(catName, dbName,
tableName, maxParts, true, false);
List ormResults = getPartitionsInternal(catName, dbName,
tableName, maxParts, false, true);
verifyLists(sqlResults, ormResults, Partition.class);
+commitTransaction();
Review comment:
what if exception happens when getPartitionsInternal, the transaction
may become unbalanced
##
File path:
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
##
@@ -7109,22 +7121,37 @@ protected String describeResult() {
}
@Override
- public List listPrincipalDBGrantsAll(
- String principalName, PrincipalType principalType) {
-try (QueryWrapper queryWrapper = new QueryWrapper()) {
- return convertDB(listPrincipalAllDBGrant(principalName, principalType,
queryWrapper));
+ public List listPrincipalDBGrantsAll(String
principalName, PrincipalType principalType) {
+List results = Collections.emptyList();
+try {
+ openTransaction();
+ results = convertDB(listPrincipalAllDBGrant(principalName,
principalType));
+ commitTransaction();
+} catch (Exception e) {
+ throw new RuntimeException(e);
+} finally {
+ rollbackAndCleanup(true, null);
}
+return results;
}
@Override
public List listDBGrantsAll(String catName, String
dbName) {
-return listDBGrantsAll(catName, dbName, null);
+List results = Collections.emptyList();
+try {
+ openTransaction();
+ results = listDBGrantsAll(catName, dbName, null);
+ commitTransaction();
+} catch (Exception e) {
+ throw new RuntimeException(e);
+} finally {
+ rollbackAndCleanup(true, null);
Review comment:
Why pass true to rollbackAndCleanup here?
##
File path:
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
##
@@ -7109,22 +7121,37 @@ protected String describeResult() {
}
@Override
- public List listPrincipalDBGrantsAll(
- String principalName, PrincipalType principalType) {
-try (QueryWrapper queryWrapper = new QueryWrapper()) {
- return convertDB(listPrincipalAllDBGrant(principalName, principalType,
queryWrapper));
+ public List listPrincipalDBGrantsAll(String
principalName, PrincipalType principalType) {
+List results = Collections.emptyList();
+try {
+ openTransaction();
+ results = convertDB(listPrincipalAllDBGrant(principalName,
principalType));
+ commitTransaction();
+} catch (Exception e) {
+ throw new RuntimeException(e);
+} finally {
+ rollbackAndCleanup(true, null);
}
+return results;
}
@Override
public List listDBGrantsAll(String catName, String
dbName) {
-return listDBGrantsAll(catName, dbName, null);
+List results = Collections.emptyList();
+try {
+ openTransaction();
+ results = listDBGrantsAll(catName, dbName, null);
+ commitTransaction();
+} catch (Exception e) {
+ throw new RuntimeException(e);
+} finally {
+ rollbackAndCleanup(true, null);
Review comment:
rollbackAndCleanup
##
File path:
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
##
@@ -7109,22 +7121,37 @@ protected String describeResult() {
}
@Override
- public List listPrincipalDBGrantsAll(
- String principalName, PrincipalType principalType) {
-try (QueryWrapper queryWrapper = new QueryWrapper()) {
- return convertDB(listPrincipalAllDBGrant(principalName, principalType,
queryWrapper));
+ public List listPrincipalDBGrantsAll(String
principalName, PrincipalType principalType) {
+List results = Collections.emptyList();
+try {
+ openTransaction();
+ results = convertDB(listPrincipalAllDBGrant(principalName,
principalType));
+ commitTransaction();
+} catch (Exception e) {
+ throw new RuntimeException(e);
+
[jira] [Work logged] (HIVE-23266) Remove QueryWrapper from ObjectStore
[
https://issues.apache.org/jira/browse/HIVE-23266?focusedWorklogId=442806=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-442806
]
ASF GitHub Bot logged work on HIVE-23266:
-
Author: ASF GitHub Bot
Created on: 09/Jun/20 16:00
Start Date: 09/Jun/20 16:00
Worklog Time Spent: 10m
Work Description: belugabehr commented on pull request #1078:
URL: https://github.com/apache/hive/pull/1078#issuecomment-641327981
@dengzhhu653 Thanks *so* much for the feedback.
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: 442806)
Time Spent: 20m (was: 10m)
> Remove QueryWrapper from ObjectStore
>
>
> Key: HIVE-23266
> URL: https://issues.apache.org/jira/browse/HIVE-23266
> Project: Hive
> Issue Type: Improvement
>Reporter: David Mollitor
>Assignee: David Mollitor
>Priority: Major
> Labels: pull-request-available
> Attachments: HIVE-23266.1.patch, HIVE-23266.10.patch,
> HIVE-23266.11.patch, HIVE-23266.2.patch, HIVE-23266.2.patch,
> HIVE-23266.3.patch, HIVE-23266.4.patch, HIVE-23266.5.patch,
> HIVE-23266.6.patch, HIVE-23266.7.patch, HIVE-23266.8.patch, HIVE-23266.9.patch
>
> Time Spent: 20m
> Remaining Estimate: 0h
>
> There is currently a utility called {{QueryWrapper}} that makes a normal
> {{Query}} auto-closable. However, {{Query}} is now in fact already
> auto-closing, so there is no need for this class. In trying to remove it, I
> realized that this wrapper was being passed around in pretty convoluted ways
> and also it was sometimes being created in a {{try-with-resources}} block but
> then never actually used in any way.
> Remove the {{QueryWrapper}} from the class and simplify some of the DB
> interactions.
> https://github.com/apache/hive/blob/master/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java#L178
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Work logged] (HIVE-23266) Remove QueryWrapper from ObjectStore
[
https://issues.apache.org/jira/browse/HIVE-23266?focusedWorklogId=442746=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-442746
]
ASF GitHub Bot logged work on HIVE-23266:
-
Author: ASF GitHub Bot
Created on: 09/Jun/20 15:53
Start Date: 09/Jun/20 15:53
Worklog Time Spent: 10m
Work Description: belugabehr commented on a change in pull request #1078:
URL: https://github.com/apache/hive/pull/1078#discussion_r437458416
##
File path:
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/VerifyingObjectStore.java
##
@@ -93,9 +93,11 @@ public boolean getPartitionsByExpr(String catName, String
dbName, String tblName
@Override
public List getPartitions(
String catName, String dbName, String tableName, int maxParts) throws
MetaException, NoSuchObjectException {
+openTransaction();
List sqlResults = getPartitionsInternal(catName, dbName,
tableName, maxParts, true, false);
List ormResults = getPartitionsInternal(catName, dbName,
tableName, maxParts, false, true);
verifyLists(sqlResults, ormResults, Partition.class);
+commitTransaction();
Review comment:
This is a test class. If things become unbalanced here it doesn't
really matter because the test will fail from Exception anyway.
##
File path:
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
##
@@ -7109,22 +7121,37 @@ protected String describeResult() {
}
@Override
- public List listPrincipalDBGrantsAll(
- String principalName, PrincipalType principalType) {
-try (QueryWrapper queryWrapper = new QueryWrapper()) {
- return convertDB(listPrincipalAllDBGrant(principalName, principalType,
queryWrapper));
+ public List listPrincipalDBGrantsAll(String
principalName, PrincipalType principalType) {
+List results = Collections.emptyList();
+try {
+ openTransaction();
+ results = convertDB(listPrincipalAllDBGrant(principalName,
principalType));
+ commitTransaction();
+} catch (Exception e) {
+ throw new RuntimeException(e);
+} finally {
+ rollbackAndCleanup(true, null);
}
+return results;
}
@Override
public List listDBGrantsAll(String catName, String
dbName) {
-return listDBGrantsAll(catName, dbName, null);
+List results = Collections.emptyList();
+try {
+ openTransaction();
+ results = listDBGrantsAll(catName, dbName, null);
+ commitTransaction();
+} catch (Exception e) {
+ throw new RuntimeException(e);
+} finally {
+ rollbackAndCleanup(true, null);
Review comment:
I'm sorry?
##
File path:
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
##
@@ -7109,22 +7121,37 @@ protected String describeResult() {
}
@Override
- public List listPrincipalDBGrantsAll(
- String principalName, PrincipalType principalType) {
-try (QueryWrapper queryWrapper = new QueryWrapper()) {
- return convertDB(listPrincipalAllDBGrant(principalName, principalType,
queryWrapper));
+ public List listPrincipalDBGrantsAll(String
principalName, PrincipalType principalType) {
+List results = Collections.emptyList();
+try {
+ openTransaction();
+ results = convertDB(listPrincipalAllDBGrant(principalName,
principalType));
+ commitTransaction();
+} catch (Exception e) {
+ throw new RuntimeException(e);
+} finally {
+ rollbackAndCleanup(true, null);
Review comment:
Why is that?
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: 442746)
Remaining Estimate: 0h
Time Spent: 10m
> Remove QueryWrapper from ObjectStore
>
>
> Key: HIVE-23266
> URL: https://issues.apache.org/jira/browse/HIVE-23266
> Project: Hive
> Issue Type: Improvement
>Reporter: David Mollitor
>Assignee: David Mollitor
>Priority: Major
> Attachments: HIVE-23266.1.patch, HIVE-23266.10.patch,
> HIVE-23266.11.patch, HIVE-23266.2.patch, HIVE-23266.2.patch,
> HIVE-23266.3.patch, HIVE-23266.4.patch, HIVE-23266.5.patch,
> HIVE-23266.6.patch, HIVE-23266.7.patch, HIVE-23266.8.patch, HIVE-23266.9.patch
>
> Time Spent: 10m
> Remaining Estimate: 0h
>
> There is currently a utility called {{QueryWrapper}} that makes a normal
> {{Query}} auto-closable. However, {{Query}} is now in fact
