You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Dimitris Tsirogiannis (Code Review)" <ge...@cloudera.org> on 2018/02/06 18:11:31 UTC

[Impala-ASF-CR] IMPALA-6424: Avoid loading metadata twice when refreshing unloaded tables

Dimitris Tsirogiannis has uploaded this change for review. ( http://gerrit.cloudera.org:8080/9224


Change subject: IMPALA-6424: Avoid loading metadata twice when refreshing unloaded tables
......................................................................

IMPALA-6424: Avoid loading metadata twice when refreshing unloaded tables

This commit fixes an issue where table metadata are loaded twice if a
REFRESH statement is executed on an unloaded table. With this fix, we
initially check the state of the table (loaded, unloaded) and we only
refresh metadata if the table is in a loaded state. If table is
unloaded, we load the metadata from scratch.

Testing:
Run exhaustive tests

Change-Id: Ie41a734493dcea0e36d6b051966f1d0302907dee
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
1 file changed, 23 insertions(+), 5 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/24/9224/1
-- 
To view, visit http://gerrit.cloudera.org:8080/9224
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie41a734493dcea0e36d6b051966f1d0302907dee
Gerrit-Change-Number: 9224
Gerrit-PatchSet: 1
Gerrit-Owner: Dimitris Tsirogiannis <dt...@cloudera.com>

[Impala-ASF-CR] IMPALA-6424: Avoid loading metadata twice when refreshing unloaded tables

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9224 )

Change subject: IMPALA-6424: Avoid loading metadata twice when refreshing unloaded tables
......................................................................

IMPALA-6424: Avoid loading metadata twice when refreshing unloaded tables

This commit fixes an issue where table metadata are loaded twice if a
REFRESH statement is executed on an unloaded table. With this fix, we
initially check the state of the table (loaded, unloaded) and we only
refresh metadata if the table is in a loaded state. If table is
unloaded, we load the metadata from scratch.

Testing:
Run exhaustive tests

Change-Id: Ie41a734493dcea0e36d6b051966f1d0302907dee
Reviewed-on: http://gerrit.cloudera.org:8080/9224
Reviewed-by: Dimitris Tsirogiannis <dt...@cloudera.com>
Tested-by: Impala Public Jenkins
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
1 file changed, 23 insertions(+), 5 deletions(-)

Approvals:
  Dimitris Tsirogiannis: Looks good to me, approved
  Impala Public Jenkins: Verified

-- 
To view, visit http://gerrit.cloudera.org:8080/9224
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie41a734493dcea0e36d6b051966f1d0302907dee
Gerrit-Change-Number: 9224
Gerrit-PatchSet: 3
Gerrit-Owner: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins

[Impala-ASF-CR] IMPALA-6424: Avoid loading metadata twice when refreshing unloaded tables

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/9224 )

Change subject: IMPALA-6424: Avoid loading metadata twice when refreshing unloaded tables
......................................................................


Patch Set 2:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1959/


-- 
To view, visit http://gerrit.cloudera.org:8080/9224
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie41a734493dcea0e36d6b051966f1d0302907dee
Gerrit-Change-Number: 9224
Gerrit-PatchSet: 2
Gerrit-Owner: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Comment-Date: Tue, 20 Feb 2018 18:30:36 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6424: Avoid loading metadata twice when refreshing unloaded tables

Posted by "Dimitris Tsirogiannis (Code Review)" <ge...@cloudera.org>.
Dimitris Tsirogiannis has posted comments on this change. ( http://gerrit.cloudera.org:8080/9224 )

Change subject: IMPALA-6424: Avoid loading metadata twice when refreshing unloaded tables
......................................................................


Patch Set 2: Code-Review+2

(1 comment)

Rebase, carry Alex's +2

http://gerrit.cloudera.org:8080/#/c/9224/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/9224/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3105
PS1, Line 3105:           tbl = getExistingTable(tblName.getDb(), tblName.getTbl());
> Change seems fine.
I looked at it but it requires changes in the getOrLoad function and that thing is called in so many places that it's not clear if this is going to make things cleaner. Thoughts?



-- 
To view, visit http://gerrit.cloudera.org:8080/9224
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie41a734493dcea0e36d6b051966f1d0302907dee
Gerrit-Change-Number: 9224
Gerrit-PatchSet: 2
Gerrit-Owner: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-Comment-Date: Tue, 20 Feb 2018 18:30:04 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6424: Avoid loading metadata twice when refreshing unloaded tables

Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change. ( http://gerrit.cloudera.org:8080/9224 )

Change subject: IMPALA-6424: Avoid loading metadata twice when refreshing unloaded tables
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9224/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/9224/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3105
PS1, Line 3105:           tbl = getExistingTable(tblName.getDb(), tblName.getTbl());
Change seems fine.

Have you considered changing getExistingTable() to also return whether the table was cached or loaded? Seems potentially cleaner, but more invasive.



-- 
To view, visit http://gerrit.cloudera.org:8080/9224
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie41a734493dcea0e36d6b051966f1d0302907dee
Gerrit-Change-Number: 9224
Gerrit-PatchSet: 1
Gerrit-Owner: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Feb 2018 23:22:16 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6424: Avoid loading metadata twice when refreshing unloaded tables

Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change. ( http://gerrit.cloudera.org:8080/9224 )

Change subject: IMPALA-6424: Avoid loading metadata twice when refreshing unloaded tables
......................................................................


Patch Set 1: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9224/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/9224/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3105
PS1, Line 3105:           tbl = getExistingTable(tblName.getDb(), tblName.getTbl());
> Change seems fine.
Dimitris and I looked into this a little deeper, and concluded that the refactor would be too invasive. Let's go with this change for now.



-- 
To view, visit http://gerrit.cloudera.org:8080/9224
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie41a734493dcea0e36d6b051966f1d0302907dee
Gerrit-Change-Number: 9224
Gerrit-PatchSet: 1
Gerrit-Owner: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Comment-Date: Fri, 16 Feb 2018 22:45:35 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6424: Avoid loading metadata twice when refreshing unloaded tables

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/9224 )

Change subject: IMPALA-6424: Avoid loading metadata twice when refreshing unloaded tables
......................................................................


Patch Set 2: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/9224
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie41a734493dcea0e36d6b051966f1d0302907dee
Gerrit-Change-Number: 9224
Gerrit-PatchSet: 2
Gerrit-Owner: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Comment-Date: Tue, 20 Feb 2018 22:09:55 +0000
Gerrit-HasComments: No