Dimitris Tsirogiannis has posted comments on this change. Change subject: IMPALA-3499: Split catalog update ......................................................................
Patch Set 9: (11 comments) http://gerrit.cloudera.org:8080/#/c/3067/9/be/src/service/frontend.h File be/src/service/frontend.h: PS9, Line 178: 2D_ I don't think we need to change the name of this jmethod. http://gerrit.cloudera.org:8080/#/c/3067/9/be/src/service/impala-server.cc File be/src/service/impala-server.cc: PS9, Line 197: MAX_CATALOG_CACHE_UPDATE_SIZE With this new patch, this no longer represents the maximum size of a catalog update but the batch size, correct? Maybe we should rename this variable to CATALOG_CACHE_UPDATE_BATCH_SIZE or something along these lines. PS9, Line 1243: Update is split in pieces with size limit to : // be max_catalog_cache_update_size for multiple updates nit: Maybe we should just say "An update is split into batches of size MAX... each". PS9, Line 1284: push_back(update_req); Isn't this calling the copy constructor of TUpdateCatalogCacheRequest? If so, isn't this going to be a bit expensive? Line 1291: update_reqs.push_back(update_req); > do we still want to do this? I believe we do, otherwise the size of the current update_req may be close to 2GB and be pushed off the cliff by adding the deletions below, right? http://gerrit.cloudera.org:8080/#/c/3067/9/be/src/util/jni-util.h File be/src/util/jni-util.h: PS9, Line 275: _2D Not a big fan of this "2D" but I'll leave it up to you and Henry to decide. PS9, Line 276: args.size(),bytearray_class,NULL nit: spaces after "," http://gerrit.cloudera.org:8080/#/c/3067/9/fe/src/main/java/com/cloudera/impala/service/JniFrontend.java File fe/src/main/java/com/cloudera/impala/service/JniFrontend.java: PS9, Line 160: cur_update You're in Java world :) maybe catalogUpdate? PS9, Line 161: incremental_req incrementalRequest PS9, Line 168: req.getUpdated_objects() == null if (!req.isSetUpdated_objects()) PS9, Line 173: if (req.getRemoved_objects() == null) { if (!req.isSetRemoved_objects()) -- 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: 9 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Huaisi Xu <[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
