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-5307: Part 4: copy out uncompressed text and seq
Date Tue, 31 Oct 2017 21:48:42 GMT
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8172 )

Change subject: IMPALA-5307: Part 4: copy out uncompressed text and seq
......................................................................


Patch Set 8:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/8172/8/be/src/exec/hdfs-scan-node.cc
File be/src/exec/hdfs-scan-node.cc:

http://gerrit.cloudera.org:8080/#/c/8172/8/be/src/exec/hdfs-scan-node.cc@238
PS8, Line 238: 
> nit: a lot of unnecessary blank lines. maybe condense a bit so more code fi
Done


http://gerrit.cloudera.org:8080/#/c/8172/8/be/src/exec/hdfs-sequence-scanner.cc
File be/src/exec/hdfs-sequence-scanner.cc:

http://gerrit.cloudera.org:8080/#/c/8172/8/be/src/exec/hdfs-sequence-scanner.cc@329
PS8, Line 329:       add_row = WriteCompleteTuple(row_batch->tuple_data_pool(), field_locations_.data(),
> I missed updating this additional code path, we also need to copy out strin
Done. The below test was previously failing but now succeeds. We should get better coverage
of some of these things once we switch the I/O mgr to the buffer pool and have ASAN poisoning
enabled for recycled buffers.

  ./buildall.sh -asan -skiptests -noclean -ninja -notests && start-impala-cluster.py
--impalad_args=--disable_mem_pools=true  && impala-py.test -n1 --verbose tests/query_test/test_scanners.py
tests/query_test/test_aggregation.py --workload_exploration_strategy=functional-query:exhaustive
-k seq


http://gerrit.cloudera.org:8080/#/c/8172/8/be/src/exec/hdfs-text-scanner.cc
File be/src/exec/hdfs-text-scanner.cc:

http://gerrit.cloudera.org:8080/#/c/8172/8/be/src/exec/hdfs-text-scanner.cc@704
PS8, Line 704:   bool split_delimiter_possible = context_->partition_descriptor()->line_delim()
== '\n'
> I think there's a latent bug here where we're touching byte_buffer_ *after*
I filed IMPALA-6137. I think there are pre-existing bugs here so won't tackle them in this
patch (this patch may have to wait for those though since it increase the odds of hitting
them).



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I304fd002b61bfedf41c8b1405cd7eb7b492bb941
Gerrit-Change-Number: 8172
Gerrit-PatchSet: 8
Gerrit-Owner: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Michael Ho <kwho@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-Comment-Date: Tue, 31 Oct 2017 21:48:42 +0000
Gerrit-HasComments: Yes

Mime
  • Unnamed multipart/alternative (inline, 8-Bit, 0 bytes)
View raw message