Dimitris Tsirogiannis has posted comments on this change.

Change subject: IMPALA-3499: Frontend cannot deserialize catalog-update topic 
more than 2GB.
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/3067/2/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:

Line 1291: break
> Error will print later at line 1339. You mean I should print here as well?
The logic now is confusing and perhaps hides a number of bugs. Previously, 
there was one call to UpdateCatalogCache() in L1337 and the code below that was 
handling the successful or failed invocation of that. Now you're doing a few 
things that don't seem right:
1. A failed call to UpdateCatalogCache() isn't handled immediately. You break, 
perform all the deletions and then check the error message later. It isn't 
obvious at that point which 'status' are you processing; i.e. which call does 
it correspond to. 
2. Do we understand what will happen if one call to UpdateCatalogCache fails 
after a few successful ones? Is the logic in L1339-1349 going to handle this 
correctly or do we leave the catalog cache in some inconsistent state?


Line 1293: update_req.__set_is_delta(true);
> none delta will clear impalad's cache and create a new one.
How do you know this isn't the first update?


-- 
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: 2
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Huaisi Xu <[email protected]>
Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]>
Gerrit-Reviewer: Huaisi Xu <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-HasComments: Yes

Reply via email to