KYLIN-3926 set sourceRecordCount when updating statistics #579

kyotoYaho · 2019-04-03T10:25:57Z

No description provided.

asfgit · 2019-04-03T10:26:05Z

Can one of the admins verify this patch?

asfgit · 2019-04-03T10:26:06Z

Can one of the admins verify this patch?

coveralls · 2019-04-03T10:54:01Z

Pull Request Test Coverage Report for Build 4464

0 of 51 (0.0%) changed or added relevant lines in 6 files are covered.
2935 unchanged lines in 63 files lost coverage.
Overall coverage decreased (-0.01%) to 27.857%

Changes Missing Coverage	Changed/Added Lines	%
engine-mr/src/main/java/org/apache/kylin/engine/mr/common/CubeStatsReader.java	1	0.0%
engine-mr/src/main/java/org/apache/kylin/engine/mr/steps/MergeStatisticsWithOldStep.java	1	0.0%
engine-mr/src/main/java/org/apache/kylin/engine/mr/common/CubeStatsWriter.java	2	0.0%
engine-mr/src/main/java/org/apache/kylin/engine/mr/steps/MergeDictionaryMapper.java	13	0.0%
engine-mr/src/main/java/org/apache/kylin/engine/mr/steps/MergeStatisticsStep.java	17	0.0%
engine-spark/src/main/java/org/apache/kylin/engine/spark/SparkMergingDictionary.java	17	0.0%

Files with Coverage Reduction	New Missed Lines	%
source-jdbc/src/main/java/org/apache/kylin/source/jdbc/extensible/JdbcHiveInputBase.java	1	94.0%
engine-mr/src/main/java/org/apache/kylin/engine/mr/steps/MergeDictionaryMapper.java	1	0.0%
core-job/src/main/java/org/apache/kylin/job/dao/ExecutablePO.java	2	90.48%
core-cube/src/main/java/org/apache/kylin/cube/common/RowKeySplitter.java	2	82.22%
core-job/src/main/java/org/apache/kylin/engine/EngineFactory.java	3	0.0%
core-cube/src/main/java/org/apache/kylin/cube/inmemcubing/MemDiskStore.java	5	77.81%
core-metadata/src/main/java/org/apache/kylin/measure/bitmap/BitmapMeasureType.java	5	18.87%
core-job/src/main/java/org/apache/kylin/job/impl/threadpool/DefaultFetcherRunner.java	6	82.05%
server-base/src/main/java/org/apache/kylin/rest/controller/JobController.java	6	0.0%
core-job/src/main/java/org/apache/kylin/job/impl/threadpool/FetcherRunner.java	6	84.62%

Totals
Change from base Build 4342:	-0.01%
Covered Lines:	22802
Relevant Lines:	81855

💛 - Coveralls

codecov-io · 2019-04-03T10:58:08Z

Codecov Report

Merging #579 into master will decrease coverage by <.01%.
The diff coverage is 0%.

@@             Coverage Diff              @@
##             master     #579      +/-   ##
============================================
- Coverage     25.38%   25.38%   -0.01%     
- Complexity     5803     5864      +61     
============================================
  Files          1372     1379       +7     
  Lines         81137    81855     +718     
  Branches      11377    11475      +98     
============================================
+ Hits          20599    20777     +178     
- Misses        58525    59057     +532     
- Partials       2013     2021       +8

Impacted Files	Coverage Δ	Complexity Δ
...in/engine/mr/steps/MergeStatisticsWithOldStep.java	`0% <0%> (ø)`	`0 <0> (ø)`	⬇️
...apache/kylin/engine/mr/common/CubeStatsWriter.java	`58.62% <0%> (-4.35%)`	`3 <0> (ø)`
...apache/kylin/engine/mr/common/CubeStatsReader.java	`0% <0%> (ø)`	`0 <0> (ø)`	⬇️
...che/kylin/engine/mr/steps/MergeStatisticsStep.java	`0% <0%> (ø)`	`0 <0> (ø)`	⬇️
...che/kylin/engine/spark/SparkMergingDictionary.java	`0% <0%> (ø)`	`0 <0> (ø)`	⬇️
...e/kylin/engine/mr/steps/MergeDictionaryMapper.java	`0% <0%> (ø)`	`0 <0> (ø)`	⬇️
...va/org/apache/kylin/source/hive/HiveInputBase.java	`7.72% <0%> (-4.98%)`	`4% <0%> (ø)`
...g/apache/kylin/engine/mr/common/MapReduceUtil.java	`8.95% <0%> (-3.29%)`	`2% <0%> (ø)`
...che/kylin/common/util/SourceConfigurationUtil.java	`57.44% <0%> (-3.02%)`	`7% <0%> (ø)`
.../java/org/apache/kylin/common/util/DateFormat.java	`31.81% <0%> (-2.06%)`	`8% <0%> (ø)`
... and 68 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7a1f90d...e7a40dc. Read the comment docs.

nichunen · 2019-04-10T11:27:08Z

engine-mr/src/main/java/org/apache/kylin/engine/mr/common/CubeStatsWriter.java

@@ -43,6 +43,11 @@ public static void writeCuboidStatistics(Configuration conf, Path outputPath, //
        writeCuboidStatistics(conf, outputPath, cuboidHLLMap, samplingPercentage, 0, 0, 0);
    }

+    public static void writeCuboidStatistics(Configuration conf, Path outputPath, //
+            Map<Long, HLLCounter> cuboidHLLMap, int samplingPercentage, long sourceRecordCoun) throws IOException {
+        writeCuboidStatistics(conf, outputPath, cuboidHLLMap, samplingPercentage, 0, 0, sourceRecordCoun);


typo: should be sourceRecordCount

Just copied from the existing parameter name. However, it's better to change it from sourceRecordCoun to sourceRecordCount.

nichunen · 2019-04-10T12:06:06Z

engine-mr/src/main/java/org/apache/kylin/engine/mr/steps/MergeStatisticsStep.java

@@ -120,8 +129,11 @@ protected ExecuteResult doWork(ExecutableContext context) throws ExecuteExceptio
                        tempFile.delete();
                }
            }
+            sourceRecordCount *= effectiveTimeRange == 0 ? 0 : newSegment.getTSRange().duration() / effectiveTimeRange;


why not convert to double to calculate the accurate data?

It's not necessary to do so. Row count itself is an approximate one. And this data is set to long type at other place and we shouldn't break the compatibility.

For details, please check http://issues.apache.org/jira/browse/KYLIN-3453

I mean the division

nichunen · 2019-04-11T02:35:21Z

Should SparkMergingDictionary MergeDictionaryMapper also be changed?

nichunen · 2019-04-25T08:23:31Z

@kyotoYaho Hi, would you please update the pr?

nichunen

Fine to me

nichunen · 2019-04-10T14:26:23Z

engine-mr/src/main/java/org/apache/kylin/engine/mr/steps/MergeStatisticsStep.java

@@ -120,8 +129,11 @@ protected ExecuteResult doWork(ExecutableContext context) throws ExecuteExceptio
                        tempFile.delete();
                }
            }
+            sourceRecordCount *= effectiveTimeRange == 0 ? 0 : newSegment.getTSRange().duration() / effectiveTimeRange;


I mean the division

KYLIN-3926 set sourceRecordCount when updating statistics

2b9108b

nichunen requested changes Apr 10, 2019

View reviewed changes

shaofengshi mentioned this pull request May 4, 2019

#3926, set sourceRecordCount when updating statistics #626

Closed

KYLIN-3926 Code review

e7a40dc

nichunen approved these changes May 7, 2019

View reviewed changes

nichunen merged commit ab124ac into apache:master May 7, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

KYLIN-3926 set sourceRecordCount when updating statistics #579

KYLIN-3926 set sourceRecordCount when updating statistics #579

kyotoYaho commented Apr 3, 2019

asfgit commented Apr 3, 2019

asfgit commented Apr 3, 2019

coveralls commented Apr 3, 2019 •

edited

Loading

codecov-io commented Apr 3, 2019 •

edited

Loading

nichunen Apr 10, 2019

kyotoYaho Apr 10, 2019

nichunen Apr 10, 2019

kyotoYaho Apr 10, 2019

kyotoYaho Apr 10, 2019

nichunen Apr 10, 2019

nichunen commented Apr 11, 2019

nichunen commented Apr 25, 2019

nichunen left a comment

nichunen Apr 10, 2019

KYLIN-3926 set sourceRecordCount when updating statistics #579

KYLIN-3926 set sourceRecordCount when updating statistics #579

Conversation

kyotoYaho commented Apr 3, 2019

asfgit commented Apr 3, 2019

asfgit commented Apr 3, 2019

coveralls commented Apr 3, 2019 • edited Loading

Pull Request Test Coverage Report for Build 4464

💛 - Coveralls

codecov-io commented Apr 3, 2019 • edited Loading

Codecov Report

nichunen Apr 10, 2019

Choose a reason for hiding this comment

kyotoYaho Apr 10, 2019

Choose a reason for hiding this comment

nichunen Apr 10, 2019

Choose a reason for hiding this comment

kyotoYaho Apr 10, 2019

Choose a reason for hiding this comment

kyotoYaho Apr 10, 2019

Choose a reason for hiding this comment

nichunen Apr 10, 2019

Choose a reason for hiding this comment

nichunen commented Apr 11, 2019

nichunen commented Apr 25, 2019

nichunen left a comment

Choose a reason for hiding this comment

nichunen Apr 10, 2019

Choose a reason for hiding this comment

coveralls commented Apr 3, 2019 •

edited

Loading

codecov-io commented Apr 3, 2019 •

edited

Loading