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

[FLINK-36143][ES6][ES7] Intro retry-on-conflict param to resolve Sink ES occurred "version conflict" #109

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

leosanqing
Copy link

When I used ES connector Write update datas, it occurred "version conflict", like this " org.elasticsearch.index.engine.VersionConflictEngineException: [project][1140860]: version conflict, current version [5] is different than the one provided [4]
"
When we set the conf failure-handler = ignore, It lead to lost data, and if set the conf failure-handler = fail, job will restore. this cost is too heavy. maybe second try it will success.

so I intro this conf param, this will make job failed cost smaller.

Copy link

boring-cyborg bot commented Aug 23, 2024

Thanks for opening this pull request! Please check out our contributing guidelines. (https://flink.apache.org/contributing/how-to-contribute.html)

@leosanqing
Copy link
Author

@schulzp @alpinegizmo @aljoscha @rmetzger Has any body to review my code? thk ❤️❤️

Copy link
Contributor

@schulzp schulzp left a comment

Choose a reason for hiding this comment

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

Thank you for your patch! I am not an official maintainer. Your changes look reasonable, however, they lack test coverage. One other thing I noticed: The newly introduced constant RETRY_ON_CONFLICT_NUM does not follow the naming convention of other config options in ElasticsearchConnectorOptions: Most of them appear to end on [...]_OPTION but do not have the type-indicating suffix. Renaming it to sink.retries-on-conflict could eliminate the confusion wether this is a boolean flag or a number.

@leosanqing
Copy link
Author

Thanks for your advices,and I fixed this. Has anybody can review my code?

Thank you for your patch! I am not an official maintainer. Your changes look reasonable, however, they lack test coverage. One other thing I noticed: The newly introduced constant RETRY_ON_CONFLICT_NUM does not follow the naming convention of other config options in ElasticsearchConnectorOptions: Most of them appear to end on [...]_OPTION but do not have the type-indicating suffix. Renaming it to sink.retries-on-conflict could eliminate the confusion wether this is a boolean flag or a number.

@leosanqing
Copy link
Author

@reswqa @mtfelisb hi guys,could you review my pr? thx

@leosanqing
Copy link
Author

@rmetzger hi, thx for your triggering. I see there has some error when run IT case. And there also has same error in before PR . Is there some problems in project? The leatest merged pr doesn't have this step, latest un-merged PRs both have this error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants