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 2017/01/27 23:21:22 UTC
[Impala-ASF-CR] IMPALA-2518: DROP DATABASE CASCADE doesn't remove cache directives of tables
Dimitris Tsirogiannis has uploaded a new change for review.
http://gerrit.cloudera.org:8080/5815
Change subject: IMPALA-2518: DROP DATABASE CASCADE doesn't remove cache directives of tables
......................................................................
IMPALA-2518: DROP DATABASE CASCADE doesn't remove cache directives of
tables
This commit fixes an issue where the DROP DATABASE CASCADE statement
will not remove the cache directives of the underlying tables and their
partitions.
Change-Id: I83ef5a33e06728c2b3f833a0309d9da64dce7b88
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/HdfsCachingUtil.java
M tests/query_test/test_hdfs_caching.py
3 files changed, 66 insertions(+), 34 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/15/5815/1
--
To view, visit http://gerrit.cloudera.org:8080/5815
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I83ef5a33e06728c2b3f833a0309d9da64dce7b88
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis <dt...@cloudera.com>
[Impala-ASF-CR] IMPALA-2518: DROP DATABASE CASCADE doesn't remove cache directives of tables
Posted by "Dimitris Tsirogiannis (Code Review)" <ge...@cloudera.org>.
Dimitris Tsirogiannis has posted comments on this change.
Change subject: IMPALA-2518: DROP DATABASE CASCADE doesn't remove cache directives of tables
......................................................................
Patch Set 1:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/5815/1//COMMIT_MSG
Commit Message:
Line 7: IMPALA-2518: DROP DATABASE CASCADE doesn't remove cache directives of
> describe the fix not the bug:
Done
http://gerrit.cloudera.org:8080/#/c/5815/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:
Line 1279: uncacheTable(removedDb.getTable(tableName));
> It's a little tricky to figure out, but I think we may have to lock these t
This is quite tricky. Keep in mind that we perform these operations on the deleted table objects. So the question is whether we correctly handle dropping a table and at the same time altering it. The answer I believe is no. The former operation uses the metastoreDdlLock_ while the latter is protected by the table lock. In theory, and assuming we had proper hierarchical locking, no concurrent access should be allowed on these tables as it would require, in the least, a read lock on the db to be dropped. We could acquire the lock on the dropped tables but, I'd rather not do it in this patch.
http://gerrit.cloudera.org:8080/#/c/5815/1/fe/src/main/java/org/apache/impala/util/HdfsCachingUtil.java
File fe/src/main/java/org/apache/impala/util/HdfsCachingUtil.java:
Line 163: org.apache.hadoop.hive.metastore.api.Partition part) throws ImpalaException {
> tab
Done
http://gerrit.cloudera.org:8080/#/c/5815/1/tests/query_test/test_hdfs_caching.py
File tests/query_test/test_hdfs_caching.py:
Line 212: """The DROP DATABASE CASCADE should properly drop all impacted cache directives
> Remove "The"
Done
Line 213: IMPALA-2518"""
> use IMPALA-2518 as a prefix to comment (like we typically do)
Done
Line 215: # Creates `cachedb` database with some cached tables and partitions
> Populates the 'cachedb' database...
Done
--
To view, visit http://gerrit.cloudera.org:8080/5815
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I83ef5a33e06728c2b3f833a0309d9da64dce7b88
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-2518: DROP DATABASE CASCADE removes cache directives of tables
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged.
Change subject: IMPALA-2518: DROP DATABASE CASCADE removes cache directives of tables
......................................................................
IMPALA-2518: DROP DATABASE CASCADE removes cache directives of tables
This commit fixes an issue where the DROP DATABASE CASCADE statement
will not remove the cache directives of the underlying tables and their
partitions.
Change-Id: I83ef5a33e06728c2b3f833a0309d9da64dce7b88
Reviewed-on: http://gerrit.cloudera.org:8080/5815
Reviewed-by: Dimitris Tsirogiannis <dt...@cloudera.com>
Tested-by: Impala Public Jenkins
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/HdfsCachingUtil.java
M tests/query_test/test_hdfs_caching.py
3 files changed, 66 insertions(+), 34 deletions(-)
Approvals:
Impala Public Jenkins: Verified
Dimitris Tsirogiannis: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/5815
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I83ef5a33e06728c2b3f833a0309d9da64dce7b88
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
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-2518: DROP DATABASE CASCADE removes cache directives of tables
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change.
Change subject: IMPALA-2518: DROP DATABASE CASCADE removes cache directives of tables
......................................................................
Patch Set 2: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/5815/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:
Line 1279: uncacheTable(removedDb.getTable(tableName));
> This is quite tricky. Keep in mind that we perform these operations on the
Agreed. Might be worth adding a brief comment in one of the existing JIRAs just to keep a trail that this CC issue was discussed in this CR.
--
To view, visit http://gerrit.cloudera.org:8080/5815
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I83ef5a33e06728c2b3f833a0309d9da64dce7b88
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-2518: DROP DATABASE CASCADE removes cache directives of tables
Posted by "Dimitris Tsirogiannis (Code Review)" <ge...@cloudera.org>.
Dimitris Tsirogiannis has uploaded a new patch set (#2).
Change subject: IMPALA-2518: DROP DATABASE CASCADE removes cache directives of tables
......................................................................
IMPALA-2518: DROP DATABASE CASCADE removes cache directives of tables
This commit fixes an issue where the DROP DATABASE CASCADE statement
will not remove the cache directives of the underlying tables and their
partitions.
Change-Id: I83ef5a33e06728c2b3f833a0309d9da64dce7b88
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/HdfsCachingUtil.java
M tests/query_test/test_hdfs_caching.py
3 files changed, 66 insertions(+), 34 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/15/5815/2
--
To view, visit http://gerrit.cloudera.org:8080/5815
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I83ef5a33e06728c2b3f833a0309d9da64dce7b88
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
[Impala-ASF-CR] IMPALA-2518: DROP DATABASE CASCADE removes cache directives of tables
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: IMPALA-2518: DROP DATABASE CASCADE removes cache directives of tables
......................................................................
Patch Set 4:
Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/231/
--
To view, visit http://gerrit.cloudera.org:8080/5815
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I83ef5a33e06728c2b3f833a0309d9da64dce7b88
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
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-HasComments: No
[Impala-ASF-CR] IMPALA-2518: DROP DATABASE CASCADE removes cache directives of tables
Posted by "Dimitris Tsirogiannis (Code Review)" <ge...@cloudera.org>.
Dimitris Tsirogiannis has posted comments on this change.
Change subject: IMPALA-2518: DROP DATABASE CASCADE removes cache directives of tables
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/5815/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:
Line 1279: uncacheTable(removedDb.getTable(tableName));
> Agreed. Might be worth adding a brief comment in one of the existing JIRAs
Done
--
To view, visit http://gerrit.cloudera.org:8080/5815
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I83ef5a33e06728c2b3f833a0309d9da64dce7b88
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-2518: DROP DATABASE CASCADE removes cache directives of tables
Posted by "Dimitris Tsirogiannis (Code Review)" <ge...@cloudera.org>.
Dimitris Tsirogiannis has posted comments on this change.
Change subject: IMPALA-2518: DROP DATABASE CASCADE removes cache directives of tables
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/5815
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I83ef5a33e06728c2b3f833a0309d9da64dce7b88
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-2518: DROP DATABASE CASCADE doesn't remove cache directives of tables
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change.
Change subject: IMPALA-2518: DROP DATABASE CASCADE doesn't remove cache directives of tables
......................................................................
Patch Set 1:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/5815/1//COMMIT_MSG
Commit Message:
Line 7: IMPALA-2518: DROP DATABASE CASCADE doesn't remove cache directives of
describe the fix not the bug:
DROP DATABASE CASCADE removes caching directives
http://gerrit.cloudera.org:8080/#/c/5815/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:
Line 1279: uncacheTable(removedDb.getTable(tableName));
It's a little tricky to figure out, but I think we may have to lock these tables. For example, there might be a concurrent request for caching a table/partition and there could be a concurrent deepCopy() of the msTbl which may not be safe with removing entries from the msTbl properties. Might be worth looking into at least.
http://gerrit.cloudera.org:8080/#/c/5815/1/fe/src/main/java/org/apache/impala/util/HdfsCachingUtil.java
File fe/src/main/java/org/apache/impala/util/HdfsCachingUtil.java:
Line 163: org.apache.hadoop.hive.metastore.api.Partition part) throws ImpalaException {
tab
http://gerrit.cloudera.org:8080/#/c/5815/1/tests/query_test/test_hdfs_caching.py
File tests/query_test/test_hdfs_caching.py:
Line 212: """The DROP DATABASE CASCADE should properly drop all impacted cache directives
Remove "The"
Line 213: IMPALA-2518"""
use IMPALA-2518 as a prefix to comment (like we typically do)
Line 215: # Creates `cachedb` database with some cached tables and partitions
Populates the 'cachedb' database...
--
To view, visit http://gerrit.cloudera.org:8080/5815
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I83ef5a33e06728c2b3f833a0309d9da64dce7b88
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-2518: DROP DATABASE CASCADE removes cache directives of tables
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: IMPALA-2518: DROP DATABASE CASCADE removes cache directives of tables
......................................................................
Patch Set 4: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/5815
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I83ef5a33e06728c2b3f833a0309d9da64dce7b88
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
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-HasComments: No