You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Alex Behm (Code Review)" <ge...@cloudera.org> on 2017/02/28 05:59:08 UTC
[Impala-ASF-CR] IMPALA-4998: Fix missing table lock acquisition.
Alex Behm has uploaded a new change for review.
http://gerrit.cloudera.org:8080/6177
Change subject: IMPALA-4998: Fix missing table lock acquisition.
......................................................................
IMPALA-4998: Fix missing table lock acquisition.
Testing: Before this patch test_views_compatibility.py
failed locally reliably. After this patch the test
passes locally.
Change-Id: I0e0270daf59fce95f1a1520fc5aaf91d3a7b99fe
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
1 file changed, 6 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/77/6177/1
--
To view, visit http://gerrit.cloudera.org:8080/6177
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0e0270daf59fce95f1a1520fc5aaf91d3a7b99fe
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <al...@cloudera.com>
[Impala-ASF-CR] IMPALA-4998: Fix missing table lock acquisition.
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change.
Change subject: IMPALA-4998: Fix missing table lock acquisition.
......................................................................
Patch Set 5:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/6177/5/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:
PS5, Line 934: tbl instanceof IncompleteTable
> Not related to this change but I believe we load the metadata twice if we c
That's correct. It's a little tricky to fix because the first load is initiated during analysis of the REFRESH stmt, and then once we get into the CatalogOpExecutor the table should be loaded, so we would not load it again here in getOrLoadTable() (unless it got invalidated in the meantime). I think we need to re-think the FE analysis of REFRESH to fix it.
This relates to some of the work Amos is doing.
I filed IMPALA-5012.
--
To view, visit http://gerrit.cloudera.org:8080/6177
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0e0270daf59fce95f1a1520fc5aaf91d3a7b99fe
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4998: Fix missing table lock acquisition.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: IMPALA-4998: Fix missing table lock acquisition.
......................................................................
Patch Set 6:
Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/324/
--
To view, visit http://gerrit.cloudera.org:8080/6177
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0e0270daf59fce95f1a1520fc5aaf91d3a7b99fe
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4998: Fix missing table lock acquisition.
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has uploaded a new patch set (#2).
Change subject: IMPALA-4998: Fix missing table lock acquisition.
......................................................................
IMPALA-4998: Fix missing table lock acquisition.
The following commit broke test_views_compatibility.py
which only runs in exhaustive mode, so the issue was
not caught by pre-commit testing:
a71636847fe742a9d0eb770516aff34ff16bbca1
Testing: Before this patch test_views_compatibility.py
failed locally reliably. After this patch the test
passes locally.
Change-Id: I0e0270daf59fce95f1a1520fc5aaf91d3a7b99fe
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
1 file changed, 12 insertions(+), 30 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/77/6177/2
--
To view, visit http://gerrit.cloudera.org:8080/6177
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0e0270daf59fce95f1a1520fc5aaf91d3a7b99fe
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
[Impala-ASF-CR] IMPALA-4998: Fix missing table lock acquisition.
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has uploaded a new patch set (#3).
Change subject: IMPALA-4998: Fix missing table lock acquisition.
......................................................................
IMPALA-4998: Fix missing table lock acquisition.
The following commit broke test_views_compatibility.py
which only runs in exhaustive mode, so the issue was
not caught by pre-commit testing:
a71636847fe742a9d0eb770516aff34ff16bbca1
Testing: Before this patch test_views_compatibility.py
failed locally reliably. After this patch the test
passes locally.
Change-Id: I0e0270daf59fce95f1a1520fc5aaf91d3a7b99fe
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
1 file changed, 16 insertions(+), 38 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/77/6177/3
--
To view, visit http://gerrit.cloudera.org:8080/6177
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0e0270daf59fce95f1a1520fc5aaf91d3a7b99fe
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
[Impala-ASF-CR] IMPALA-4998: Fix missing table lock acquisition.
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has uploaded a new patch set (#5).
Change subject: IMPALA-4998: Fix missing table lock acquisition.
......................................................................
IMPALA-4998: Fix missing table lock acquisition.
The following commit broke test_views_compatibility.py
which only runs in exhaustive mode, so the issue was
not caught by pre-commit testing:
a71636847fe742a9d0eb770516aff34ff16bbca1
Testing: Before this patch test_views_compatibility.py
failed locally reliably. After this patch the test
passes locally.
Change-Id: I0e0270daf59fce95f1a1520fc5aaf91d3a7b99fe
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/TableLoadingMgr.java
2 files changed, 23 insertions(+), 49 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/77/6177/5
--
To view, visit http://gerrit.cloudera.org:8080/6177
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0e0270daf59fce95f1a1520fc5aaf91d3a7b99fe
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
[Impala-ASF-CR] IMPALA-4998: Fix missing table lock acquisition.
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change.
Change subject: IMPALA-4998: Fix missing table lock acquisition.
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/6177/3/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:
Line 768: if (tbl == null || tbl.isLoaded()) return tbl;
> I'm wondering if this can potentially return an IncompleteTable and that ev
Sorry I mean the following method sets a cause, that can cause isLoaded() to return True.
public static IncompleteTable createFailedMetadataLoadTable(Db db, String name,
ImpalaException e) {
return new IncompleteTable(db, name, e);
}
--
To view, visit http://gerrit.cloudera.org:8080/6177
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0e0270daf59fce95f1a1520fc5aaf91d3a7b99fe
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4998: Fix missing table lock acquisition.
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change.
Change subject: IMPALA-4998: Fix missing table lock acquisition.
......................................................................
Patch Set 5:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/6177/5/fe/src/main/java/org/apache/impala/catalog/TableLoadingMgr.java
File fe/src/main/java/org/apache/impala/catalog/TableLoadingMgr.java:
Line 318: if (tbl == null || tbl instanceof IncompleteTable || !tbl.isLoaded()) return;
Doesn't this mean that we can never "refresh" an IncompleteTable? Without this patch, we'd submit a loadAsync() request if we encounter an IncompleteTable and with this patch we just return. Isn't it a behavioral regression?
--
To view, visit http://gerrit.cloudera.org:8080/6177
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0e0270daf59fce95f1a1520fc5aaf91d3a7b99fe
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4998: Fix missing table lock acquisition.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: IMPALA-4998: Fix missing table lock acquisition.
......................................................................
Patch Set 6: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/6177
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0e0270daf59fce95f1a1520fc5aaf91d3a7b99fe
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4998: Fix missing table lock acquisition.
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has uploaded a new patch set (#4).
Change subject: IMPALA-4998: Fix missing table lock acquisition.
......................................................................
IMPALA-4998: Fix missing table lock acquisition.
The following commit broke test_views_compatibility.py
which only runs in exhaustive mode, so the issue was
not caught by pre-commit testing:
a71636847fe742a9d0eb770516aff34ff16bbca1
Testing: Before this patch test_views_compatibility.py
failed locally reliably. After this patch the test
passes locally.
Change-Id: I0e0270daf59fce95f1a1520fc5aaf91d3a7b99fe
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/TableLoadingMgr.java
2 files changed, 24 insertions(+), 41 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/77/6177/4
--
To view, visit http://gerrit.cloudera.org:8080/6177
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0e0270daf59fce95f1a1520fc5aaf91d3a7b99fe
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
[Impala-ASF-CR] IMPALA-4998: Fix missing table lock acquisition.
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change.
Change subject: IMPALA-4998: Fix missing table lock acquisition.
......................................................................
Patch Set 1:
(2 comments)
I'll run exhaustive on this.
http://gerrit.cloudera.org:8080/#/c/6177/1//COMMIT_MSG
Commit Message:
Line 11: passes locally.
> Curious how GVO didn't catch this. Is the failure intermittent or something
Now explained in commit msg.
http://gerrit.cloudera.org:8080/#/c/6177/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:
Line 957: return result.toTCatalogObject();
> Update this?
Good catch. The code here doesn't seem very useful. I modified it to make more sense, and then the locking issue goes away naturally.
--
To view, visit http://gerrit.cloudera.org:8080/6177
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0e0270daf59fce95f1a1520fc5aaf91d3a7b99fe
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4998: Fix missing table lock acquisition.
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change.
Change subject: IMPALA-4998: Fix missing table lock acquisition.
......................................................................
Patch Set 5:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/6177/5/fe/src/main/java/org/apache/impala/catalog/TableLoadingMgr.java
File fe/src/main/java/org/apache/impala/catalog/TableLoadingMgr.java:
Line 318: if (tbl == null || tbl instanceof IncompleteTable || !tbl.isLoaded()) return;
> Doesn't this mean that we can never "refresh" an IncompleteTable? Without t
We never execute a REFRESH command asynchronously.
This particular code is only called for HDFS caching. When caching a table or partition, we asynchronously wait for the HDFS caching request to fully complete, and once that is done, we automatically issue a REFRESH from this code path to pick up the new cached blocks. I'm not sure this flow even makes sense, but it doesn't make sense to me to load an IncompleteTable here or 'fix' an IncompleteTable that failed to load for whatever reason. That seems beyond the scope of what we are trying to do here and could lead to confusion.
Take a look at asyncRefreshThread_ in TableLoadingMgr.
--
To view, visit http://gerrit.cloudera.org:8080/6177
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0e0270daf59fce95f1a1520fc5aaf91d3a7b99fe
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4998: Fix missing table lock acquisition.
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change.
Change subject: IMPALA-4998: Fix missing table lock acquisition.
......................................................................
Patch Set 5:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/6177/5/fe/src/main/java/org/apache/impala/catalog/TableLoadingMgr.java
File fe/src/main/java/org/apache/impala/catalog/TableLoadingMgr.java:
Line 318: if (tbl == null || tbl instanceof IncompleteTable || !tbl.isLoaded()) return;
> Oops, I thought this was the actual code path for triggering refresh. Thank
Yep. This code is weird and a little misleading :/
--
To view, visit http://gerrit.cloudera.org:8080/6177
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0e0270daf59fce95f1a1520fc5aaf91d3a7b99fe
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4998: Fix missing table lock acquisition.
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change.
Change subject: IMPALA-4998: Fix missing table lock acquisition.
......................................................................
Patch Set 5: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/6177/5/fe/src/main/java/org/apache/impala/catalog/TableLoadingMgr.java
File fe/src/main/java/org/apache/impala/catalog/TableLoadingMgr.java:
Line 318: if (tbl == null || tbl instanceof IncompleteTable || !tbl.isLoaded()) return;
> We never execute a REFRESH command asynchronously.
Oops, I thought this was the actual code path for triggering refresh. Thanks for correcting me. I looked at the CatalogOpEx.getExistingTable() and that answered by question.
--
To view, visit http://gerrit.cloudera.org:8080/6177
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0e0270daf59fce95f1a1520fc5aaf91d3a7b99fe
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4998: Fix missing table lock acquisition.
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change.
Change subject: IMPALA-4998: Fix missing table lock acquisition.
......................................................................
Patch Set 6: Code-Review+2
Carry Dimitris' +2
--
To view, visit http://gerrit.cloudera.org:8080/6177
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0e0270daf59fce95f1a1520fc5aaf91d3a7b99fe
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4998: Fix missing table lock acquisition.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged.
Change subject: IMPALA-4998: Fix missing table lock acquisition.
......................................................................
IMPALA-4998: Fix missing table lock acquisition.
The following commit broke test_views_compatibility.py
which only runs in exhaustive mode, so the issue was
not caught by pre-commit testing:
a71636847fe742a9d0eb770516aff34ff16bbca1
Testing: Before this patch test_views_compatibility.py
failed locally reliably. After this patch the test
passes locally.
Change-Id: I0e0270daf59fce95f1a1520fc5aaf91d3a7b99fe
Reviewed-on: http://gerrit.cloudera.org:8080/6177
Reviewed-by: Alex Behm <al...@cloudera.com>
Tested-by: Impala Public Jenkins
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/TableLoadingMgr.java
2 files changed, 23 insertions(+), 49 deletions(-)
Approvals:
Impala Public Jenkins: Verified
Alex Behm: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/6177
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I0e0270daf59fce95f1a1520fc5aaf91d3a7b99fe
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-4998: Fix missing table lock acquisition.
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change.
Change subject: IMPALA-4998: Fix missing table lock acquisition.
......................................................................
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/6177/3/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:
Line 768: if (tbl == null || tbl.isLoaded()) return tbl;
I'm wondering if this can potentially return an IncompleteTable and that eventually hits the preconditions check in reloadTable(). Reason being, IncompleteTable.isLoaded() needn't always return false.
> public boolean isLoaded() { return cause_ != null; }
Especially the static method that we use to create IncompleteTables sets the cause_ as null.
public static IncompleteTable createUninitializedTable(Db db, String name) {
return new IncompleteTable(db, name, null);
}
Please correct me if I'm wrong.
PS3, Line 926: which not be
rephrase?
--
To view, visit http://gerrit.cloudera.org:8080/6177
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0e0270daf59fce95f1a1520fc5aaf91d3a7b99fe
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4998: Fix missing table lock acquisition.
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change.
Change subject: IMPALA-4998: Fix missing table lock acquisition.
......................................................................
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/6177/3/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:
Line 768: if (tbl == null || tbl.isLoaded()) return tbl;
> Sorry I mean the following method sets a cause, that can cause isLoaded() t
You are right. I did some more cleanup to fix this issue.
PS3, Line 926: which not be
> rephrase?
Done
--
To view, visit http://gerrit.cloudera.org:8080/6177
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0e0270daf59fce95f1a1520fc5aaf91d3a7b99fe
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4998: Fix missing table lock acquisition.
Posted by "Dimitris Tsirogiannis (Code Review)" <ge...@cloudera.org>.
Dimitris Tsirogiannis has posted comments on this change.
Change subject: IMPALA-4998: Fix missing table lock acquisition.
......................................................................
Patch Set 5: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/6177/5/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:
PS5, Line 934: tbl instanceof IncompleteTable
Not related to this change but I believe we load the metadata twice if we call refresh on an incomplete table, once during the getOrLoadTable() call and once here. If you agree, let's file a jira about it. Let me know..
--
To view, visit http://gerrit.cloudera.org:8080/6177
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0e0270daf59fce95f1a1520fc5aaf91d3a7b99fe
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4998: Fix missing table lock acquisition.
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change.
Change subject: IMPALA-4998: Fix missing table lock acquisition.
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/6177/1//COMMIT_MSG
Commit Message:
Line 11: passes locally.
Curious how GVO didn't catch this. Is the failure intermittent or something?
http://gerrit.cloudera.org:8080/#/c/6177/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:
Line 957: return result.toTCatalogObject();
Update this?
--
To view, visit http://gerrit.cloudera.org:8080/6177
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0e0270daf59fce95f1a1520fc5aaf91d3a7b99fe
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-HasComments: Yes