impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bharath Vissapragada (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-5881: Use native allocation while building catalog updates
Date Tue, 12 Sep 2017 22:01:42 GMT
Bharath Vissapragada has posted comments on this change.

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

Patch Set 18:

File be/src/catalog/

PS18, Line 117: DCHECK_LE(len, numeric_limits<uint32_t>::max());
> This DCHECK needs to be run against nbuffer.buffer_len directly.
I'm not totally sure about this part, but looks like the static_cast() can silently fail.
Either way I think its good to make this check on buffer_len
File fe/src/main/java/org/apache/impala/service/

PS18, Line 155: Catalog
> nit: catalog
File fe/src/test/java/org/apache/impala/thrift/

Line 61:     nbaos.write(b, offset, len);
> try catch and assert if we catch an exception or use the exceptionCaught pa
As discussed, not really needed since we expect the write to throw if an error occurs and
then it is caught by the test.

Line 67:   // Makes sure that the memory is freed by the end of nboas.write().
> Expects a write() to fail and checks that the memory is freed after the uns

Line 71:       nbaos.write(b, offset, len);
> Add an assert that fails if the write succeeded.

Line 95:   public void nbaosTest() {
> NbaosTest

Line 99:     testAllocator  = new NativeTestAllocator();
> move into L96

Line 100:     // Check that initial allocation failure in NBAOS c'tor
> remove "that"

To view, visit
To unsubscribe, visit

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

View raw message