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

fix heartbeating in fakeProgress activity #366

Open
ssrohit opened this issue May 19, 2024 · 2 comments
Open

fix heartbeating in fakeProgress activity #366

ssrohit opened this issue May 19, 2024 · 2 comments
Labels
bug Something isn't working

Comments

@ssrohit
Copy link

ssrohit commented May 19, 2024

What are you really trying to do?

When heartbeating after some task (that may be writing data in DB) in fakeProgress activity when there is a failure the activity retries from a previous successful progress which may lead to data inconsistency(when dealing with data insertion in DB).

Describe the bug

With the existing heartbeat in the fakeProgress activity it whem retried it have to perform an extra iteration, which when the heartbeat is sent immediately after entering the loop saves this extra iteration. When some data insertion is a step in this loop, this may lead to data duplication.

Minimal Reproduction

I have identified this issue on my personal project and this is quite self explanatory.
Example case:
when progress = 4
heartbeat details -> 3 (previous progress)
if activity fails at this progress it will be retried from the point where progress == 3, and the iteration with progress == 3 is a successful iteration which should not be the retrying point.
PR - #363

Environment/Versions

temporal sdk version - 1.9.3

@ssrohit ssrohit added the bug Something isn't working label May 19, 2024
@johnson-lu-opus
Copy link

You cannot trust the progress, as the heartbeat(progress) could be throttled. The progress in the last heartbeat is probably less than the actual progress.
It is better to always to do some checks before inserting to database (or catch the unique key constraint violations).

@ssrohit
Copy link
Author

ssrohit commented Oct 6, 2024

@johnson-lu-opus I agree with you that there should be checks before inserting to DB, but it doesn't makes sense when an activity gets retried from a successful iteration.
@mjameswh can you please review this issue and the PR attached.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants