You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org> on 2017/01/30 18:52:17 UTC
[Impala-ASF-CR] Improvements to "Unknown disk-ID" warning
Bharath Vissapragada has uploaded a new change for review.
http://gerrit.cloudera.org:8080/5828
Change subject: Improvements to "Unknown disk-ID" warning
......................................................................
Improvements to "Unknown disk-ID" warning
This commit,
- Appends the table name to the "Unknown disk-ID" warning when
disk IDs are missing, for supportability.
- Removes reference to enabling dfs block metadata configuration,
since it doesn't apply anymore.
- Removes VolumeId terminology from the runtime profile.
Change-Id: Iddb132ff7ad66f3291b93bf9d8061bd0525ef1b2
---
M be/src/exec/hdfs-scan-node-base.cc
M common/thrift/generate_error_codes.py
2 files changed, 5 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/28/5828/1
--
To view, visit http://gerrit.cloudera.org:8080/5828
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iddb132ff7ad66f3291b93bf9d8061bd0525ef1b2
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
[Impala-ASF-CR] IMPALA-1427: Improvements to "Unknown disk-ID" warning
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change.
Change subject: IMPALA-1427: Improvements to "Unknown disk-ID" warning
......................................................................
Patch Set 7: Code-Review+2
--
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: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-1427: Improvements to "Unknown disk-ID" warning
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change.
Change subject: IMPALA-1427: Improvements to "Unknown disk-ID" warning
......................................................................
Patch Set 5:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/5828/5//COMMIT_MSG
Commit Message:
Line 9: This commit,
> remove
Done
http://gerrit.cloudera.org:8080/#/c/5828/5/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java:
Line 610: output.append(String.format("%smissing disk ids: " +
> let's not output this when there are no missing disk ids
Done
--
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: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1427: Improvements to "Unknown disk-ID" warning
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change.
Change subject: IMPALA-1427: Improvements to "Unknown disk-ID" warning
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/5828/2/common/thrift/generate_error_codes.py
File common/thrift/generate_error_codes.py:
Line 97: ("HDFS_SCAN_NODE_UNKNOWN_DISK", 26, "Encountered unknown disk id for table: '$0'."
Forgot to git-add changes for this file. Uploading a new PS.
--
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: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1427: Improvements to "Unknown disk-ID" warning
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change.
Change subject: IMPALA-1427: Improvements to "Unknown disk-ID" warning
......................................................................
Patch Set 6: Code-Review+2
Carrying Alex's +2. Will wait for Dan to sign off.
--
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 <bh...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-1427: Improvements to "Unknown disk-ID" warning
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has uploaded a new patch set (#2).
Change subject: IMPALA-1427: Improvements to "Unknown disk-ID" warning
......................................................................
IMPALA-1427: Improvements to "Unknown disk-ID" warning
This commit,
- Removes the runtime unknown disk ID reporting and instead moves
it to the explain plan as a counter that prints the number of
scan ranges missing disk IDs in the corresponding HDFS scan nodes.
- Removes reference to enabling dfs block metadata configuration,
since it doesn't apply anymore.
- Removes VolumeId terminology from the runtime profile.
Change-Id: Iddb132ff7ad66f3291b93bf9d8061bd0525ef1b2
---
M be/src/exec/hdfs-scan-node-base.cc
M be/src/exec/hdfs-scan-node-base.h
M common/thrift/generate_error_codes.py
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
4 files changed, 11 insertions(+), 19 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/28/5828/2
--
To view, visit http://gerrit.cloudera.org:8080/5828
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iddb132ff7ad66f3291b93bf9d8061bd0525ef1b2
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
[Impala-ASF-CR] IMPALA-1427: Improvements to "Unknown disk-ID" warning
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged.
Change subject: IMPALA-1427: Improvements to "Unknown disk-ID" warning
......................................................................
IMPALA-1427: Improvements to "Unknown disk-ID" warning
- Removes the runtime unknown disk ID reporting and instead moves
it to the explain plan as a counter that prints the number of
scan ranges missing disk IDs in the corresponding HDFS scan nodes.
- Adds a warning to the header of query profile/explain plan with a
list of tables missing disk ids.
- Removes reference to enabling dfs block metadata configuration,
since it doesn't apply anymore.
- Removes VolumeId terminology from the runtime profile.
Change-Id: Iddb132ff7ad66f3291b93bf9d8061bd0525ef1b2
Reviewed-on: http://gerrit.cloudera.org:8080/5828
Reviewed-by: Bharath Vissapragada <bh...@cloudera.com>
Tested-by: Impala Public Jenkins
---
M be/src/exec/hdfs-scan-node-base.cc
M be/src/exec/hdfs-scan-node-base.h
M be/src/service/query-exec-state.cc
M common/thrift/ImpalaInternalService.thrift
M common/thrift/generate_error_codes.py
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
8 files changed, 166 insertions(+), 102 deletions(-)
Approvals:
Impala Public Jenkins: Verified
Bharath Vissapragada: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/5828
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Iddb132ff7ad66f3291b93bf9d8061bd0525ef1b2
Gerrit-PatchSet: 9
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-1427: Improvements to "Unknown disk-ID" warning
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: IMPALA-1427: Improvements to "Unknown disk-ID" warning
......................................................................
Patch Set 8: Verified+1
--
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: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-1427: Improvements to "Unknown disk-ID" warning
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change.
Change subject: IMPALA-1427: Improvements to "Unknown disk-ID" warning
......................................................................
Patch Set 7:
Thanks for the review Dan. I'll wait for Alex's +2.
I ran a quick test against an S3 table and I don't see missing disk ID warnings.
[localhost:21000] > show create table tpch.customer;
Query: show create table tpch.customer
+---------------------------------------------------------------------------------------------------------------------+
| result |
+---------------------------------------------------------------------------------------------------------------------+
| CREATE EXTERNAL TABLE tpch.customer ( |
| c_custkey BIGINT, |
| c_name STRING, |
| c_address STRING, |
| c_nationkey SMALLINT, |
| c_phone STRING, |
| c_acctbal DECIMAL(12,2), |
| c_mktsegment STRING, |
| c_comment STRING |
| ) |
| ROW FORMAT DELIMITED FIELDS TERMINATED BY '|' |
| WITH SERDEPROPERTIES ('serialization.format'='|', 'field.delim'='|') |
| STORED AS TEXTFILE |
| LOCATION 's3a://impala-cdh5-trunk/test-warehouse/tpch.customer' |
| TBLPROPERTIES ('numFiles'='0', 'COLUMN_STATS_ACCURATE'='true', 'numRows'='-1', 'totalSize'='0', 'rawDataSize'='-1') |
+---------------------------------------------------------------------------------------------------------------------+
Fetched 1 row(s) in 0.01s
[localhost:21000] > select count(*) from tpch.customer;
Query: select count(*) from tpch.customer
Query submitted at: 2017-02-06 13:43:02 (Coordinator: http://optimus:25000)
Query progress can be monitored at: http://localhost:25000/query_plan?query_id=4e4afd8ed0ed8a76:16fd658200000000
+----------+
| count(*) |
+----------+
| 150000 |
+----------+
Fetched 1 row(s) in 0.88s
[localhost:21000] > explain select count(*) from tpch.customer;
Query: explain select count(*) from tpch.customer
+------------------------------------------------------------------------------------+
| Explain String |
+------------------------------------------------------------------------------------+
| Estimated Per-Host Requirements: Memory=74.00MB VCores=1 |
| WARNING: The following tables are missing relevant table and/or column statistics. |
| tpch.customer |
| |
| PLAN-ROOT SINK |
| | |
| 03:AGGREGATE [FINALIZE] |
| | output: count:merge(*) |
| | |
| 02:EXCHANGE [UNPARTITIONED] |
| | |
| 01:AGGREGATE |
| | output: count(*) |
| | |
| 00:SCAN HDFS [tpch.customer] |
| partitions=1/1 files=1 size=23.08MB |
+------------------------------------------------------------------------------------+
Fetched 16 row(s) in 0.01s
[localhost:21000] > set explain_level=3;
EXPLAIN_LEVEL set to 3
[localhost:21000] > explain select count(*) from tpch.customer;
Query: explain select count(*) from tpch.customer
+------------------------------------------------------------------------------------+
| Explain String |
+------------------------------------------------------------------------------------+
| Estimated Per-Host Requirements: Memory=74.00MB VCores=1 |
| WARNING: The following tables are missing relevant table and/or column statistics. |
| tpch.customer |
| |
| F01:PLAN FRAGMENT [UNPARTITIONED] |
| PLAN-ROOT SINK |
| | |
| 03:AGGREGATE [FINALIZE] |
| | output: count:merge(*) |
| | hosts=1 per-host-mem=unavailable |
| | tuple-ids=1 row-size=8B cardinality=1 |
| | |
| 02:EXCHANGE [UNPARTITIONED] |
| hosts=1 per-host-mem=unavailable |
| tuple-ids=1 row-size=8B cardinality=1 |
| |
| F00:PLAN FRAGMENT [RANDOM] |
| DATASTREAM SINK [FRAGMENT=F01, EXCHANGE=02, UNPARTITIONED] |
| 01:AGGREGATE |
| | output: count(*) |
| | hosts=1 per-host-mem=10.00MB |
| | tuple-ids=1 row-size=8B cardinality=1 |
| | |
| 00:SCAN HDFS [tpch.customer, RANDOM] |
| partitions=1/1 files=1 size=23.08MB |
| table stats: unavailable |
| column stats: all |
| hosts=1 per-host-mem=64.00MB |
| tuple-ids=0 row-size=0B cardinality=unavailable |
+------------------------------------------------------------------------------------+
Fetched 29 row(s) in 0.01s
Profile snippet-
Planner Timeline: 2.947ms
- Analysis finished: 879.310us (879.310us)
- Equivalence classes computed: 943.765us (64.455us)
- Single node plan created: 1.337ms (393.340us)
- Runtime filters computed: 1.357ms (20.630us)
- Distributed plan created: 1.546ms (188.497us)
- Planning finished: 2.947ms (1.401ms)
Query Timeline: 623.505ms
- Query submitted: 38.990us (38.990us)
- Planning finished: 3.766ms (3.727ms)
- Submit for admission: 3.946ms (180.034us)
- Completed admission: 3.987ms (41.480us)
- Ready to start 2 fragment instances: 4.125ms (137.963us)
- All 2 fragment instances started: 4.729ms (603.115us)
- Rows available: 543.038ms (538.309ms)
- First row fetched: 620.644ms (77.606ms)
- Unregister query: 622.297ms (1.652ms)
- ComputeScanRangeAssignmentTimer: 40.299us
ImpalaServer:
- ClientFetchWaitTimer: 78.305ms
- RowMaterializationTimer: 934.632us
Execution Profile 3549f42079a127df:70e6a82900000000:(Total: 539.718ms, non-child: 0.000ns, % non-child: 0.00%)
Number of filters: 0
Filter routing table:
ID Src. Node Tgt. Node(s) Targets Target type Partition filter Pending (Expected) First arrived Completed Enabled
----------------------------------------------------------------------------------------------------------------------------
Fragment instance start latencies: Count: 2, 25th %-ile: 0, 50th %-ile: 0, 75th %-ile: 1ms, 90th %-ile: 1ms, 95th %-ile: 1ms, 99.9th %-ile: 1ms
Per Node Peak Memory Usage: optimus:22000(24.06 MB)
- FiltersReceived: 0 (0)
- FinalizationTimer: 0.000ns
--
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: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-1427: Improvements to "Unknown disk-ID" warning
Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Dan Hecht has posted comments on this change.
Change subject: IMPALA-1427: Improvements to "Unknown disk-ID" warning
......................................................................
Patch Set 7:
I thought the missing stats was a query warning, but it looks like it's not. I agree it makes sense for this warning to be compile time and at the same "level" as missing stats.
Can you verify that the various new messages don't show up on S3 by running a quick test against S3?
Otherwise, looks good to me. Alex, why don't you take a final look at the latest version and give the +2 again.
--
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: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-1427: Improvements to "Unknown disk-ID" warning
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change.
Change subject: IMPALA-1427: Improvements to "Unknown disk-ID" warning
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/5828/1//COMMIT_MSG
Commit Message:
Line 7: IMPALA-1427: Improvements to "Unknown disk-ID" warning
> IMPALA-1427?
Done
Line 9: This commit,
> This patch seems fine.
Great suggestion. That makes sense to me. Adding it.
--
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: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1427: Improvements to "Unknown disk-ID" warning
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
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 <bh...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1427: Improvements to "Unknown disk-ID" warning
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change.
Change subject: IMPALA-1427: Improvements to "Unknown disk-ID" warning
......................................................................
Patch Set 3:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/5828/3/be/src/exec/hdfs-scan-node-base.cc
File be/src/exec/hdfs-scan-node-base.cc:
Line 62
> Sorry for the confusion.
Ah, thanks for clarifying. Makes sense now. I thought gflags would gracefully handle that situation and ignore any such un-declared flags. I'll make this change.
http://gerrit.cloudera.org:8080/#/c/5828/3/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java:
Line 343: if (block.getDiskId(i) == -1) ++numScanRangesNoDiskIds_;
> scan ranges is a pretty low granularity, can you also roll this up to the f
Done
Line 589: if (numScanRangesNoDiskIds_ > 0) {
> let's only add this for >= EXTENDED
Done
--
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: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1427: Improvements to "Unknown disk-ID" warning
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change.
Change subject: IMPALA-1427: Improvements to "Unknown disk-ID" warning
......................................................................
Patch Set 8:
Looks like it is not related to the PATCH. Can someone please re-trigger this job?
16:35:07 Traceback (most recent call last):
16:35:07 File "/mnt/tmp.li7HlvN7pW/infra/python/env/local/lib/python2.7/site-packages/pip/basecommand.py", line 223, in main
16:35:07 status = self.run(options, args)
16:35:07 File "/mnt/tmp.li7HlvN7pW/infra/python/env/local/lib/python2.7/site-packages/pip/commands/install.py", line 293, in run
16:35:07 wb.build(autobuilding=True)
16:35:07 File "/mnt/tmp.li7HlvN7pW/infra/python/env/local/lib/python2.7/site-packages/pip/wheel.py", line 705, in build
16:35:07 self.requirement_set.prepare_files(self.finder)
16:35:07 File "/mnt/tmp.li7HlvN7pW/infra/python/env/local/lib/python2.7/site-packages/pip/req/req_set.py", line 334, in prepare_files
16:35:07 functools.partial(self._prepare_file, finder))
16:35:07 File "/mnt/tmp.li7HlvN7pW/infra/python/env/local/lib/python2.7/site-packages/pip/req/req_set.py", line 321, in _walk_req_to_install
16:35:07 more_reqs = handler(req_to_install)
16:35:07 File "/mnt/tmp.li7HlvN7pW/infra/python/env/local/lib/python2.7/site-packages/pip/req/req_set.py", line 491, in _prepare_file
16:35:07 session=self.session)
16:35:07 File "/mnt/tmp.li7HlvN7pW/infra/python/env/local/lib/python2.7/site-packages/pip/download.py", line 825, in unpack_url
16:35:07 session,
16:35:07 File "/mnt/tmp.li7HlvN7pW/infra/python/env/local/lib/python2.7/site-packages/pip/download.py", line 673, in unpack_http_url
16:35:07 from_path, content_type = _download_http_url(link, session, temp_dir)
16:35:07 File "/mnt/tmp.li7HlvN7pW/infra/python/env/local/lib/python2.7/site-packages/pip/download.py", line 886, in _download_http_url
16:35:07 _download_url(resp, link, content_file)
16:35:07 File "/mnt/tmp.li7HlvN7pW/infra/python/env/local/lib/python2.7/site-packages/pip/download.py", line 621, in _download_url
16:35:07 for chunk in progress_indicator(resp_read(4096), 4096):
16:35:07 File "/mnt/tmp.li7HlvN7pW/infra/python/env/local/lib/python2.7/site-packages/pip/utils/ui.py", line 133, in iter
16:35:07 for x in it:
16:35:07 File "/mnt/tmp.li7HlvN7pW/infra/python/env/local/lib/python2.7/site-packages/pip/download.py", line 586, in resp_read
16:35:07 decode_content=False):
16:35:07 File "/mnt/tmp.li7HlvN7pW/infra/python/env/local/lib/python2.7/site-packages/pip/_vendor/requests/packages/urllib3/response.py", line 307, in stream
16:35:07 data = self.read(amt=amt, decode_content=decode_content)
16:35:07 File "/mnt/tmp.li7HlvN7pW/infra/python/env/local/lib/python2.7/site-packages/pip/_vendor/requests/packages/urllib3/response.py", line 243, in read
16:35:07 data = self._fp.read(amt)
16:35:07 File "/mnt/tmp.li7HlvN7pW/infra/python/env/local/lib/python2.7/site-packages/pip/_vendor/cachecontrol/filewrapper.py", line 49, in read
16:35:07 data = self.__fp.read(amt)
16:35:07 File "/usr/lib/python2.7/httplib.py", line 602, in read
16:35:07 s = self.fp.read(amt)
16:35:07 File "/usr/lib/python2.7/socket.py", line 380, in read
16:35:07 data = self._sock.recv(left)
16:35:07 File "/usr/lib/python2.7/ssl.py", line 341, in recv
16:35:07 return self.read(buflen)
16:35:07 File "/usr/lib/python2.7/ssl.py", line 260, in read
16:35:07 return self._sslobj.read(len)
16:35:07 error: [Errno 104] Connection reset by peer
--
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: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-1427: Improvements to "Unknown disk-ID" warning
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change.
Change subject: IMPALA-1427: Improvements to "Unknown disk-ID" warning
......................................................................
Patch Set 4:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/5828/4/be/src/exec/hdfs-scan-node-base.cc
File be/src/exec/hdfs-scan-node-base.cc:
Line 63: // TODO: Remove this flag.
> ... in a compatibility-breaking release.
Done
http://gerrit.cloudera.org:8080/#/c/5828/4/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java:
Line 125: // Counters that track the number of objects mapping to an unknown disk ID. Here, we use
> How about:
Sounds simpler. Done
Line 129: private int numFilesNoDiskIds_ = 0;
> remove blank lines (helps wit visual clustering)
Done
Line 355: LOG.trace("Hdfs block missing disk ID information: " + block.toString());
> I don't think this prints the full file path right? I think printing the fi
Done
http://gerrit.cloudera.org:8080/#/c/5828/4/fe/src/main/java/org/apache/impala/planner/Planner.java
File fe/src/main/java/org/apache/impala/planner/Planner.java:
Line 313: "disk Id information.\n" + Joiner.on(", ").join(tableNames) + "\n");
> disk id
Done
http://gerrit.cloudera.org:8080/#/c/5828/4/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:
Line 953: queryCtx.addToTables_missing_stats(
> The old code with TreeSet was removing duplicate table names, are you still
Ouch, I didn't realize that logic was for removing duplicates. Putting it back. Nice catch.
--
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: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1427: Improvements to "Unknown disk-ID" warning
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change.
Change subject: IMPALA-1427: Improvements to "Unknown disk-ID" warning
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/5828/3/be/src/exec/hdfs-scan-node-base.cc
File be/src/exec/hdfs-scan-node-base.cc:
Line 62
> Just want to confirm before I make changes, do you mean we should retain it
Sorry for the confusion.
What I mean is that we should preserve this option exactly as is because if somebody has that option set, then that cluster won't start anymore after this change. Clusters configured that way should continue to start. We can remove the option during a compatibility-breaking release.
You should change the textual description of this option to clearly indicate that it is deprecated, something like:
DEFINE_bool(suppress_unknown_disk_id_warnings, false,
"Deprecated");
You can grep for "Deprecated" and see how we deal with this elsewhere.
--
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: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1427: Improvements to "Unknown disk-ID" warning
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has uploaded a new patch set (#5).
Change subject: IMPALA-1427: Improvements to "Unknown disk-ID" warning
......................................................................
IMPALA-1427: Improvements to "Unknown disk-ID" warning
This commit,
- Removes the runtime unknown disk ID reporting and instead moves
it to the explain plan as a counter that prints the number of
scan ranges missing disk IDs in the corresponding HDFS scan nodes.
- Removes reference to enabling dfs block metadata configuration,
since it doesn't apply anymore.
- Removes VolumeId terminology from the runtime profile.
Change-Id: Iddb132ff7ad66f3291b93bf9d8061bd0525ef1b2
---
M be/src/exec/hdfs-scan-node-base.cc
M be/src/exec/hdfs-scan-node-base.h
M common/thrift/ImpalaInternalService.thrift
M common/thrift/generate_error_codes.py
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
7 files changed, 132 insertions(+), 101 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/28/5828/5
--
To view, visit http://gerrit.cloudera.org:8080/5828
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iddb132ff7ad66f3291b93bf9d8061bd0525ef1b2
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
[Impala-ASF-CR] IMPALA-1427: Improvements to "Unknown disk-ID" warning
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change.
Change subject: IMPALA-1427: Improvements to "Unknown disk-ID" warning
......................................................................
Patch Set 3:
(3 comments)
Thanks for doing this, Bharath! Much better. Just a few more requests as final polish.
http://gerrit.cloudera.org:8080/#/c/5828/3/be/src/exec/hdfs-scan-node-base.cc
File be/src/exec/hdfs-scan-node-base.cc:
Line 62
I think it's safer to leave this, but mention in the description that this option is deprecated. Also add a TODO to get rid of this.
http://gerrit.cloudera.org:8080/#/c/5828/3/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java:
Line 343: if (block.getDiskId(i) == -1) ++numScanRangesNoDiskIds_;
scan ranges is a pretty low granularity, can you also roll this up to the file and partition level?
I imagine the explain output looking like this:
missing disk ids: x/y scan ranges, x/y files, x/y partitions
also seems useful to LOG.trace() the file paths (not just the file name) that are missing disk ids, so we can get them if needed. Looking forward to your logging change also :)
Line 589: if (numScanRangesNoDiskIds_ > 0) {
let's only add this for >= EXTENDED
it will show up in query profiles, but not in regular explain plans
I think we should add a warning at the top of the explain plan listing the tables that have missing disk ids (like we do for missing stats)
--
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: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1427: Improvements to "Unknown disk-ID" warning
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change.
Change subject: IMPALA-1427: Improvements to "Unknown disk-ID" warning
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/5828/3/be/src/exec/hdfs-scan-node-base.cc
File be/src/exec/hdfs-scan-node-base.cc:
Line 62
> I think it's safer to leave this, but mention in the description that this
Just want to confirm before I make changes, do you mean we should retain it here as "suppress_unknown_disk_id_warnings_deprecated" or something and not use it anywhere in the code? If yes, I'm wondering how does that help the users.
--
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: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1427: Improvements to "Unknown disk-ID" warning
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change.
Change subject: IMPALA-1427: Improvements to "Unknown disk-ID" warning
......................................................................
Patch Set 4:
A sample output looks like this.
[localhost:21000] > explain select * from logging_test;
Query: explain select * from logging_test
+------------------------------------------------------------------------------------+
| Explain String |
+------------------------------------------------------------------------------------+
| Estimated Per-Host Requirements: Memory=32.00MB VCores=1 |
| WARNING: The following tables are missing relevant table and/or column statistics. |
| default.logging_test |
| WARNING: The following tables have scan ranges with missing disk Id information. |
| default.logging_test |
| |
| F01:PLAN FRAGMENT [UNPARTITIONED] |
| PLAN-ROOT SINK |
| | |
| 01:EXCHANGE [UNPARTITIONED] |
| hosts=1 per-host-mem=unavailable |
| tuple-ids=0 row-size=4B cardinality=unavailable |
| |
| F00:PLAN FRAGMENT [RANDOM] |
| DATASTREAM SINK [FRAGMENT=F01, EXCHANGE=01, UNPARTITIONED] |
| 00:SCAN HDFS [default.logging_test, RANDOM] |
| partitions=1/1 files=1 size=2B |
| table stats: unavailable |
| column stats: unavailable |
| missing disk ids: partitions=0/1 files=0/1 scan ranges 0/1 |
| hosts=1 per-host-mem=32.00MB |
| tuple-ids=0 row-size=4B cardinality=unavailable |
+------------------------------------------------------------------------------------+
Fetched 22 row(s) in 5.23s
[localhost:21000] >
--
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: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-1427: Improvements to "Unknown disk-ID" warning
Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Dan Hecht has posted comments on this change.
Change subject: IMPALA-1427: Improvements to "Unknown disk-ID" warning
......................................................................
Patch Set 6:
(3 comments)
AFAICT, IMPALA-1427 should have been resolved when we stopped printing the warning multiple times. So, what's the motivation for this change now? I'm not necessarily opposed to it, but won't this make the warning less visible, and if so, what's the justification for that?
http://gerrit.cloudera.org:8080/#/c/5828/6//COMMIT_MSG
Commit Message:
Line 17:
put a bullet for the new compile-time warning.
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 around with us in the TQueryCtx. But it is consistent with tables_missing_stats, etc, so okay for now.
Oh, actually, table_missing_stats is used in ExecQueryOrDmlRequest(). Should we be doing something similar for missing disk ids?
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 notion of disk ids, e.g. S3?
not your change, but if we want to do it for this case, is there a reason we aren't doing this for the missing stats case also?
--
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 <bh...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1427: Improvements to "Unknown disk-ID" warning
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has uploaded a new patch set (#4).
Change subject: IMPALA-1427: Improvements to "Unknown disk-ID" warning
......................................................................
IMPALA-1427: Improvements to "Unknown disk-ID" warning
This commit,
- Removes the runtime unknown disk ID reporting and instead moves
it to the explain plan as a counter that prints the number of
scan ranges missing disk IDs in the corresponding HDFS scan nodes.
- Removes reference to enabling dfs block metadata configuration,
since it doesn't apply anymore.
- Removes VolumeId terminology from the runtime profile.
Change-Id: Iddb132ff7ad66f3291b93bf9d8061bd0525ef1b2
---
M be/src/exec/hdfs-scan-node-base.cc
M be/src/exec/hdfs-scan-node-base.h
M common/thrift/ImpalaInternalService.thrift
M common/thrift/generate_error_codes.py
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
7 files changed, 129 insertions(+), 107 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/28/5828/4
--
To view, visit http://gerrit.cloudera.org:8080/5828
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iddb132ff7ad66f3291b93bf9d8061bd0525ef1b2
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
[Impala-ASF-CR] IMPALA-1427: Improvements to "Unknown disk-ID" warning
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: IMPALA-1427: Improvements to "Unknown disk-ID" warning
......................................................................
Patch Set 8:
Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/243/
--
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: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-1427: Improvements to "Unknown disk-ID" warning
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change.
Change subject: IMPALA-1427: Improvements to "Unknown disk-ID" warning
......................................................................
Patch Set 5: Code-Review+2
(2 comments)
Before you merge, let me ask Dan to take a quick look to see if he agrees with removing the disk id stuff from the BE.
http://gerrit.cloudera.org:8080/#/c/5828/5//COMMIT_MSG
Commit Message:
Line 9: This commit,
remove
http://gerrit.cloudera.org:8080/#/c/5828/5/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java:
Line 610: output.append(String.format("%smissing disk ids: " +
let's not output this when there are no missing disk ids
--
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: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1427: Improvements to "Unknown disk-ID" warning
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change.
Change subject: IMPALA-1427: Improvements to "Unknown disk-ID" warning
......................................................................
Patch Set 8:
Full trace, if someone is interested.
16:35:06 INFO:bootstrap_virtualenv:Installing packages into the virtualenv
16:35:07 Traceback (most recent call last):
16:35:07 File "/mnt/tmp.li7HlvN7pW/bin/../infra/python/bootstrap_virtualenv.py", line 287, in <module>
16:35:07 setup_virtualenv_if_not_exists()
16:35:07 File "/mnt/tmp.li7HlvN7pW/bin/../infra/python/bootstrap_virtualenv.py", line 264, in setup_virtualenv_if_not_exists
16:35:07 install_deps()
16:35:07 File "/mnt/tmp.li7HlvN7pW/bin/../infra/python/bootstrap_virtualenv.py", line 130, in install_deps
16:35:07 exec_pip_install(["-r", REQS_PATH])
16:35:07 File "/mnt/tmp.li7HlvN7pW/bin/../infra/python/bootstrap_virtualenv.py", line 93, in exec_pip_install
16:35:07 "file://%s" % urllib.pathname2url(os.path.abspath(DEPS_DIR))] + args, **popen_kwargs)
16:35:07 File "/mnt/tmp.li7HlvN7pW/bin/../infra/python/bootstrap_virtualenv.py", line 80, in exec_cmd
16:35:07 % (args, output))
16:35:07 Exception: Command returned non-zero status
16:35:07 Command: ['/mnt/tmp.li7HlvN7pW/bin/../infra/python/env/bin/python', '/mnt/tmp.li7HlvN7pW/bin/../infra/python/env/bin/pip', 'install', '--no-binary', '--no-index', '--find-links', 'file:///mnt/tmp.li7HlvN7pW/infra/python/deps', '-r', '/mnt/tmp.li7HlvN7pW/bin/../infra/python/deps/requirements.txt']
16:35:07 Output: /mnt/tmp.li7HlvN7pW/infra/python/env/local/lib/python2.7/site-packages/pip/_vendor/requests/packages/urllib3/util/ssl_.py:90: InsecurePlatformWarning: A true SSLContext object is not available. This prevents urllib3 from configuring SSL appropriately and may cause certain SSL connections to fail. For more information, see https://urllib3.readthedocs.org/en/latest/security.html#insecureplatformwarning.
16:35:07 InsecurePlatformWarning
16:35:07 You are using pip version 7.1.0, however version 9.0.1 is available.
16:35:07 You should consider upgrading via the 'pip install --upgrade pip' command.
16:35:07 Ignoring readline: markers "sys_platform == 'darwin'" don't match your environment
16:35:07 Collecting allpairs==2.0.1 (from -r /mnt/tmp.li7HlvN7pW/bin/../infra/python/deps/requirements.txt (line 1))
16:35:07 /mnt/tmp.li7HlvN7pW/infra/python/env/local/lib/python2.7/site-packages/pip/_vendor/requests/packages/urllib3/util/ssl_.py:90: InsecurePlatformWarning: A true SSLContext object is not available. This prevents urllib3 from configuring SSL appropriately and may cause certain SSL connections to fail. For more information, see https://urllib3.readthedocs.org/en/latest/security.html#insecureplatformwarning.
16:35:07 InsecurePlatformWarning
16:35:07 Collecting argparse==1.4.0 (from -r /mnt/tmp.li7HlvN7pW/bin/../infra/python/deps/requirements.txt (line 2))
16:35:07 Downloading argparse-1.4.0-py2.py3-none-any.whl
16:35:07 Collecting boto3==1.2.3 (from -r /mnt/tmp.li7HlvN7pW/bin/../infra/python/deps/requirements.txt (line 3))
16:35:07 Downloading boto3-1.2.3-py2.py3-none-any.whl (108kB)
16:35:07 Collecting simplejson==3.3.0 (from -r /mnt/tmp.li7HlvN7pW/bin/../infra/python/deps/requirements.txt (line 4))
16:35:07 Collecting botocore==1.3.30 (from -r /mnt/tmp.li7HlvN7pW/bin/../infra/python/deps/requirements.txt (line 5))
16:35:07 Downloading botocore-1.3.30-py2.py3-none-any.whl (2.2MB)
16:35:07 Exception:
16:35:07 Traceback (most recent call last):
16:35:07 File "/mnt/tmp.li7HlvN7pW/infra/python/env/local/lib/python2.7/site-packages/pip/basecommand.py", line 223, in main
16:35:07 status = self.run(options, args)
16:35:07 File "/mnt/tmp.li7HlvN7pW/infra/python/env/local/lib/python2.7/site-packages/pip/commands/install.py", line 293, in run
16:35:07 wb.build(autobuilding=True)
16:35:07 File "/mnt/tmp.li7HlvN7pW/infra/python/env/local/lib/python2.7/site-packages/pip/wheel.py", line 705, in build
16:35:07 self.requirement_set.prepare_files(self.finder)
16:35:07 File "/mnt/tmp.li7HlvN7pW/infra/python/env/local/lib/python2.7/site-packages/pip/req/req_set.py", line 334, in prepare_files
16:35:07 functools.partial(self._prepare_file, finder))
16:35:07 File "/mnt/tmp.li7HlvN7pW/infra/python/env/local/lib/python2.7/site-packages/pip/req/req_set.py", line 321, in _walk_req_to_install
16:35:07 more_reqs = handler(req_to_install)
16:35:07 File "/mnt/tmp.li7HlvN7pW/infra/python/env/local/lib/python2.7/site-packages/pip/req/req_set.py", line 491, in _prepare_file
16:35:07 session=self.session)
16:35:07 File "/mnt/tmp.li7HlvN7pW/infra/python/env/local/lib/python2.7/site-packages/pip/download.py", line 825, in unpack_url
16:35:07 session,
16:35:07 File "/mnt/tmp.li7HlvN7pW/infra/python/env/local/lib/python2.7/site-packages/pip/download.py", line 673, in unpack_http_url
16:35:07 from_path, content_type = _download_http_url(link, session, temp_dir)
16:35:07 File "/mnt/tmp.li7HlvN7pW/infra/python/env/local/lib/python2.7/site-packages/pip/download.py", line 886, in _download_http_url
16:35:07 _download_url(resp, link, content_file)
16:35:07 File "/mnt/tmp.li7HlvN7pW/infra/python/env/local/lib/python2.7/site-packages/pip/download.py", line 621, in _download_url
16:35:07 for chunk in progress_indicator(resp_read(4096), 4096):
16:35:07 File "/mnt/tmp.li7HlvN7pW/infra/python/env/local/lib/python2.7/site-packages/pip/utils/ui.py", line 133, in iter
16:35:07 for x in it:
16:35:07 File "/mnt/tmp.li7HlvN7pW/infra/python/env/local/lib/python2.7/site-packages/pip/download.py", line 586, in resp_read
16:35:07 decode_content=False):
16:35:07 File "/mnt/tmp.li7HlvN7pW/infra/python/env/local/lib/python2.7/site-packages/pip/_vendor/requests/packages/urllib3/response.py", line 307, in stream
16:35:07 data = self.read(amt=amt, decode_content=decode_content)
16:35:07 File "/mnt/tmp.li7HlvN7pW/infra/python/env/local/lib/python2.7/site-packages/pip/_vendor/requests/packages/urllib3/response.py", line 243, in read
16:35:07 data = self._fp.read(amt)
16:35:07 File "/mnt/tmp.li7HlvN7pW/infra/python/env/local/lib/python2.7/site-packages/pip/_vendor/cachecontrol/filewrapper.py", line 49, in read
16:35:07 data = self.__fp.read(amt)
16:35:07 File "/usr/lib/python2.7/httplib.py", line 602, in read
16:35:07 s = self.fp.read(amt)
16:35:07 File "/usr/lib/python2.7/socket.py", line 380, in read
16:35:07 data = self._sock.recv(left)
16:35:07 File "/usr/lib/python2.7/ssl.py", line 341, in recv
16:35:07 return self.read(buflen)
16:35:07 File "/usr/lib/python2.7/ssl.py", line 260, in read
16:35:07 return self._sslobj.read(len)
16:35:07 error: [Errno 104] Connection reset by peer
16:35:07
--
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: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-1427: Improvements to "Unknown disk-ID" warning
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change.
Change subject: IMPALA-1427: Improvements to "Unknown disk-ID" warning
......................................................................
Patch Set 8: Code-Review+2
Rebased, carrying +2. Thanks for the reviews.
--
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: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-1427: Improvements to "Unknown disk-ID" warning
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has uploaded a new patch set (#3).
Change subject: IMPALA-1427: Improvements to "Unknown disk-ID" warning
......................................................................
IMPALA-1427: Improvements to "Unknown disk-ID" warning
This commit,
- Removes the runtime unknown disk ID reporting and instead moves
it to the explain plan as a counter that prints the number of
scan ranges missing disk IDs in the corresponding HDFS scan nodes.
- Removes reference to enabling dfs block metadata configuration,
since it doesn't apply anymore.
- Removes VolumeId terminology from the runtime profile.
Change-Id: Iddb132ff7ad66f3291b93bf9d8061bd0525ef1b2
---
M be/src/exec/hdfs-scan-node-base.cc
M be/src/exec/hdfs-scan-node-base.h
M common/thrift/generate_error_codes.py
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
4 files changed, 84 insertions(+), 95 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/28/5828/3
--
To view, visit http://gerrit.cloudera.org:8080/5828
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iddb132ff7ad66f3291b93bf9d8061bd0525ef1b2
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
[Impala-ASF-CR] IMPALA-1427: Improvements to "Unknown disk-ID" warning
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Hello Alex Behm,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/5828
to look at the new patch set (#6).
Change subject: IMPALA-1427: Improvements to "Unknown disk-ID" warning
......................................................................
IMPALA-1427: Improvements to "Unknown disk-ID" warning
- Removes the runtime unknown disk ID reporting and instead moves
it to the explain plan as a counter that prints the number of
scan ranges missing disk IDs in the corresponding HDFS scan nodes.
- Removes reference to enabling dfs block metadata configuration,
since it doesn't apply anymore.
- Removes VolumeId terminology from the runtime profile.
Change-Id: Iddb132ff7ad66f3291b93bf9d8061bd0525ef1b2
---
M be/src/exec/hdfs-scan-node-base.cc
M be/src/exec/hdfs-scan-node-base.h
M common/thrift/ImpalaInternalService.thrift
M common/thrift/generate_error_codes.py
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
7 files changed, 134 insertions(+), 101 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/28/5828/6
--
To view, visit http://gerrit.cloudera.org:8080/5828
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iddb132ff7ad66f3291b93bf9d8061bd0525ef1b2
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
[Impala-ASF-CR] IMPALA-1427: Improvements to "Unknown disk-ID" warning
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change.
Change subject: IMPALA-1427: Improvements to "Unknown disk-ID" warning
......................................................................
Patch Set 4:
(6 comments)
So much better!
http://gerrit.cloudera.org:8080/#/c/5828/4/be/src/exec/hdfs-scan-node-base.cc
File be/src/exec/hdfs-scan-node-base.cc:
Line 63: // TODO: Remove this flag.
... in a compatibility-breaking release.
http://gerrit.cloudera.org:8080/#/c/5828/4/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java:
Line 125: // Counters that track the number of objects mapping to an unknown disk ID. Here, we use
How about:
Number of scan-ranges/files/partitions that have missing disk ids. Reported in the explain plan.
Line 129: private int numFilesNoDiskIds_ = 0;
remove blank lines (helps wit visual clustering)
Line 355: LOG.trace("Hdfs block missing disk ID information: " + block.toString());
I don't think this prints the full file path right? I think printing the file would be sufficient, no need to go to the block level.
http://gerrit.cloudera.org:8080/#/c/5828/4/fe/src/main/java/org/apache/impala/planner/Planner.java
File fe/src/main/java/org/apache/impala/planner/Planner.java:
Line 313: "disk Id information.\n" + Joiner.on(", ").join(tableNames) + "\n");
disk id
http://gerrit.cloudera.org:8080/#/c/5828/4/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:
Line 953: queryCtx.addToTables_missing_stats(
The old code with TreeSet was removing duplicate table names, are you still eliminating duplicates?
--
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: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] Improvements to "Unknown disk-ID" warning
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change.
Change subject: Improvements to "Unknown disk-ID" warning
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/5828/1//COMMIT_MSG
Commit Message:
Line 7: Improvements to "Unknown disk-ID" warning
IMPALA-1427?
Line 9: This commit,
This patch seems fine.
Another idea to consider:
I'm not really sure how useful it is to return these missing disk id warnings from scans. An alternative might be to report the number of missing disk ids in the explain plan of a query and throw away this runtime checking and warning through the existing path.
--
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: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1427: Improvements to "Unknown disk-ID" warning
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Hello Alex Behm,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/5828
to look at the new patch set (#7).
Change subject: IMPALA-1427: Improvements to "Unknown disk-ID" warning
......................................................................
IMPALA-1427: Improvements to "Unknown disk-ID" warning
- Removes the runtime unknown disk ID reporting and instead moves
it to the explain plan as a counter that prints the number of
scan ranges missing disk IDs in the corresponding HDFS scan nodes.
- Adds a warning to the header of query profile/explain plan with a
list of tables missing disk ids.
- Removes reference to enabling dfs block metadata configuration,
since it doesn't apply anymore.
- Removes VolumeId terminology from the runtime profile.
Change-Id: Iddb132ff7ad66f3291b93bf9d8061bd0525ef1b2
---
M be/src/exec/hdfs-scan-node-base.cc
M be/src/exec/hdfs-scan-node-base.h
M be/src/service/query-exec-state.cc
M common/thrift/ImpalaInternalService.thrift
M common/thrift/generate_error_codes.py
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
8 files changed, 166 insertions(+), 102 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/28/5828/7
--
To view, visit http://gerrit.cloudera.org:8080/5828
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iddb132ff7ad66f3291b93bf9d8061bd0525ef1b2
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
[Impala-ASF-CR] IMPALA-1427: Improvements to "Unknown disk-ID" warning
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: IMPALA-1427: Improvements to "Unknown disk-ID" warning
......................................................................
Patch Set 8:
Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/245/
--
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: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-1427: Improvements to "Unknown disk-ID" warning
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: IMPALA-1427: Improvements to "Unknown disk-ID" warning
......................................................................
Patch Set 8: Verified-1
Build failed: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/243/
--
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: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-HasComments: No