Bharath Vissapragada has posted comments on this change. Change subject: IMPALA-3499: Split catalog update ......................................................................
Patch Set 16: (5 comments) http://gerrit.cloudera.org:8080/#/c/3067/16/be/src/service/frontend.h File be/src/service/frontend.h: PS16, Line 38: of "of" twice. http://gerrit.cloudera.org:8080/#/c/3067/16/be/src/service/impala-server.cc File be/src/service/impala-server.cc: PS16, Line 1243: each for multiple updates. IMPALA-3499 Remove? Line 1263: incremental_request->__set_catalog_service_id( For supportability purposes, could you log something about the catalog_object and its size here? We have always found it difficult to figure out what specific object was causing the huge catalog updates, May be we can log something if size(catalog_object) > 100mb or something? Line 1278: if (current_topic_update_size_bytes + len > MAX_CATALOG_UPDATE_BATCH_SIZE_BYTES) { I still think its cleaner to move this block after L1254. (even though the frontend can handle setting catalog_service_id to a different incremental_request). PS16, Line 1279: TUpdateCatalogCacheRequest Based on my understanding of Java thrift libraries, calling the default constructor doesn't set default values for the required fields. if you try to serialize/deserialize the objects without setting the required fields, it fails with an error "required field <field> not set". I'm surprised you didn't hit that. May be the behavior changed. If you are confident that this works, please ignore, else try calling the constructor with arguments/set this explicitly. -- 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: 16 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
