impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bharath Vissapragada (Code Review)" <ger...@cloudera.org>
Subject [Impala-CR](cdh5-trunk) IMPALA-3499: Split catalog update
Date Fri, 10 Jun 2016 11:27:48 GMT
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 <hxu@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bharathv@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogiannis@cloudera.com>
Gerrit-Reviewer: Henry Robinson <henry@cloudera.com>
Gerrit-Reviewer: Huaisi Xu <hxu@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message