impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alex Behm (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5881: Use native allocation while building catalog updates
Date Thu, 07 Sep 2017 00:27:24 GMT
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 <bharathv@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bharathv@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogiannis@cloudera.com>
Gerrit-Reviewer: Mostafa Mokhtar <mmokhtar@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message