You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by mehant <gi...@git.apache.org> on 2015/11/03 01:19:51 UTC
[GitHub] drill pull request: DRILL-3941: Add timing instrumentation in diff...
GitHub user mehant opened a pull request:
https://github.com/apache/drill/pull/230
DRILL-3941: Add timing instrumentation in different phases of partiti…
…on pruning
Add basic timing instrumentation while initializing FileSelection and reading the metadata cache file
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/mehant/drill DRILL-3941_1
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/drill/pull/230.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #230
----
commit 20c781b2350cbfd12e256d0826f4e99788372c77
Author: Mehant Baid <me...@gmail.com>
Date: 2015-10-23T21:01:09Z
DRILL-3941: Add timing instrumentation in different phases of partition pruning
Add basic timing instrumentation while initializing FileSelection and reading the metadata cache file
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-3941: Add timing instrumentation in diff...
Posted by mehant <gi...@git.apache.org>.
Github user mehant commented on a diff in the pull request:
https://github.com/apache/drill/pull/230#discussion_r43706628
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/Metadata.java ---
@@ -163,7 +163,9 @@ private ParquetTableMetadata_v1 getParquetTableMetadata(String path) throws IOEx
watch.start();
List<FileStatus> fileStatuses = getFileStatuses(fileStatus);
logger.info("Took {} ms to get file statuses", watch.elapsed(TimeUnit.MILLISECONDS));
- return getParquetTableMetadata(fileStatuses);
+ ParquetTableMetadata_v1 metadata_v1 = getParquetTableMetadata(fileStatuses);
+ logger.info("Took {} ms to read file metadata", watch.elapsed(TimeUnit.MILLISECONDS));
--- End diff --
I will modify this to exclude the getFileStatuses().
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-3941: Add timing instrumentation in diff...
Posted by mehant <gi...@git.apache.org>.
Github user mehant commented on a diff in the pull request:
https://github.com/apache/drill/pull/230#discussion_r43706882
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/partition/PruneScanRule.java ---
@@ -166,16 +174,28 @@ protected void doOnMatch(RelOptRuleCall call, DrillFilterRel filterRel, DrillPro
}
if (partitionColumnBitSet.isEmpty()) {
- logger.debug("No partition columns are projected from the scan..continue.");
+ logger.info("No partition columns are projected from the scan..continue. " +
+ "Total pruning elapsed time: {} ms", totalPruningTime.elapsed(TimeUnit.MILLISECONDS));
--- End diff --
I think this is fine, since we always print a message at the beginning of PruneScanRule, it makes sense to print message indicating when we terminated. Also the message before ("No partition columns...) indicates that we didn't perform actual pruning. My objective was to capture total time spent in PruneScanRule.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-3941: Add timing instrumentation in diff...
Posted by amansinha100 <gi...@git.apache.org>.
Github user amansinha100 commented on the pull request:
https://github.com/apache/drill/pull/230#issuecomment-153780284
+1. LGTM.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-3941: Add timing instrumentation in diff...
Posted by amansinha100 <gi...@git.apache.org>.
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/230#discussion_r43706195
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/Metadata.java ---
@@ -163,7 +163,9 @@ private ParquetTableMetadata_v1 getParquetTableMetadata(String path) throws IOEx
watch.start();
List<FileStatus> fileStatuses = getFileStatuses(fileStatus);
logger.info("Took {} ms to get file statuses", watch.elapsed(TimeUnit.MILLISECONDS));
- return getParquetTableMetadata(fileStatuses);
+ ParquetTableMetadata_v1 metadata_v1 = getParquetTableMetadata(fileStatuses);
+ logger.info("Took {} ms to read file metadata", watch.elapsed(TimeUnit.MILLISECONDS));
--- End diff --
Is this time supposed to be only for reading file metadata ? This is reporting the time inclusive of getFileStatuses().
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-3941: Add timing instrumentation in diff...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/drill/pull/230
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-3941: Add timing instrumentation in diff...
Posted by mehant <gi...@git.apache.org>.
Github user mehant commented on the pull request:
https://github.com/apache/drill/pull/230#issuecomment-153201984
@amansinha100 can you please review.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-3941: Add timing instrumentation in diff...
Posted by amansinha100 <gi...@git.apache.org>.
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/230#discussion_r43706257
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/partition/PruneScanRule.java ---
@@ -166,16 +174,28 @@ protected void doOnMatch(RelOptRuleCall call, DrillFilterRel filterRel, DrillPro
}
if (partitionColumnBitSet.isEmpty()) {
- logger.debug("No partition columns are projected from the scan..continue.");
+ logger.info("No partition columns are projected from the scan..continue. " +
+ "Total pruning elapsed time: {} ms", totalPruningTime.elapsed(TimeUnit.MILLISECONDS));
--- End diff --
When the pruning rule does early return due to non-applicability, the 'Total pruning elapsed time' message could be misleading since it did not actually do pruning.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---