[Enhancement] combined count when loadfile in dpp #34787

MaxWk · 2023-11-10T08:52:57Z

Fixes #14292

What type of PR is this:

Does this PR entail a change in behavior?

Yes, this PR will result in a change in behavior.
No, this PR will not result in a change in behavior.

If yes, please specify the type of change:

Interface/UI changes: syntax, type conversion, expression evaluation, display information
Parameter changes: default values, similar parameters but with different default values
Policy changes: use new policy to replace old one, functionality automatically enabled
Feature removed
Miscellaneous: upgrade & downgrade compatibility, etc.

Checklist:

I have added test cases for my bug fix or my new feature
This pr needs user documentation (for new or modified features or behaviors)
- I have added documentation for my new feature or new function

Bugfix cherry-pick branch check:

CLAassistant · 2023-11-10T08:53:03Z

All committers have signed the CLA.

starrocks-cr · 2023-11-10T08:53:50Z

fe/spark-dpp/src/main/java/com/starrocks/load/loadv2/dpp/SparkDpp.java

@@ -1297,7 +1318,7 @@ private void writeDppResult(DppResult dppResult) throws Exception {
        URI uri = new URI(outputPath);
        Path filePath = new Path(resultFilePath);
        try (FileSystem fs = FileSystem.get(uri, serializableHadoopConf.value());
-                FSDataOutputStream outputStream = fs.create(filePath)) {
+             FSDataOutputStream outputStream = fs.create(filePath)) {
            Gson gson = new Gson();
            outputStream.write(gson.toJson(dppResult).getBytes());
            outputStream.write('\n');


The most risky bug in this code is:
There could be unexpected behavior when counting the number of rows with scannedRowsAcc.add(fileGroupDataframe.count()); due to a costly action being triggered on the Spark DataFrame within a transformation process. This line will force the evaluation of all transformations applied to the fileGroupDataframe up to this point, which can be highly inefficient if the DataFrame is large as it triggers a full scan of the data.

You can modify the code like this:

// Move the row count until after the transformations are complete and an action is required. // Thus, avoiding triggering multiple actions unnecessarily. // The exact spot to place the count action depends on the broader context of how the produced DataFrame is utilized downstream. // Ensure there's a valid action where the result of the dataframe needs to be materialized before counting the rows.

Note that without additional context on how fileGroupDataframe is used later on, I cannot provide an exact location in the code where .count() should be placed. It's important that you review the logic to ensure scannedRowsAcc.add(...) is only called at a point where evaluating the DataFrame is unavoidable or beneficial for subsequent operations (like caching or writing to disk).

fe/spark-dpp/src/main/java/com/starrocks/load/loadv2/etl/EtlJobConfig.java

Astralidea · 2023-11-15T01:40:04Z

fe/spark-dpp/src/main/java/com/starrocks/load/loadv2/dpp/SparkDpp.java

            } catch (Exception e) {
                LOG.warn("parse path failed:" + filePath);
                throw e;
            }
        }
+
+        if (fileGroup.fileFormat != null &&


nit:

Set<String> formats = new HashSet<>(Arrays.asList("orc", "parquet")); if (fileGroup.fileFormat != null && formats.contains(fileGroup.fileFormat.toLowerCase())) { scannedRowsAcc.add(fileGroupDataframe.count()); }

Signed-off-by: kevin wan <[email protected]>

sonarcloud · 2023-11-20T10:41:57Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

0.0% Coverage
0.0% Duplication

The version of Java (11.0.21) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.
Read more here

wanpengfei-git · 2023-11-21T03:55:49Z

@Mergifyio backport branch-3.2

wanpengfei-git · 2023-11-21T03:55:51Z

@Mergifyio backport branch-3.1

wanpengfei-git · 2023-11-21T03:55:53Z

@Mergifyio backport branch-3.0

wanpengfei-git · 2023-11-21T03:55:54Z

@Mergifyio backport branch-2.5

mergify · 2023-11-21T03:56:09Z

backport branch-3.2

✅ Backports have been created

#35458 [Enhancement] combined count when loadfile in dpp (backport #34787) has been created for branch branch-3.2

mergify · 2023-11-21T03:56:11Z

backport branch-3.1

✅ Backports have been created

#35459 [Enhancement] combined count when loadfile in dpp (backport #34787) has been created for branch branch-3.1

mergify · 2023-11-21T03:56:13Z

backport branch-3.0

✅ Backports have been created

#35460 [Enhancement] combined count when loadfile in dpp (backport #34787) has been created for branch branch-3.0

mergify · 2023-11-21T03:56:15Z

backport branch-2.5

✅ Backports have been created

#35461 [Enhancement] combined count when loadfile in dpp (backport #34787) has been created for branch branch-2.5

Signed-off-by: kevin wan <[email protected]> Co-authored-by: mingge <[email protected]> (cherry picked from commit 154ae93)

github-actions · 2023-11-21T03:58:07Z

[FE Incremental Coverage Report]

✅ pass : 0 / 0 (0%)

github-actions · 2023-11-21T04:15:39Z

[BE Incremental Coverage Report]

✅ pass : 0 / 0 (0%)

Signed-off-by: kevin wan <[email protected]> Co-authored-by: mingge <[email protected]> (cherry picked from commit 154ae93)

github-actions bot added the title needs [type] label Nov 10, 2023

mergify bot assigned MaxWk Nov 10, 2023

starrocks-cr bot reviewed Nov 10, 2023

View reviewed changes

MaxWk changed the title ~~combined count when loadfile~~ [Enhancement] combined count when loadfile in dpp Nov 10, 2023

github-actions bot added 3.2 3.1 3.0 2.5 and removed title needs [type] labels Nov 10, 2023

wyb reviewed Nov 13, 2023

View reviewed changes

fe/spark-dpp/src/main/java/com/starrocks/load/loadv2/etl/EtlJobConfig.java Outdated Show resolved Hide resolved

MaxWk force-pushed the combine-count-opt-in-dpp branch from f074eae to bfabd7a Compare November 13, 2023 05:29

MaxWk requested a review from wyb November 14, 2023 02:06

wyb approved these changes Nov 15, 2023

View reviewed changes

Astralidea reviewed Nov 15, 2023

View reviewed changes

Astralidea approved these changes Nov 15, 2023

View reviewed changes

Astralidea enabled auto-merge (squash) November 15, 2023 04:33

auto-merge was automatically disabled November 16, 2023 07:23
Head branch was pushed to by a user without write access

MaxWk force-pushed the combine-count-opt-in-dpp branch 2 times, most recently from 7f2b4f0 to 0f4ef16 Compare November 16, 2023 07:30

wyb approved these changes Nov 16, 2023

View reviewed changes

Astralidea approved these changes Nov 16, 2023

View reviewed changes

combined count when loadfile

5101e40

Signed-off-by: kevin wan <[email protected]>

MaxWk force-pushed the combine-count-opt-in-dpp branch from 0f4ef16 to 5101e40 Compare November 17, 2023 16:10

wyb enabled auto-merge (squash) November 20, 2023 10:36

wyb merged commit 154ae93 into StarRocks:main Nov 21, 2023
37 checks passed

github-actions bot removed the 3.2 label Nov 21, 2023

github-actions bot removed the 3.1 label Nov 21, 2023

github-actions bot removed the 3.0 label Nov 21, 2023

github-actions bot removed the 2.5 label Nov 21, 2023

mergify bot mentioned this pull request Nov 21, 2023

[Enhancement] combined count when loadfile in dpp (backport #34787) #35458

Merged

mergify bot pushed a commit that referenced this pull request Nov 21, 2023

[Enhancement] combined count when loadfile in dpp (#34787)

7c0e125

Signed-off-by: kevin wan <[email protected]> Co-authored-by: mingge <[email protected]> (cherry picked from commit 154ae93)

mergify bot pushed a commit that referenced this pull request Nov 21, 2023

[Enhancement] combined count when loadfile in dpp (#34787)

d5b37b6

Signed-off-by: kevin wan <[email protected]> Co-authored-by: mingge <[email protected]> (cherry picked from commit 154ae93)

mergify bot mentioned this pull request Nov 21, 2023

[Enhancement] combined count when loadfile in dpp (backport #34787) #35459

Merged

mergify bot pushed a commit that referenced this pull request Nov 21, 2023

[Enhancement] combined count when loadfile in dpp (#34787)

9876920

Signed-off-by: kevin wan <[email protected]> Co-authored-by: mingge <[email protected]> (cherry picked from commit 154ae93)

mergify bot mentioned this pull request Nov 21, 2023

[Enhancement] combined count when loadfile in dpp (backport #34787) #35460

Merged

mergify bot pushed a commit that referenced this pull request Nov 21, 2023

[Enhancement] combined count when loadfile in dpp (#34787)

834e8c4

Signed-off-by: kevin wan <[email protected]> Co-authored-by: mingge <[email protected]> (cherry picked from commit 154ae93)

mergify bot mentioned this pull request Nov 21, 2023

[Enhancement] combined count when loadfile in dpp (backport #34787) #35461

Merged

wanpengfei-git pushed a commit that referenced this pull request Nov 21, 2023

[Enhancement] combined count when loadfile in dpp (#34787)

2239626

Signed-off-by: kevin wan <[email protected]> Co-authored-by: mingge <[email protected]> (cherry picked from commit 154ae93)

wanpengfei-git pushed a commit that referenced this pull request Nov 21, 2023

[Enhancement] combined count when loadfile in dpp (#34787)

011db37

Signed-off-by: kevin wan <[email protected]> Co-authored-by: mingge <[email protected]> (cherry picked from commit 154ae93)

wanpengfei-git pushed a commit that referenced this pull request Nov 21, 2023

[Enhancement] combined count when loadfile in dpp (#34787)

33d4832

Signed-off-by: kevin wan <[email protected]> Co-authored-by: mingge <[email protected]> (cherry picked from commit 154ae93)

wanpengfei-git pushed a commit that referenced this pull request Nov 21, 2023

[Enhancement] combined count when loadfile in dpp (#34787)

7878a00

Signed-off-by: kevin wan <[email protected]> Co-authored-by: mingge <[email protected]> (cherry picked from commit 154ae93)

wanpengfei-git added the backport_ok label Nov 23, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Enhancement] combined count when loadfile in dpp #34787

[Enhancement] combined count when loadfile in dpp #34787

MaxWk commented Nov 10, 2023 •

edited by wyb

Loading

CLAassistant commented Nov 10, 2023 •

edited

Loading

starrocks-cr bot Nov 10, 2023

Astralidea Nov 15, 2023

sonarcloud bot commented Nov 20, 2023

wanpengfei-git commented Nov 21, 2023

wanpengfei-git commented Nov 21, 2023

wanpengfei-git commented Nov 21, 2023

wanpengfei-git commented Nov 21, 2023

mergify bot commented Nov 21, 2023 •

edited

Loading

mergify bot commented Nov 21, 2023 •

edited

Loading

mergify bot commented Nov 21, 2023 •

edited

Loading

mergify bot commented Nov 21, 2023 •

edited

Loading

github-actions bot commented Nov 21, 2023

github-actions bot commented Nov 21, 2023

[Enhancement] combined count when loadfile in dpp #34787

[Enhancement] combined count when loadfile in dpp #34787

Conversation

MaxWk commented Nov 10, 2023 • edited by wyb Loading

What type of PR is this:

Checklist:

Bugfix cherry-pick branch check:

CLAassistant commented Nov 10, 2023 • edited Loading

starrocks-cr bot Nov 10, 2023

Choose a reason for hiding this comment

Astralidea Nov 15, 2023

Choose a reason for hiding this comment

sonarcloud bot commented Nov 20, 2023

wanpengfei-git commented Nov 21, 2023

wanpengfei-git commented Nov 21, 2023

wanpengfei-git commented Nov 21, 2023

wanpengfei-git commented Nov 21, 2023

mergify bot commented Nov 21, 2023 • edited Loading

✅ Backports have been created

mergify bot commented Nov 21, 2023 • edited Loading

✅ Backports have been created

mergify bot commented Nov 21, 2023 • edited Loading

✅ Backports have been created

mergify bot commented Nov 21, 2023 • edited Loading

✅ Backports have been created

github-actions bot commented Nov 21, 2023

[FE Incremental Coverage Report]

github-actions bot commented Nov 21, 2023

[BE Incremental Coverage Report]

MaxWk commented Nov 10, 2023 •

edited by wyb

Loading

CLAassistant commented Nov 10, 2023 •

edited

Loading

mergify bot commented Nov 21, 2023 •

edited

Loading

mergify bot commented Nov 21, 2023 •

edited

Loading

mergify bot commented Nov 21, 2023 •

edited

Loading

mergify bot commented Nov 21, 2023 •

edited

Loading