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 <zams...@cloudera.com> Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com> Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogian...@cloudera.com> Gerrit-Reviewer: Zach Amsden <zams...@cloudera.com> Gerrit-HasComments: Yes
