Marcell Szabo has posted comments on this change.

Change subject: IMPALA-2518: DROP DATABASE CASCADE does not remove cache 
directives of tables
......................................................................


Patch Set 3:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/1966/3/fe/src/main/java/com/cloudera/impala/service/CatalogOpExecutor.java
File fe/src/main/java/com/cloudera/impala/service/CatalogOpExecutor.java:

Line 1162: it expects
> What happens if this expectation is violated, perhaps by someone pulling th
Very good comment. It would be nice to review the whole metadata solution and 
implement some kind of journaling and 2/3 phase commits. The current system 
doesn't seem to have such guarantees at any point.
Regarding this specific case: the user will see that the drop table query 
failed and will need to run it again. The rerun will skip the already deleted 
cache directives with a warning.


http://gerrit.cloudera.org:8080/#/c/1966/3/fe/src/main/java/com/cloudera/impala/util/HdfsCachingUtil.java
File fe/src/main/java/com/cloudera/impala/util/HdfsCachingUtil.java:

Line 127:    * If the table has partitions, uncachePartition() needs to be 
called.
> Are the callers already doing this correctly?
With this patch, yes, all calls behave as documented here.


http://gerrit.cloudera.org:8080/#/c/1966/3/tests/query_test/test_hdfs_caching.py
File tests/query_test/test_hdfs_caching.py:

Line 135:   def test_caching_ddl_drop_database(self, vector):
> This could use a docstring.
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I58270e1be49e71a08e12021e7dddab01969d1810
Gerrit-PatchSet: 3
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Marcell Szabo <[email protected]>
Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]>
Gerrit-Reviewer: Jim Apple <[email protected]>
Gerrit-Reviewer: Juan Yu <[email protected]>
Gerrit-Reviewer: Marcell Szabo <[email protected]>
Gerrit-HasComments: Yes

Reply via email to