impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tim Armstrong (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-4539: fix bug when scratch batch references I/O buffers
Date Thu, 08 Dec 2016 01:23:51 GMT
Tim Armstrong has posted comments on this change.

Change subject: IMPALA-4539: fix bug when scratch batch references I/O buffers
......................................................................


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/5406/2/be/src/exec/hdfs-parquet-scanner.cc
File be/src/exec/hdfs-parquet-scanner.cc:

PS2, Line 245: 
> What do you mean by wrong point ?
I just meant that there were certain states where it could trigger the bug - specifically
if GetNextInternal() returns before the whole scratch batch is empty.

That phrase is removed anyway.


http://gerrit.cloudera.org:8080/#/c/5406/3/be/src/exec/hdfs-parquet-scanner.cc
File be/src/exec/hdfs-parquet-scanner.cc:

PS3, Line 601: (dst_batch->AtCapacity() || context_->num_completed_io_buffers() >
0)
> Do we still need these conditions ? I suppose FlushRowGroupResources() will
I don't believe they're necessary for correctness, but I'd have to think it through more.
I think it's fine to attach the resources at any point once we've copied the tuples. It can
save us iterating over all the streams to check them, but it's unclear how important that
is.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic27e7251e0f633cb694b506f6eb62beed6e66ad9
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Marcel Kornacker <marcel@cloudera.com>
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Michael Ho <kwho@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message