Tianyi Wang has posted comments on this change. ( http://gerrit.cloudera.org:8080/8825 )
Change subject: IMPALA-5990: End-to-end compression of metadata ...................................................................... Patch Set 8: (14 comments) http://gerrit.cloudera.org:8080/#/c/8825/6/be/src/catalog/catalog-server.h File be/src/catalog/catalog-server.h: http://gerrit.cloudera.org:8080/#/c/8825/6/be/src/catalog/catalog-server.h@78 PS6, Line 78: /// The return value is true if the operation succeeds and false otherwise. > topic_data -> item_data Done http://gerrit.cloudera.org:8080/#/c/8825/6/be/src/catalog/catalog-util-test.cc File be/src/catalog/catalog-util-test.cc: http://gerrit.cloudera.org:8080/#/c/8825/6/be/src/catalog/catalog-util-test.cc@40 PS6, Line 40: uint32_t large_string_size = 0x7E000000; // LZ4_MAX_INPUT_SIZE > * reserve() the size - Done. - LZ4 cannot compress 2GB data. I changed it to the exact LZ4_MAX_INPUT_SIZE now. http://gerrit.cloudera.org:8080/#/c/8825/6/be/src/catalog/catalog-util.h File be/src/catalog/catalog-util.h: http://gerrit.cloudera.org:8080/#/c/8825/6/be/src/catalog/catalog-util.h@36 PS6, Line 36: public: > space before private/protected/public Done http://gerrit.cloudera.org:8080/#/c/8825/6/be/src/catalog/catalog-util.h@45 PS6, Line 45: /// will be skipped and the next valid object is returned. > will be skipped and the next valid object is returned. Done http://gerrit.cloudera.org:8080/#/c/8825/6/be/src/catalog/catalog-util.h@46 PS6, Line 46: virtual jobject next(JNIEnv* env) = 0; > Better to add a virtual destructor Done http://gerrit.cloudera.org:8080/#/c/8825/6/be/src/catalog/catalog-util.h@54 PS6, Line 54: > nit: use common abbreviations for JNI stuff, "cl" for "class" and "ctor" fo Done http://gerrit.cloudera.org:8080/#/c/8825/6/be/src/catalog/catalog-util.cc File be/src/catalog/catalog-util.cc: http://gerrit.cloudera.org:8080/#/c/8825/6/be/src/catalog/catalog-util.cc@106 PS6, Line 106: ++pos_; > ++pos Done http://gerrit.cloudera.org:8080/#/c/8825/6/be/src/catalog/catalog-util.cc@118 PS6, Line 118: } > remove newline Done http://gerrit.cloudera.org:8080/#/c/8825/6/be/src/service/fe-support.cc File be/src/service/fe-support.cc: http://gerrit.cloudera.org:8080/#/c/8825/6/be/src/service/fe-support.cc@428 PS6, Line 428: JNIEXPORT void JNICALL > I'm thinking this function should return a bool for success/failure. That w Done http://gerrit.cloudera.org:8080/#/c/8825/6/be/src/service/fe-support.cc@435 PS6, Line 435: if (!JniUtfCharGuard::create(env, key, &key_str).ok()) return; > Comment seems wrong, the FE won't see an exception. Done http://gerrit.cloudera.org:8080/#/c/8825/6/be/src/service/fe-support.cc@455 PS6, Line 455: Java_org_apache_impala_service_FeSupport_NativeLibCacheSetNeedsRefresh(JNIEnv* env, > Same comment about returning bool. Done http://gerrit.cloudera.org:8080/#/c/8825/6/be/src/service/fe-support.cc@468 PS6, Line 468: > Same comment about returning bool. Done http://gerrit.cloudera.org:8080/#/c/8825/6/be/src/util/jni-util.h File be/src/util/jni-util.h: http://gerrit.cloudera.org:8080/#/c/8825/6/be/src/util/jni-util.h@172 PS6, Line 172: public: > single space before public/private Done http://gerrit.cloudera.org:8080/#/c/8825/6/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java: http://gerrit.cloudera.org:8080/#/c/8825/6/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@404 PS6, Line 404: TSerializer serializer; > move to prev line The previous line has 89 characters and " {" doesn't fit. -- To view, visit http://gerrit.cloudera.org:8080/8825 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a8819cad734b3a416eef6c954e55b73cc6023ae Gerrit-Change-Number: 8825 Gerrit-PatchSet: 8 Gerrit-Owner: Tianyi Wang <tw...@cloudera.com> Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com> Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogian...@cloudera.com> Gerrit-Reviewer: Philip Zeyliger <phi...@cloudera.com> Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com> Gerrit-Comment-Date: Thu, 25 Jan 2018 18:52:38 +0000 Gerrit-HasComments: Yes
