Skip to content

Conversation

@caidezhi
Copy link

@caidezhi caidezhi commented Feb 9, 2021

Tips

What is the purpose of the pull request

run HoodieJavaWriteClientExample locally ,the code fail with exception. The root cause is that BaseJavaCommitActionExecutor#execute method does not commit the index when it is needed.
the PR is to fix the issue.

Brief change log

org.apache.hudi.table.action.commit.BaseJavaCommitActionExecutor

Verify this pull request

This pull request is already covered by existing tests: HoodieJavaWriteClientExample.main()
I 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-io
Copy link

codecov-io commented Feb 9, 2021

Codecov Report

Merging #2560 (835aed0) into master (b0010bf) will increase coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #2560   +/-   ##
=========================================
  Coverage     51.15%   51.15%           
- Complexity     3212     3213    +1     
=========================================
  Files           436      436           
  Lines         19987    19987           
  Branches       2057     2057           
=========================================
+ Hits          10224    10225    +1     
  Misses         8922     8922           
+ Partials        841      840    -1     
Flag Coverage Δ Complexity Δ
hudicli 36.90% <ø> (ø) 0.00 <ø> (ø)
hudiclient 100.00% <ø> (ø) 0.00 <ø> (ø)
hudicommon 51.39% <ø> (ø) 0.00 <ø> (ø)
hudiflink 45.44% <ø> (ø) 0.00 <ø> (ø)
hudihadoopmr 33.16% <ø> (ø) 0.00 <ø> (ø)
hudisparkdatasource 69.73% <ø> (ø) 0.00 <ø> (ø)
hudisync 48.61% <ø> (ø) 0.00 <ø> (ø)
huditimelineservice 66.49% <ø> (ø) 0.00 <ø> (ø)
hudiutilities 69.44% <ø> (+0.05%) 0.00 <ø> (ø)

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

Impacted Files Coverage Δ Complexity Δ
...apache/hudi/utilities/deltastreamer/DeltaSync.java 70.35% <0.00%> (+0.35%) 51.00% <0.00%> (+1.00%)

recordsSoFar.stream().map(r -> new HoodieRecord<HoodieAvroPayload>(r)).collect(Collectors.toList());
client.upsert(writeRecords, newCommitTime);
List<WriteStatus> insertStatus = client.upsert(writeRecords, newCommitTime);
client.commit(newCommitTime,insertStatus);
Copy link
Contributor

Choose a reason for hiding this comment

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

would we just change the BaseJavaCommitActionExecutor#execute method to add updateIndexAndCommitIfNeeded method and align with BaseSparkCommitActionExecutor?

Copy link
Author

Choose a reason for hiding this comment

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

@leesf many thanks for your review. You suggestion make more sense and i have change the code accordingly.

@caidezhi caidezhi changed the title [HUDI-1606]fix HoodieJavaWriteClientExample [HUDI-1606]align BaseJavaCommitActionExecuto#execute method with BaseSparkCommitActionExecutor Feb 10, 2021
@nsivabalan nsivabalan added the priority:high Significant impact; potential bugs label Feb 21, 2021
result.setWriteStatuses(statuses);
}

protected void updateIndexAndCommitIfNeeded(List<WriteStatus> writeStatuses, HoodieWriteMetadata<List<WriteStatus>> result) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@caidezhi Sorry for the delay, there is a PR #2382 to support COW in Java client which includes the changes here.

Copy link
Author

Choose a reason for hiding this comment

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

@leesf if it is already covered by other PR, please feel free to close this one. Thanks for your review.

@leesf leesf closed this Feb 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority:high Significant impact; potential bugs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants