Alex Behm has posted comments on this change. Change subject: IMPALA-5881: Use native allocation while building catalog updates ......................................................................
Patch Set 4: (2 comments) Responding to comments, will look at code next. 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 think the problem is when nbuffer_ptr is non zero but DeSerialize call fa I'm not really sure what error scenario you are worried about. We allocate the pointer with malloc(). If we return from DeserializeThriftMsg() with a non-OK status, I don't think we can definitely conclude that we got a bad pointer. I'm thinking that DeserializeThriftMsg() could fail due to a bug in thrift (or other recoverable reasons?), and that the next time around deserialization will just work with a different payload. 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: if (serializedByteSize >= SERIALIZED_TOPIC_SIZE_WARNING_LIMIT_BYTES) { > I included this for supportability purposes. With this we log the untracked I'm not opposed to adding warnings, just concerned that users will worry about them and not know how to fix them. Let me think about this a little more. I'm also wondering what we could actually do with this information - the next question would probably be a breakdown of why the topic is so big. -- 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: 4 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
