Henry Robinson has posted comments on this change. Change subject: IMPALA-3499: Split catalog update ......................................................................
Patch Set 14: (12 comments) http://gerrit.cloudera.org:8080/#/c/3067/14//COMMIT_MSG Commit Message: PS14, Line 9: deserialized serialized byte array, no? PS14, Line 10: Instead of PS14, Line 11: update the update PS14, Line 12: 2GB 500MB? http://gerrit.cloudera.org:8080/#/c/3067/14/be/src/service/frontend.h File be/src/service/frontend.h: PS14, Line 39: contains nit: contain http://gerrit.cloudera.org:8080/#/c/3067/14/be/src/service/impala-server.cc File be/src/service/impala-server.cc: PS14, Line 196: CATALOG_CACHE_UPDATE_BATCH_SIZE_BYTES call this: MAX_CATALOG_UPDATE_BATCH_SIZE_BYTES ? PS14, Line 1247: incrementalRequest C++-style naming http://gerrit.cloudera.org:8080/#/c/3067/14/be/src/util/jni-util.h File be/src/util/jni-util.h: Line 275: jobjectArray jArrayOfByteArray = C++-style naming, please. http://gerrit.cloudera.org:8080/#/c/3067/14/fe/src/main/java/com/cloudera/impala/service/JniFrontend.java File fe/src/main/java/com/cloudera/impala/service/JniFrontend.java: Line 158: TUpdateCatalogCacheRequest merged_update_request = new TUpdateCatalogCacheRequest(); Needs a comment about what is going on in this method. PS14, Line 164: if (!merged_update_request.isSetIs_delta()) { is_delta is a required field. Can't you just OR- the is_delta fields from each update together? Line 167: if (!incrementalRequest.getCatalog_service_id().equals(default_catalog_service_id)) { long line PS14, Line 171: if (!merged_update_request.isSetUpdated_objects()) { all of these fields are required, which means they'll be initialised by the object constructor, right? So you can just addAll() without the conditional. -- To view, visit http://gerrit.cloudera.org:8080/3067 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I176db25124a32944f2396ce8aafbed49cac95928 Gerrit-PatchSet: 14 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Huaisi Xu <[email protected]> Gerrit-Reviewer: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]> Gerrit-Reviewer: Henry Robinson <[email protected]> Gerrit-Reviewer: Huaisi Xu <[email protected]> Gerrit-HasComments: Yes
