Bharath Vissapragada has posted comments on this change.

Change subject: IMPALA-5881: Use native allocation while building catalog 
updates
......................................................................


Patch Set 6:

(2 comments)

Fixed the tests. Figured out that the problem was with thrift doing an initial 
copy using getBytes() which causes problems. Worked around it by splitting the 
test struct into 8 strings (instead of 2). Now I can repro all scenarios in 
unit tests.

http://gerrit.cloudera.org:8080/#/c/7955/1/be/src/catalog/catalog.cc
File be/src/catalog/catalog.cc:

PS1, Line 106: TNativeByteBuffer nbuffer;
> I'm not really sure what error scenario you are worried about. We allocate 
makes sense. Done.


http://gerrit.cloudera.org:8080/#/c/7955/4/fe/src/main/java/org/apache/impala/service/JniCatalog.java
File fe/src/main/java/org/apache/impala/service/JniCatalog.java:

Line 153:       LOG.warn("Detected a potentially large catalog update. 
Serialized size: "
> I'm not opposed to adding warnings, just concerned that users will worry ab
Hmm, my thinking was that we can use this for postmortem  in case of crashes. 
Let me know your thoughts. I'm fine either way.


-- 
To view, visit http://gerrit.cloudera.org:8080/7955
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I383684effa9524734ce3c6c0fb7ed37de0e15782
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <[email protected]>
Gerrit-Reviewer: Alex Behm <[email protected]>
Gerrit-Reviewer: Bharath Vissapragada <[email protected]>
Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]>
Gerrit-Reviewer: Mostafa Mokhtar <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-HasComments: Yes

Reply via email to