You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by GitBox <gi...@apache.org> on 2020/12/10 03:40:39 UTC
[GitHub] [hudi] danny0405 opened a new pull request #2319: [MINOR] Improve to only compute fileComparisonsRDD once when 'hoodie.…
danny0405 opened a new pull request #2319:
URL: https://github.com/apache/hudi/pull/2319
…bloom.index.prune.by.ranges' is set to true
## What is the purpose of the pull request
The option 'hoodie.bloom.index.prune.by.ranges' is default true, so it
expects to be an improvement.
## Brief change log
- Some refactoring to `SparkHoodieBloomIndex`
## Verify this pull request
Existing tests
## Committer checklist
- [ ] Has a corresponding JIRA in PR title & commit
- [ ] Commit message is descriptive of the change
- [ ] CI is green
- [ ] Necessary doc changes done or have another open PR
- [ ] For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] danny0405 commented on pull request #2319: [MINOR] Improve to only compute fileComparisonsRDD once when 'hoodie.…
Posted by GitBox <gi...@apache.org>.
danny0405 commented on pull request #2319:
URL: https://github.com/apache/hudi/pull/2319#issuecomment-743084273
> The other usage, where we actually do the range pruning, we cannot do sampling
Thanks, got your idea ~
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] vinothchandar commented on a change in pull request #2319: [MINOR] Improve to only compute fileComparisonsRDD once when 'hoodie.…
Posted by GitBox <gi...@apache.org>.
vinothchandar commented on a change in pull request #2319:
URL: https://github.com/apache/hudi/pull/2319#discussion_r541059766
##########
File path: hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/index/bloom/SparkHoodieBloomIndex.java
##########
@@ -122,13 +122,15 @@ public SparkHoodieBloomIndex(HoodieWriteConfig config) {
// Step 3: Obtain a RDD, for each incoming record, that already exists, with the file id,
// that contains it.
+ JavaRDD<Tuple2<String, HoodieKey>> fileComparisonsRDD =
Review comment:
Correct. That's what i think will happen
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] danny0405 commented on a change in pull request #2319: [MINOR] Improve to only compute fileComparisonsRDD once when 'hoodie.…
Posted by GitBox <gi...@apache.org>.
danny0405 commented on a change in pull request #2319:
URL: https://github.com/apache/hudi/pull/2319#discussion_r540751433
##########
File path: hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/index/bloom/SparkHoodieBloomIndex.java
##########
@@ -252,11 +252,10 @@ public boolean isImplicitWithStorage() {
* Make sure the parallelism is atleast the groupby parallelism for tagging location
*/
JavaPairRDD<HoodieKey, HoodieRecordLocation> findMatchingFilesForRecordKeys(
- final Map<String, List<BloomIndexFileInfo>> partitionToFileIndexInfo,
- JavaPairRDD<String, String> partitionRecordKeyPairRDD, int shuffleParallelism, HoodieTable hoodieTable,
+ JavaRDD<Tuple2<String, HoodieKey>> fileComparisonsRDD,
+ int shuffleParallelism,
+ HoodieTable hoodieTable,
Review comment:
Thanks, i just notice that Hoodie use the whitespace for indentation.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] vinothchandar commented on a change in pull request #2319: [MINOR] Improve to only compute fileComparisonsRDD once when 'hoodie.…
Posted by GitBox <gi...@apache.org>.
vinothchandar commented on a change in pull request #2319:
URL: https://github.com/apache/hudi/pull/2319#discussion_r540767320
##########
File path: hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/index/bloom/SparkHoodieBloomIndex.java
##########
@@ -122,13 +122,15 @@ public SparkHoodieBloomIndex(HoodieWriteConfig config) {
// Step 3: Obtain a RDD, for each incoming record, that already exists, with the file id,
// that contains it.
+ JavaRDD<Tuple2<String, HoodieKey>> fileComparisonsRDD =
Review comment:
So Spark does lazy evaluation of an RDD. If the RDD is not persisted to disk/cache, it simply recomputes it. In this case, `fileComparisonsRDD` would be recomputed twice during runtime. the method `explodeRecordRDDWithFileComparisons()` will only be called once, but it does nothing in practice except "define" the RDD it returns.
In contrast, if you notice this line of code at the start of `tagLocation`
```
// Step 0: cache the input record RDD
if (config.getBloomIndexUseCaching()) {
recordRDD.persist(SparkMemoryUtils.getBloomIndexInputStorageLevel(config.getProps()));
}
```
This caches the incoming recordRDD and thus however many times this RDD is used in the indexing DAG, it will not go to the previous stage. if we did not have the .persist() in here, then everytime an RDD derived off this `recordRDD` is needed for a Spark action, it will keep reading from source and compute the recordRDD again.
Apologies, if you knew all this already. :) and I am failing to see how this wont happen. but at-least you get my concern with this explanation.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] danny0405 commented on pull request #2319: [MINOR] Improve to only compute fileComparisonsRDD once when 'hoodie.…
Posted by GitBox <gi...@apache.org>.
danny0405 commented on pull request #2319:
URL: https://github.com/apache/hudi/pull/2319#issuecomment-743034840
> Thanks for the PR @danny0405 !
>
> This leaves the code easier to read. but unsure of the intended effects. I have actually thought more about how to avoid this. and it seems hard. we do need some evaluation of how much pruning is going to happen for us to size the parallelism and since that is very workload specific, it does need an evaluation.
>
> best idea I could come up with, was to see if we can do some sampling say 10% of the records to estimate.
>
> Something like below.
>
> ```
> / we will just try exploding the input and then count to determine comparisons
> // FIX(vc): Only do sampling here and extrapolate?
> fileToComparisons = explodeRecordRDDWithFileComparisons(partitionToFileInfo, partitionRecordKeyPairRDD.sample(0.1))
> .mapToPair(t -> t).countByKey();
> ```
Thanks so much for the review @vinothchandar ~ Can we really do a sampling here ? Then how can we ensure the `FileId -> HoodieKey` candidates are complete so that the final built Index is accurate. (e.g. No `UPDATE` records are tagged as `INSERT`).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] vinothchandar commented on pull request #2319: [MINOR] Improve to only compute fileComparisonsRDD once when 'hoodie.…
Posted by GitBox <gi...@apache.org>.
vinothchandar commented on pull request #2319:
URL: https://github.com/apache/hudi/pull/2319#issuecomment-743048327
>Can we really do a sampling here ?
yes, there is a trade-off here. We have to rely on the fact that the previous stage does a `reduceByKey` so the data would be distributed evenly across the RDD partitions :). So, extrapolation could be okay, for purposes of `computeComparisonsPerFileGroup()`. The other usage, where we actually do the range pruning, we cannot do sampling.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] codecov-io commented on pull request #2319: [MINOR] Improve to only compute fileComparisonsRDD once when 'hoodie.…
Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #2319:
URL: https://github.com/apache/hudi/pull/2319#issuecomment-743041298
# [Codecov](https://codecov.io/gh/apache/hudi/pull/2319?src=pr&el=h1) Report
> Merging [#2319](https://codecov.io/gh/apache/hudi/pull/2319?src=pr&el=desc) (861fbe0) into [master](https://codecov.io/gh/apache/hudi/commit/bd9cceccb582ede88b989824241498e8c32d4f13?el=desc) (bd9ccec) will **decrease** coverage by `1.09%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/hudi/pull/2319?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #2319 +/- ##
============================================
- Coverage 52.29% 51.20% -1.10%
+ Complexity 2630 2589 -41
============================================
Files 329 329
Lines 14743 14808 +65
Branches 1484 1484
============================================
- Hits 7710 7582 -128
- Misses 6419 6624 +205
+ Partials 614 602 -12
```
| Flag | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| hudicli | `38.83% <ø> (+0.32%)` | `0.00 <ø> (ø)` | |
| hudiclient | `100.00% <ø> (ø)` | `0.00 <ø> (ø)` | |
| hudicommon | `55.09% <ø> (-0.01%)` | `0.00 <ø> (ø)` | |
| hudihadoopmr | `33.29% <ø> (+0.35%)` | `0.00 <ø> (ø)` | |
| huditimelineservice | `65.30% <ø> (ø)` | `0.00 <ø> (ø)` | |
| hudiutilities | `60.49% <ø> (-9.63%)` | `0.00 <ø> (ø)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/hudi/pull/2319?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...ies/exception/HoodieSnapshotExporterException.java](https://codecov.io/gh/apache/hudi/pull/2319/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL2V4Y2VwdGlvbi9Ib29kaWVTbmFwc2hvdEV4cG9ydGVyRXhjZXB0aW9uLmphdmE=) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (-1.00%)` | |
| [.../apache/hudi/utilities/HoodieSnapshotExporter.java](https://codecov.io/gh/apache/hudi/pull/2319/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL0hvb2RpZVNuYXBzaG90RXhwb3J0ZXIuamF2YQ==) | `5.21% <0.00%> (-83.48%)` | `0.00% <0.00%> (-28.00%)` | |
| [...hudi/utilities/schema/JdbcbasedSchemaProvider.java](https://codecov.io/gh/apache/hudi/pull/2319/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3NjaGVtYS9KZGJjYmFzZWRTY2hlbWFQcm92aWRlci5qYXZh) | `0.00% <0.00%> (-72.23%)` | `0.00% <0.00%> (-2.00%)` | |
| [...he/hudi/utilities/transform/AWSDmsTransformer.java](https://codecov.io/gh/apache/hudi/pull/2319/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3RyYW5zZm9ybS9BV1NEbXNUcmFuc2Zvcm1lci5qYXZh) | `0.00% <0.00%> (-66.67%)` | `0.00% <0.00%> (-2.00%)` | |
| [...in/java/org/apache/hudi/utilities/UtilHelpers.java](https://codecov.io/gh/apache/hudi/pull/2319/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL1V0aWxIZWxwZXJzLmphdmE=) | `30.12% <0.00%> (-34.34%)` | `23.00% <0.00%> (-9.00%)` | |
| [...mmon/table/log/HoodieUnMergedLogRecordScanner.java](https://codecov.io/gh/apache/hudi/pull/2319/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL3RhYmxlL2xvZy9Ib29kaWVVbk1lcmdlZExvZ1JlY29yZFNjYW5uZXIuamF2YQ==) | `0.00% <0.00%> (ø)` | `0.00% <0.00%> (ø%)` | |
| [...mmon/table/log/AbstractHoodieLogRecordScanner.java](https://codecov.io/gh/apache/hudi/pull/2319/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL3RhYmxlL2xvZy9BYnN0cmFjdEhvb2RpZUxvZ1JlY29yZFNjYW5uZXIuamF2YQ==) | `79.84% <0.00%> (+0.15%)` | `29.00% <0.00%> (ø%)` | |
| [.../hadoop/realtime/RealtimeUnmergedRecordReader.java](https://codecov.io/gh/apache/hudi/pull/2319/diff?src=pr&el=tree#diff-aHVkaS1oYWRvb3AtbXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvaGFkb29wL3JlYWx0aW1lL1JlYWx0aW1lVW5tZXJnZWRSZWNvcmRSZWFkZXIuamF2YQ==) | `97.56% <0.00%> (+0.50%)` | `11.00% <0.00%> (ø%)` | |
| [...hadoop/realtime/RealtimeCompactedRecordReader.java](https://codecov.io/gh/apache/hudi/pull/2319/diff?src=pr&el=tree#diff-aHVkaS1oYWRvb3AtbXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvaGFkb29wL3JlYWx0aW1lL1JlYWx0aW1lQ29tcGFjdGVkUmVjb3JkUmVhZGVyLmphdmE=) | `72.72% <0.00%> (+1.02%)` | `12.00% <0.00%> (ø%)` | |
| [...apache/hudi/cli/commands/HoodieLogFileCommand.java](https://codecov.io/gh/apache/hudi/pull/2319/diff?src=pr&el=tree#diff-aHVkaS1jbGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY2xpL2NvbW1hbmRzL0hvb2RpZUxvZ0ZpbGVDb21tYW5kLmphdmE=) | `83.20% <0.00%> (+1.62%)` | `17.00% <0.00%> (ø%)` | |
| ... and [1 more](https://codecov.io/gh/apache/hudi/pull/2319/diff?src=pr&el=tree-more) | |
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] danny0405 commented on a change in pull request #2319: [MINOR] Improve to only compute fileComparisonsRDD once when 'hoodie.…
Posted by GitBox <gi...@apache.org>.
danny0405 commented on a change in pull request #2319:
URL: https://github.com/apache/hudi/pull/2319#discussion_r540808224
##########
File path: hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/index/bloom/SparkHoodieBloomIndex.java
##########
@@ -122,13 +122,15 @@ public SparkHoodieBloomIndex(HoodieWriteConfig config) {
// Step 3: Obtain a RDD, for each incoming record, that already exists, with the file id,
// that contains it.
+ JavaRDD<Tuple2<String, HoodieKey>> fileComparisonsRDD =
Review comment:
Thanks for the explanation, the `recordRDD` may be persisted, but the computation in `explodeRecordRDDWithFileComparisons` still need to do 2 times, right ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] garyli1019 commented on a change in pull request #2319: [MINOR] Improve to only compute fileComparisonsRDD once when 'hoodie.…
Posted by GitBox <gi...@apache.org>.
garyli1019 commented on a change in pull request #2319:
URL: https://github.com/apache/hudi/pull/2319#discussion_r540166539
##########
File path: hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/index/bloom/SparkHoodieBloomIndex.java
##########
@@ -252,11 +252,9 @@ public boolean isImplicitWithStorage() {
* Make sure the parallelism is atleast the groupby parallelism for tagging location
*/
JavaPairRDD<HoodieKey, HoodieRecordLocation> findMatchingFilesForRecordKeys(
- final Map<String, List<BloomIndexFileInfo>> partitionToFileIndexInfo,
- JavaPairRDD<String, String> partitionRecordKeyPairRDD, int shuffleParallelism, HoodieTable hoodieTable,
+ JavaRDD<Tuple2<String, HoodieKey>> fileComparisonsRDD,
Review comment:
@shenh062326 FYI, related to your Tuple2 refactoring PR 😉 .
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] yanghua commented on a change in pull request #2319: [MINOR] Improve to only compute fileComparisonsRDD once when 'hoodie.…
Posted by GitBox <gi...@apache.org>.
yanghua commented on a change in pull request #2319:
URL: https://github.com/apache/hudi/pull/2319#discussion_r540224339
##########
File path: hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/index/bloom/SparkHoodieBloomIndex.java
##########
@@ -252,11 +252,9 @@ public boolean isImplicitWithStorage() {
* Make sure the parallelism is atleast the groupby parallelism for tagging location
*/
JavaPairRDD<HoodieKey, HoodieRecordLocation> findMatchingFilesForRecordKeys(
- final Map<String, List<BloomIndexFileInfo>> partitionToFileIndexInfo,
- JavaPairRDD<String, String> partitionRecordKeyPairRDD, int shuffleParallelism, HoodieTable hoodieTable,
+ JavaRDD<Tuple2<String, HoodieKey>> fileComparisonsRDD,
+ int shuffleParallelism, HoodieTable hoodieTable,
Review comment:
Maybe we can move `fileGroupToComparisons ` into this line or break the `hoodieTable ` to the next line. It would be better?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] vinothchandar commented on a change in pull request #2319: [MINOR] Improve to only compute fileComparisonsRDD once when 'hoodie.…
Posted by GitBox <gi...@apache.org>.
vinothchandar commented on a change in pull request #2319:
URL: https://github.com/apache/hudi/pull/2319#discussion_r540738854
##########
File path: hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/index/bloom/SparkHoodieBloomIndex.java
##########
@@ -122,13 +122,15 @@ public SparkHoodieBloomIndex(HoodieWriteConfig config) {
// Step 3: Obtain a RDD, for each incoming record, that already exists, with the file id,
// that contains it.
+ JavaRDD<Tuple2<String, HoodieKey>> fileComparisonsRDD =
Review comment:
do you actually see from the Spark UI that its not computed twice? I ask because, `fileComparisonsRDD` is not cached and thus even though this is declared only once, during runtime, Spark will lazily recompute `fileComparisonsRDD` once for each method that uses it.
##########
File path: hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/index/bloom/SparkHoodieBloomIndex.java
##########
@@ -252,11 +252,10 @@ public boolean isImplicitWithStorage() {
* Make sure the parallelism is atleast the groupby parallelism for tagging location
*/
JavaPairRDD<HoodieKey, HoodieRecordLocation> findMatchingFilesForRecordKeys(
- final Map<String, List<BloomIndexFileInfo>> partitionToFileIndexInfo,
- JavaPairRDD<String, String> partitionRecordKeyPairRDD, int shuffleParallelism, HoodieTable hoodieTable,
+ JavaRDD<Tuple2<String, HoodieKey>> fileComparisonsRDD,
+ int shuffleParallelism,
+ HoodieTable hoodieTable,
Review comment:
nit: wondering how checkstyle is happy with the indentation here. :)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] danny0405 commented on a change in pull request #2319: [MINOR] Improve to only compute fileComparisonsRDD once when 'hoodie.…
Posted by GitBox <gi...@apache.org>.
danny0405 commented on a change in pull request #2319:
URL: https://github.com/apache/hudi/pull/2319#discussion_r540808224
##########
File path: hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/index/bloom/SparkHoodieBloomIndex.java
##########
@@ -122,13 +122,15 @@ public SparkHoodieBloomIndex(HoodieWriteConfig config) {
// Step 3: Obtain a RDD, for each incoming record, that already exists, with the file id,
// that contains it.
+ JavaRDD<Tuple2<String, HoodieKey>> fileComparisonsRDD =
Review comment:
Thanks for the explanation, the `recordRDD` may be persisted, but the computation in `explodeRecordRDDWithFileComparisons` still need to do 2 times, right ? Sorry, i'm not that familiar with the `RDD` thing.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] codecov-io edited a comment on pull request #2319: [MINOR] Improve to only compute fileComparisonsRDD once when 'hoodie.…
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #2319:
URL: https://github.com/apache/hudi/pull/2319#issuecomment-743041298
# [Codecov](https://codecov.io/gh/apache/hudi/pull/2319?src=pr&el=h1) Report
> Merging [#2319](https://codecov.io/gh/apache/hudi/pull/2319?src=pr&el=desc) (622c3c7) into [master](https://codecov.io/gh/apache/hudi/commit/bd9cceccb582ede88b989824241498e8c32d4f13?el=desc) (bd9ccec) will **decrease** coverage by `41.88%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/hudi/pull/2319?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #2319 +/- ##
=============================================
- Coverage 52.29% 10.40% -41.89%
+ Complexity 2630 48 -2582
=============================================
Files 329 51 -278
Lines 14743 1787 -12956
Branches 1484 213 -1271
=============================================
- Hits 7710 186 -7524
+ Misses 6419 1588 -4831
+ Partials 614 13 -601
```
| Flag | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| hudicli | `?` | `?` | |
| hudiclient | `?` | `?` | |
| hudicommon | `?` | `?` | |
| hudihadoopmr | `?` | `?` | |
| huditimelineservice | `?` | `?` | |
| hudiutilities | `10.40% <ø> (-59.71%)` | `0.00 <ø> (ø)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/hudi/pull/2319?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...va/org/apache/hudi/utilities/IdentitySplitter.java](https://codecov.io/gh/apache/hudi/pull/2319/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL0lkZW50aXR5U3BsaXR0ZXIuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (-2.00%)` | |
| [...va/org/apache/hudi/utilities/schema/SchemaSet.java](https://codecov.io/gh/apache/hudi/pull/2319/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3NjaGVtYS9TY2hlbWFTZXQuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (-3.00%)` | |
| [...a/org/apache/hudi/utilities/sources/RowSource.java](https://codecov.io/gh/apache/hudi/pull/2319/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3NvdXJjZXMvUm93U291cmNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (-4.00%)` | |
| [.../org/apache/hudi/utilities/sources/AvroSource.java](https://codecov.io/gh/apache/hudi/pull/2319/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3NvdXJjZXMvQXZyb1NvdXJjZS5qYXZh) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (-1.00%)` | |
| [.../org/apache/hudi/utilities/sources/JsonSource.java](https://codecov.io/gh/apache/hudi/pull/2319/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3NvdXJjZXMvSnNvblNvdXJjZS5qYXZh) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (-1.00%)` | |
| [...rg/apache/hudi/utilities/sources/CsvDFSSource.java](https://codecov.io/gh/apache/hudi/pull/2319/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3NvdXJjZXMvQ3N2REZTU291cmNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (-10.00%)` | |
| [...g/apache/hudi/utilities/sources/JsonDFSSource.java](https://codecov.io/gh/apache/hudi/pull/2319/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3NvdXJjZXMvSnNvbkRGU1NvdXJjZS5qYXZh) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (-4.00%)` | |
| [...apache/hudi/utilities/sources/JsonKafkaSource.java](https://codecov.io/gh/apache/hudi/pull/2319/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3NvdXJjZXMvSnNvbkthZmthU291cmNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (-6.00%)` | |
| [...pache/hudi/utilities/sources/ParquetDFSSource.java](https://codecov.io/gh/apache/hudi/pull/2319/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3NvdXJjZXMvUGFycXVldERGU1NvdXJjZS5qYXZh) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (-5.00%)` | |
| [...lities/schema/SchemaProviderWithPostProcessor.java](https://codecov.io/gh/apache/hudi/pull/2319/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3NjaGVtYS9TY2hlbWFQcm92aWRlcldpdGhQb3N0UHJvY2Vzc29yLmphdmE=) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (-4.00%)` | |
| ... and [306 more](https://codecov.io/gh/apache/hudi/pull/2319/diff?src=pr&el=tree-more) | |
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] danny0405 commented on a change in pull request #2319: [MINOR] Improve to only compute fileComparisonsRDD once when 'hoodie.…
Posted by GitBox <gi...@apache.org>.
danny0405 commented on a change in pull request #2319:
URL: https://github.com/apache/hudi/pull/2319#discussion_r540751159
##########
File path: hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/index/bloom/SparkHoodieBloomIndex.java
##########
@@ -122,13 +122,15 @@ public SparkHoodieBloomIndex(HoodieWriteConfig config) {
// Step 3: Obtain a RDD, for each incoming record, that already exists, with the file id,
// that contains it.
+ JavaRDD<Tuple2<String, HoodieKey>> fileComparisonsRDD =
Review comment:
I didn't check the Spark UI yet, just a simple analyze the process of data writing. For each batch of records to write, the `SparkHoodieBloomIndex.lookupIndex` was expected to be invoked once so the `fileComparisonsRDD` should only be evaluated only once, is there other invocation for `SparkHoodieBloomIndex.lookupIndex` ? Maybe i missed something.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] vinothchandar commented on pull request #2319: [MINOR] Improve to only compute fileComparisonsRDD once when 'hoodie.…
Posted by GitBox <gi...@apache.org>.
vinothchandar commented on pull request #2319:
URL: https://github.com/apache/hudi/pull/2319#issuecomment-743286022
@danny0405 I ll take another pass and merge this PR. This should not have any side effects so should qualify for a MINOR label.
IIRC there are some JIRAs with the index/performance component around some followup ideas we discussed here
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] shenh062326 commented on a change in pull request #2319: [MINOR] Improve to only compute fileComparisonsRDD once when 'hoodie.…
Posted by GitBox <gi...@apache.org>.
shenh062326 commented on a change in pull request #2319:
URL: https://github.com/apache/hudi/pull/2319#discussion_r540663418
##########
File path: hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/index/bloom/SparkHoodieBloomIndex.java
##########
@@ -252,11 +252,9 @@ public boolean isImplicitWithStorage() {
* Make sure the parallelism is atleast the groupby parallelism for tagging location
*/
JavaPairRDD<HoodieKey, HoodieRecordLocation> findMatchingFilesForRecordKeys(
- final Map<String, List<BloomIndexFileInfo>> partitionToFileIndexInfo,
- JavaPairRDD<String, String> partitionRecordKeyPairRDD, int shuffleParallelism, HoodieTable hoodieTable,
+ JavaRDD<Tuple2<String, HoodieKey>> fileComparisonsRDD,
Review comment:
I am removing the scala dependency from hudl-client-common, it seems no relationship to with this PR, since it depends on scala.Tuple2 in hudi-spark-client.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] vinothchandar merged pull request #2319: [MINOR] Improve code readability by passing in the fileComparisonsRDD in bloom index
Posted by GitBox <gi...@apache.org>.
vinothchandar merged pull request #2319:
URL: https://github.com/apache/hudi/pull/2319
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] danny0405 commented on a change in pull request #2319: [MINOR] Improve to only compute fileComparisonsRDD once when 'hoodie.…
Posted by GitBox <gi...@apache.org>.
danny0405 commented on a change in pull request #2319:
URL: https://github.com/apache/hudi/pull/2319#discussion_r540644075
##########
File path: hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/index/bloom/SparkHoodieBloomIndex.java
##########
@@ -252,11 +252,9 @@ public boolean isImplicitWithStorage() {
* Make sure the parallelism is atleast the groupby parallelism for tagging location
*/
JavaPairRDD<HoodieKey, HoodieRecordLocation> findMatchingFilesForRecordKeys(
- final Map<String, List<BloomIndexFileInfo>> partitionToFileIndexInfo,
- JavaPairRDD<String, String> partitionRecordKeyPairRDD, int shuffleParallelism, HoodieTable hoodieTable,
+ JavaRDD<Tuple2<String, HoodieKey>> fileComparisonsRDD,
+ int shuffleParallelism, HoodieTable hoodieTable,
Review comment:
Thanks, agree ~
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] nsivabalan commented on pull request #2319: [MINOR] Improve to only compute fileComparisonsRDD once when 'hoodie.…
Posted by GitBox <gi...@apache.org>.
nsivabalan commented on pull request #2319:
URL: https://github.com/apache/hudi/pull/2319#issuecomment-743289804
yeah, [same](https://github.com/apache/hudi/pull/1721) was already brought up before and we didn't proceed since it needed some perf analysis.
this was the rational:
even though explodeRecordRDDWithFileComparisons(...) is called in two places, in the first place we just do count by key which may not shuffle any actual data, where as the 2nd call could incur shuffling. Hence the pattern of usage differs. More info can be found in the attached PR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] vinothchandar commented on pull request #2319: [MINOR] Improve to only compute fileComparisonsRDD once when 'hoodie.…
Posted by GitBox <gi...@apache.org>.
vinothchandar commented on pull request #2319:
URL: https://github.com/apache/hudi/pull/2319#issuecomment-745085205
Merging.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org