Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[HUDI-1988] FinalizeWrite() been executed twice in AbstractHoodieWrit… #3050

Merged
merged 1 commit into from Jun 23, 2021

Conversation

swuferhong
Copy link
Contributor

…eClient$commitstats

Tips

What is the purpose of the pull request

FinalizeWrite() been executed twice in AbstractHoodieWriteClient$commitstats. once in commitstats() self, another in commit() method in commitstats(). I think this is a useless repetition.

Brief change log

(for example:)

  • Modify AnnotationLocation checkstyle rule in checkstyle.xml

Verify this pull request

(Please pick either of the following options)

This pull request is a trivial rework / code cleanup without any test coverage.

(or)

This pull request is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(example:)

  • Added integration tests for end-to-end.
  • Added HoodieClientWriteTest to verify the change.
  • Manually verified the change by running a job locally.

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.

@codecov-commenter
Copy link

codecov-commenter commented Jun 8, 2021

Codecov Report

Merging #3050 (1f85ab3) into master (f760ec5) will decrease coverage by 0.32%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #3050      +/-   ##
============================================
- Coverage     55.31%   54.99%   -0.33%     
- Complexity     4026     4044      +18     
============================================
  Files           520      526       +6     
  Lines         25295    25668     +373     
  Branches       2872     2950      +78     
============================================
+ Hits          13993    14117     +124     
- Misses         9914    10164     +250     
+ Partials       1388     1387       -1     
Flag Coverage Δ
hudicli 39.95% <ø> (ø)
hudiclient ∅ <ø> (∅)
hudicommon 50.03% <ø> (+0.05%) ⬆️
hudiflink 60.58% <ø> (-2.26%) ⬇️
hudihadoopmr 51.43% <ø> (ø)
hudisparkdatasource 66.53% <ø> (+0.01%) ⬆️
hudisync 47.94% <ø> (-3.51%) ⬇️
huditimelineservice 64.36% <ø> (ø)
hudiutilities 71.79% <ø> (+0.73%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...in/java/org/apache/hudi/hive/HoodieHiveClient.java 52.56% <0.00%> (-19.04%) ⬇️
...apache/hudi/sink/compact/CompactionCommitSink.java 65.00% <0.00%> (-10.00%) ⬇️
...va/org/apache/hudi/table/format/FilePathUtils.java 66.17% <0.00%> (-0.75%) ⬇️
...va/org/apache/hudi/sink/utils/HiveSyncContext.java 91.66% <0.00%> (-0.23%) ⬇️
...java/org/apache/hudi/common/fs/StorageSchemes.java 100.00% <0.00%> (ø)
.../org/apache/hudi/sink/compact/CompactFunction.java 100.00% <0.00%> (ø)
.../org/apache/hudi/streamer/FlinkStreamerConfig.java 0.00% <0.00%> (ø)
...e/hudi/sink/transform/RowDataToHoodieFunction.java 100.00% <0.00%> (ø)
...main/scala/org/apache/hudi/HoodieWriterUtils.scala 83.33% <0.00%> (ø)
...i/bootstrap/SparkParquetBootstrapDataProvider.java 80.00% <0.00%> (ø)
... and 20 more

@nsivabalan
Copy link
Contributor

@n3nash : can you take this up. I see you have added one of the finalize as part of multi-writer. not sure if it was moved from elsewhere.

@n3nash
Copy link
Contributor

n3nash commented Jun 14, 2021

@swuferhong Thanks for opening this. Looks like it might be a duplicate. Can you check why the CI is failing ?

@swuferhong swuferhong closed this Jun 15, 2021
@swuferhong swuferhong reopened this Jun 15, 2021
@swuferhong swuferhong closed this Jun 15, 2021
@swuferhong swuferhong reopened this Jun 15, 2021
@swuferhong
Copy link
Contributor Author

@swuferhong Thanks for opening this. Looks like it might be a duplicate. Can you check why the CI is failing ?

Yes, we reopened this PR, and CLI passing now.

@swuferhong
Copy link
Contributor Author

@n3nash Thanks, we reopened this PR, and CLI passing now.

@hudi-bot
Copy link

hudi-bot commented Jun 15, 2021

CI report:

Bot commands @hudi-bot supports the following commands:
  • @hudi-bot run travis re-run the last Travis build
  • @hudi-bot run azure re-run the last Azure build

@vinothchandar vinothchandar added this to Ready for Review in PR Tracker Board Jun 16, 2021
@n3nash n3nash self-requested a review June 23, 2021 05:56
PR Tracker Board automation moved this from Ready for Review to Nearing Landing Jun 23, 2021
Copy link
Contributor

@n3nash n3nash left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@n3nash n3nash merged commit 3fb59dd into apache:master Jun 23, 2021
PR Tracker Board automation moved this from Nearing Landing to Done Jun 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants