You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Zoltan Ivanfi (Code Review)" <ge...@cloudera.org> on 2016/10/25 15:14:04 UTC

[Impala-ASF-CR] Remove seemingly incorrect DCHECK-s. 

Zoltan Ivanfi has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/4835

Change subject: Remove seemingly incorrect DCHECK-s.
......................................................................

Remove seemingly incorrect DCHECK-s.

The first conditional DCHECK means that if a page's size is 0 then it's
compressed size is also 0. This, however, seems to be a false
assumption, as the compressed output may include metadata, such as
length or checksum.

The GZIP compressor, for example, states that an input of 0 bytes
requires 23 bytes when compressed. The Snappy compressor also mentions
storing length information in the compressed output. The compressed size
estimation in the LZ4 compressor also contains a constant part.

The "Last page might be empty" comment and the second DCHECK also seems
to be based on a false assumption. If a value doesn't fit on the current
page, AppendRow creates a possible bigger new page and tries writing the
data in the new page instead. This means that if the data is bigger than
the page size, then the current page is finalized and a new page is
added, even if the original page was empty. In other words, empty pages
can occur in the middle of the pages_ array as well, not only at the end
of it.

Change-Id: I52e1b1354e9ea056b49331e75e53759952a81b76
---
M be/src/exec/hdfs-parquet-table-writer.cc
1 file changed, 1 insertion(+), 3 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/35/4835/1
-- 
To view, visit http://gerrit.cloudera.org:8080/4835
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I52e1b1354e9ea056b49331e75e53759952a81b76
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zoltan Ivanfi <zi...@cloudera.com>

[Impala-ASF-CR] Remove seemingly incorrect DCHECK-s.

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.

Change subject: Remove seemingly incorrect DCHECK-s.
......................................................................


Patch Set 1:

(1 comment)

I agree that this looks like a bug.

http://gerrit.cloudera.org:8080/#/c/4835/1//COMMIT_MSG
Commit Message:

Line 7: Remove seemingly incorrect DCHECK-s.
Can you file an Impala JIRA for this?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I52e1b1354e9ea056b49331e75e53759952a81b76
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zoltan Ivanfi <zi...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4371: Incorrect DCHECK-s in hdfs-parquet-table-writer

Posted by "Zoltan Ivanfi (Code Review)" <ge...@cloudera.org>.
Zoltan Ivanfi has posted comments on this change.

Change subject: IMPALA-4371: Incorrect DCHECK-s in hdfs-parquet-table-writer
......................................................................


Patch Set 2:

> Oh, oops, I wasn't noticing this was the writier.  In that case,
 > why are these DCHECKs not valid?  Why does our code create pages
 > with zero values (other than the last)? Or were we hitting the
 > non-zero compressed size dcheck (and if so, that also doesn't seem
 > to make sense).

Originally I explained the reason in the commit message, but when I was asked to open a JIRA about it, I moved the explanation there. Please see https://issues.cloudera.org/browse/IMPALA-4371

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I52e1b1354e9ea056b49331e75e53759952a81b76
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zoltan Ivanfi <zi...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zoltan Ivanfi <zi...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4371: Incorrect DCHECK-s in hdfs-parquet-table-writer

Posted by "Internal Jenkins (Code Review)" <ge...@cloudera.org>.
Internal Jenkins has submitted this change and it was merged.

Change subject: IMPALA-4371: Incorrect DCHECK-s in hdfs-parquet-table-writer
......................................................................


IMPALA-4371: Incorrect DCHECK-s in hdfs-parquet-table-writer

Change-Id: I52e1b1354e9ea056b49331e75e53759952a81b76
Reviewed-on: http://gerrit.cloudera.org:8080/4835
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
Reviewed-by: Dan Hecht <dh...@cloudera.com>
Tested-by: Internal Jenkins
---
M be/src/exec/hdfs-parquet-table-writer.cc
1 file changed, 1 insertion(+), 3 deletions(-)

Approvals:
  Internal Jenkins: Verified
  Dan Hecht: Looks good to me, approved
  Tim Armstrong: Looks good to me, but someone else must approve



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I52e1b1354e9ea056b49331e75e53759952a81b76
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zoltan Ivanfi <zi...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zoltan Ivanfi <zi...@cloudera.com>

[Impala-ASF-CR] IMPALA-4371: Incorrect DCHECK-s in hdfs-parquet-table-writer

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.

Change subject: IMPALA-4371: Incorrect DCHECK-s in hdfs-parquet-table-writer
......................................................................


Patch Set 2:

What about Dan's comment about the test. It would be great to add a regression test if it's reproducible.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I52e1b1354e9ea056b49331e75e53759952a81b76
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zoltan Ivanfi <zi...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4371: Incorrect DCHECK-s in hdfs-parquet-table-writer

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.

Change subject: IMPALA-4371: Incorrect DCHECK-s in hdfs-parquet-table-writer
......................................................................


Patch Set 2:

Sorry about that, should've known you were on vacation!

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I52e1b1354e9ea056b49331e75e53759952a81b76
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zoltan Ivanfi <zi...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zoltan Ivanfi <zi...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] Remove seemingly incorrect DCHECK-s.

Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Dan Hecht has posted comments on this change.

Change subject: Remove seemingly incorrect DCHECK-s.
......................................................................


Patch Set 1:

Is it reasonably possible to create a file that demonstrates this and can be added to the regression test suite?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I52e1b1354e9ea056b49331e75e53759952a81b76
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zoltan Ivanfi <zi...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4371: Incorrect DCHECK-s in hdfs-parquet-table-writer

Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Dan Hecht has posted comments on this change.

Change subject: IMPALA-4371: Incorrect DCHECK-s in hdfs-parquet-table-writer
......................................................................


Patch Set 2: Code-Review+2

> > Is it reasonably possible to create a file that demonstrates this
 > > and can be added to the regression test suite?
 > 
 > Dan, it has just occurred to me that since this condition happens
 > while writing a file and not while reading it, I don't see how we
 > could use a test file to test it. Additionally, I don't see a real
 > risk of regression, as I don't expect anyone to put in DCHECK-s
 > based on the same false assumptions again.
 > 
 > So considering the value of such a test and the difficulty of
 > reproducing the exact conditions triggering the DCHECK-s, I suggest
 > not creating a test file for the regression suite.

Couldn't you just hack up the Impala parquet writer (or any other parquet writing util) to generate such a parquet file to use as a test? 

That said, given the trivial nature of the change and the zero risk, I'm okay with deferring that.  Still would be nice to exercise this path, though, in case there is another problem downstream from this code that such a file would encounter.  

Do you have access to the file to repro'ed this in the wild?  And given that DCHECKs are compiled out of release builds, how did this cause a problem in the wild?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I52e1b1354e9ea056b49331e75e53759952a81b76
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zoltan Ivanfi <zi...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zoltan Ivanfi <zi...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4371: Incorrect DCHECK-s in hdfs-parquet-table-writer

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.

Change subject: IMPALA-4371: Incorrect DCHECK-s in hdfs-parquet-table-writer
......................................................................


Patch Set 2: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I52e1b1354e9ea056b49331e75e53759952a81b76
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zoltan Ivanfi <zi...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zoltan Ivanfi <zi...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4371: Incorrect DCHECK-s in hdfs-parquet-table-writer

Posted by "Zoltan Ivanfi (Code Review)" <ge...@cloudera.org>.
Zoltan Ivanfi has uploaded a new patch set (#2).

Change subject: IMPALA-4371: Incorrect DCHECK-s in hdfs-parquet-table-writer
......................................................................

IMPALA-4371: Incorrect DCHECK-s in hdfs-parquet-table-writer

Change-Id: I52e1b1354e9ea056b49331e75e53759952a81b76
---
M be/src/exec/hdfs-parquet-table-writer.cc
1 file changed, 1 insertion(+), 3 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/35/4835/2
-- 
To view, visit http://gerrit.cloudera.org:8080/4835
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I52e1b1354e9ea056b49331e75e53759952a81b76
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zoltan Ivanfi <zi...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-4371: Incorrect DCHECK-s in hdfs-parquet-table-writer

Posted by "Internal Jenkins (Code Review)" <ge...@cloudera.org>.
Internal Jenkins has posted comments on this change.

Change subject: IMPALA-4371: Incorrect DCHECK-s in hdfs-parquet-table-writer
......................................................................


Patch Set 2: Verified-1

Build failed: http://sandbox.jenkins.cloudera.com/job/impala-external-gerrit-verify-merge-ASF/414/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I52e1b1354e9ea056b49331e75e53759952a81b76
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zoltan Ivanfi <zi...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zoltan Ivanfi <zi...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4371: Incorrect DCHECK-s in hdfs-parquet-table-writer

Posted by "Zoltan Ivanfi (Code Review)" <ge...@cloudera.org>.
Zoltan Ivanfi has posted comments on this change.

Change subject: IMPALA-4371: Incorrect DCHECK-s in hdfs-parquet-table-writer
......................................................................


Patch Set 2:

> Is it reasonably possible to create a file that demonstrates this
 > and can be added to the regression test suite?

Dan, it has just occurred to me that since this condition happens while writing a file and not while reading it, I don't see how we could use a test file to test it. Additionally, I don't see a real risk of regression, as I don't expect anyone to put in DCHECK-s based on the same false assumptions again.

So considering the value of such a test and the difficulty of reproducing the exact conditions triggering the DCHECK-s, I suggest not creating a test file for the regression suite.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I52e1b1354e9ea056b49331e75e53759952a81b76
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zoltan Ivanfi <zi...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zoltan Ivanfi <zi...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4371: Incorrect DCHECK-s in hdfs-parquet-table-writer

Posted by "Internal Jenkins (Code Review)" <ge...@cloudera.org>.
Internal Jenkins has posted comments on this change.

Change subject: IMPALA-4371: Incorrect DCHECK-s in hdfs-parquet-table-writer
......................................................................


Patch Set 2: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I52e1b1354e9ea056b49331e75e53759952a81b76
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zoltan Ivanfi <zi...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zoltan Ivanfi <zi...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4371: Incorrect DCHECK-s in hdfs-parquet-table-writer

Posted by "Zoltan Ivanfi (Code Review)" <ge...@cloudera.org>.
Zoltan Ivanfi has posted comments on this change.

Change subject: IMPALA-4371: Incorrect DCHECK-s in hdfs-parquet-table-writer
......................................................................


Patch Set 2:

Hi Tim,

Sorry for being unresponsive, I was on vacation for a week.

I would vote for opening a follow-up JIRA for test coverage, as creating such a test file will be more complicated than identifying and fixing this issue was. However, we have seen this happening in the wild, so this is certainly not only a theoretical, but a real issue.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I52e1b1354e9ea056b49331e75e53759952a81b76
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zoltan Ivanfi <zi...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zoltan Ivanfi <zi...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4371: Incorrect DCHECK-s in hdfs-parquet-table-writer

Posted by "Zoltan Ivanfi (Code Review)" <ge...@cloudera.org>.
Zoltan Ivanfi has posted comments on this change.

Change subject: IMPALA-4371: Incorrect DCHECK-s in hdfs-parquet-table-writer
......................................................................


Patch Set 2:

> Please see https://issues.cloudera.org/browse/IMPALA-4371

It seems that Gerrit auto-linkifies JIRA entries even inside URLs, which results in invalid links. I should just probably refer to it as IMPALA-4371.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I52e1b1354e9ea056b49331e75e53759952a81b76
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zoltan Ivanfi <zi...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zoltan Ivanfi <zi...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4371: Incorrect DCHECK-s in hdfs-parquet-table-writer

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.

Change subject: IMPALA-4371: Incorrect DCHECK-s in hdfs-parquet-table-writer
......................................................................


Patch Set 2:

Ping - if reproduction is hard my preference is to get this merged and file a follow-on JIRA to add the test coverage.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I52e1b1354e9ea056b49331e75e53759952a81b76
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zoltan Ivanfi <zi...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4371: Incorrect DCHECK-s in hdfs-parquet-table-writer

Posted by "Internal Jenkins (Code Review)" <ge...@cloudera.org>.
Internal Jenkins has posted comments on this change.

Change subject: IMPALA-4371: Incorrect DCHECK-s in hdfs-parquet-table-writer
......................................................................


Patch Set 2:

Build failed: http://sandbox.jenkins.cloudera.com/job/impala-external-gerrit-verify-merge-ASF/416/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I52e1b1354e9ea056b49331e75e53759952a81b76
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zoltan Ivanfi <zi...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zoltan Ivanfi <zi...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4371: Incorrect DCHECK-s in hdfs-parquet-table-writer

Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Dan Hecht has posted comments on this change.

Change subject: IMPALA-4371: Incorrect DCHECK-s in hdfs-parquet-table-writer
......................................................................


Patch Set 2: -Code-Review

> > Do you have access to the file to repro'ed this in the wild?  And
 > > given that DCHECKs are compiled out of release builds, how did
 > this
 > > cause a problem in the wild?
 > 
 > In the wild, some `insert into ... select from ...` queries have
 > hit this check when converting CSV to Parquet, but I do not have
 > access to those CSV files. A debug build was used while
 > investigating an unrelated issue, that's how the DCHECK failure was
 > noticed.

Oh, oops, I wasn't noticing this was the writier.  In that case, why are these DCHECKs not valid?  Why does our code create pages with zero values (other than the last)? Or were we hitting the non-zero compressed size dcheck (and if so, that also doesn't seem to make sense).

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I52e1b1354e9ea056b49331e75e53759952a81b76
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zoltan Ivanfi <zi...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zoltan Ivanfi <zi...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4371: Incorrect DCHECK-s in hdfs-parquet-table-writer

Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Dan Hecht has posted comments on this change.

Change subject: IMPALA-4371: Incorrect DCHECK-s in hdfs-parquet-table-writer
......................................................................


Patch Set 2: Code-Review+2

I see, thanks.  (Maybe the second case is easier to reproduce by writing a large value?)

Also was wondering if IMPALA-3376 was leading to this, but I buy that it can happen still.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I52e1b1354e9ea056b49331e75e53759952a81b76
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zoltan Ivanfi <zi...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zoltan Ivanfi <zi...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4371: Incorrect DCHECK-s in hdfs-parquet-table-writer

Posted by "Zoltan Ivanfi (Code Review)" <ge...@cloudera.org>.
Zoltan Ivanfi has posted comments on this change.

Change subject: IMPALA-4371: Incorrect DCHECK-s in hdfs-parquet-table-writer
......................................................................


Patch Set 2:

> Do you have access to the file to repro'ed this in the wild?  And
 > given that DCHECKs are compiled out of release builds, how did this
 > cause a problem in the wild?

In the wild, some `insert into ... select from ...` queries have hit this check when converting CSV to Parquet, but I do not have access to those CSV files. A debug build was used while investigating an unrelated issue, that's how the DCHECK failure was noticed.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I52e1b1354e9ea056b49331e75e53759952a81b76
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zoltan Ivanfi <zi...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zoltan Ivanfi <zi...@cloudera.com>
Gerrit-HasComments: No