Sailesh Mukil has posted comments on this change. Change subject: IMPALA-4764: Add Hedged read metrics ......................................................................
Patch Set 3: (2 comments) Thanks for the review, Lars. http://gerrit.cloudera.org:8080/#/c/6886/3//COMMIT_MSG Commit Message: Line 19: Testing: Not adding tests since it requires some custom hadoop > Can we add a custom cluster test, at least as a smoke test? So, the issue with adding tests for this is that, the configuration needed to be set is in the hadoop core-site.xml, which we cannot manipulate on a per test basis. It needs to be checked in permanently. Also, the configuration that I need to set has to be set at a unrealistic value to verify that the feature works under testing (for the sake of having quick tests). The configuration is namely: "dfs.client.hedged.read.threshold.millis" which needs to be set at 0ms, to quickly verify that all reads are hedged reads. We don't want to check in our core-site template with that value. http://gerrit.cloudera.org:8080/#/c/6886/3/be/src/util/impalad-metrics.cc File be/src/util/impalad-metrics.cc: Line 104: "impala-server.hedged-read-ops"; > Have you considered grouping them under a common prefix, like impala-server I would do it if the only FS we supported was HDFS. For example, if hedged reads is supported by some other FS soon, then we would need to rename this to be more generic again. I guess for the same reason why the other metrics aren't grouped under HDFS, eg: "impala-server.io-mgr.num-open-files", etc. -- To view, visit http://gerrit.cloudera.org:8080/6886 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id4a5d396abb3373d352ad2df8c2272db018114da Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Sailesh Mukil <[email protected]> Gerrit-Reviewer: Lars Volker <[email protected]> Gerrit-Reviewer: Sailesh Mukil <[email protected]> Gerrit-HasComments: Yes
