impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bharath Vissapragada (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-1427: Improvements to "Unknown disk-ID" warning
Date Mon, 06 Feb 2017 20:24:07 GMT
Bharath Vissapragada has posted comments on this change.

Change subject: IMPALA-1427: Improvements to "Unknown disk-ID" warning
......................................................................


Patch Set 6:

(3 comments)

- I'm not totally sure why IMPALA-1427 wasn't closed. Looks like it should've been

- The motivation for this fix was that we've started seeing more users report this warning
in their query runs and we never printed which table/file was causing it. So we need to dig
through all the Catalog logs to figure out which table loading reported such missing disk
ids to narrow it down and it is much difficult with multiple tables reporting it.

- This started off as a simple fix (PS1) that associated the table name to the warning (which
we weren't doing earlier). However, Alex made some pretty interesting comments that we unnecessarily
report it at query runtime when we could've done the same at the compile time. That sounded
unnecessary to me.

> but won't this make the warning less visible, and if so, what's the justification for
that?

IMO this has the same level of severity as the missing stats issue in the sense that it doesn't
cause query failures but can cause perf degradation. Given we still print it as a WARNING
in the explain as well as the profiles, wouldn't it still maintain the visibility?

http://gerrit.cloudera.org:8080/#/c/5828/6//COMMIT_MSG
Commit Message:

Line 17: 
> put a bullet for the new compile-time warning.
Done


http://gerrit.cloudera.org:8080/#/c/5828/6/common/thrift/ImpalaInternalService.thrift
File common/thrift/ImpalaInternalService.thrift:

Line 324:   15: optional list<CatalogObjects.TTableName> tables_missing_diskids
> this isn't used by the backend, so it's kinda unfortunate to carry it aroun
Good catch. done.


http://gerrit.cloudera.org:8080/#/c/5828/6/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java:

Line 614:             totalFiles_, numScanRangesNoDiskIds_, scanRanges_.size()));
> does this get printed when the table lives on a FS that doesn't have a noti
Good catch. Refactored the logic a little to take into account the type of file system being
checked.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iddb132ff7ad66f3291b93bf9d8061bd0525ef1b2
Gerrit-PatchSet: 6
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: Dan Hecht <dhecht@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message