-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Override useCommitCoordinator to false #9017
Conversation
@huaxingao, shall we also override these places?
|
I just override
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me.
I think this change is safe cause we don't strictly require a single task attempt to commit. We only require that Spark picks one commit message if multiple task attempts succeed. That's true even if we disable the commit coordinator.
I will leave this PR open for a bit to see if anyone has any concerns.
Thanks, @huaxingao! Could you do a similar fix for |
Thanks @aokolnychyi I will have a follow up to fix |
This change cherrypicks PR #9017 to Spark 3.4. Co-authored-by: Huaxin Gao <huaxin.gao@apple.com>
Hello, will this fix be propogated to 1.3.x and 1.4.x releases? |
hi @huaxingao, we tried to execute sql
Under: Iceberg 1.3.1 + Spark 3.4.1
|
@jiantao-vungle The problem you encountered is probably the same as the one in this PR. Are you experiencing this issue only in Spark 3.4? |
@aokolnychyi Do we need to back-port the fix to previous releases? |
Looping in @jbonofre here who's preparing the 1.4.3 release. |
@huaxingao, actually, it was encountered after we upgraded both Spark(from |
Thanks for the report. Let me take a look. |
…9028) This change cherrypicks PR apache#9017 to Spark 3.4. Co-authored-by: Huaxin Gao <huaxin.gao@apple.com>
Hi @aokolnychyi We are also facing same issue on Iceberg 1.3.0 while deleting records from the table using merge command. We recently upgraded to Spark 3.4.0 and started facing this issue. Can we back-port this fix to Iceberg 1.3.0? |
@mukeshkumarkulmi I will open a PR to backport the fix to 1.3.0 |
@mukeshkumarkulmi actually I am not sure if I can back-port the fix to 1.3.0. Can you cherry-pick the fix to your internal 1.3.0? |
In Spark
BatchWrite.java
,useCommitCoordinator
is default to true. This PR overridesuseCommitCoordinator
to false, so we can rewrite a task after it fails. Here is the issue