impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Zach Amsden (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-4762: RECOVER PARTITIONS should batch partition updates
Date Tue, 07 Mar 2017 18:35:07 GMT
Zach Amsden has posted comments on this change.

Change subject: IMPALA-4762: RECOVER PARTITIONS should batch partition updates
......................................................................


Patch Set 2:

(2 comments)

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

PS2, Line 2661: updateLastDdlTime(msTbl, msClient);
> Do we need to call this for every batch? I believe it is sufficient to call
If we get some kind of exception during a later round of processing, it is better to have
updated the time, and this could reasonably happen with a timeout or network error.


http://gerrit.cloudera.org:8080/#/c/6275/2/tests/metadata/test_recover_partitions.py
File tests/metadata/test_recover_partitions.py:

PS2, Line 190: for i in xrange(1, 700):
             :         PART_DIR = "part%d\t" % i
             :         assert not self.has_value(PART_DIR, result.data)
> This check may be an overkill since no partitions were added to this table.
It does return one row, the 'Total', which is confusing as it isn't clear this is intended
to be machine readable.  I'd rather leave the check as is since it is more obviously correct
(and also fast, this is a purely local operation).


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7f9334051b11ba8fa16159b7ca67ddc7f2392733
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden <zamsden@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogiannis@cloudera.com>
Gerrit-Reviewer: Zach Amsden <zamsden@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message