Use default context output rather than outputWithTimestamp for ElasticsearchIO#16744
Conversation
|
R: @echauchot |
|
@egalpin I thought this PR was only for cosmetics, but I see that it is linked to BEAM-14064 if I understood correctly. For next time, if it is linked with an open issue, remember to put the jira ticket in the commit. I'll not reword as it is already merged to master |
|
Will do @echauchot 👍 This change was made and merged prior to BEAM-14064, but I think it would have been useful for me to document the reason for my fix in Jira anyway. Thanks for the reminder! |
|
I think this is still incorrect as the buffered Document will be assigned to an arbitrary window and timestamp. |
|
I agree that it's sub-optimal. I do feel that having this change is far better than not, otherwise ElasticsearchIO#write will be unusable. Ideally, I'll be able to fix the data correctness issue before 2.38.0 release after getting advice on the dev@ mailing list for best practices. |
|
I thought this was considered the final fix so thanks for confirming that this is only a partial improvement and that your looking at fixing it completely. |
3 participants
Using
ProcessContext#outputWithTimestampis more fragile thanoutput, and explicit use ofoutputWithTimestampis not required in the case of ES module; in fact, code is currently trying to emulateoutputby storing and recalling element timestamps. This change is being made after finding an issue with timestamp validation when starting a pipeline from a snapshot that could have been avoided with this change.Please add a meaningful description for your change here
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
R: @username).[BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replaceBEAM-XXXwith the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.CHANGES.mdwith noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
ValidatesRunnercompliance status (on master branch)Examples testing status on various runners
Post-Commit SDK/Transform Integration Tests Status (on master branch)
Pre-Commit Tests Status (on master branch)
See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI.