Skip to content

fix: multiple snapshot task triggered #12

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

Merged
merged 1 commit into from
Aug 16, 2024

Conversation

yyzxw
Copy link
Collaborator

@yyzxw yyzxw commented Aug 14, 2024

When a pod is deleted, the Admission Webhook event handle will receive multiple events, so it will trigger the creation of multiple SnapshotTasks, so we need to determine whether there are duplicates

@muma378
Copy link
Collaborator

muma378 commented Aug 14, 2024

emmm,看起来没有什么大的问题,pod由于就绪探针导致的重启也会触发多次snapshotpodtask(但是应该会由于container image不存在导致提前中止?)
但是貌似有点复杂而且有状态了,可否直接基于上一个SnapshotPodTask 的 CreationTimestamp 判断是否要重新创建一个?例如两次之间至少相隔5s?

@yyzxw
Copy link
Collaborator Author

yyzxw commented Aug 14, 2024

emmm,看起来没有什么大的问题,pod由于就绪探针导致的重启也会触发多次snapshotpodtask(但是应该会由于container image不存在导致提前中止?) 但是貌似有点复杂而且有状态了,可否直接基于上一个SnapshotPodTask 的 CreationTimestamp 判断是否要重新创建一个?例如两次之间至少相隔5s?

这个可能来不及处理,我看了日志,webhook回调的时候时间间隔非常短(毫秒级别的),可能还没到判断上一个snapshottask的时候下一个事件就来了

@yyzxw yyzxw force-pushed the fix/mutil-snapshottask-created branch 3 times, most recently from 39c2589 to ab56813 Compare August 15, 2024 08:44
muma378
muma378 previously approved these changes Aug 15, 2024
kebe7jun
kebe7jun previously approved these changes Aug 15, 2024
@yyzxw yyzxw force-pushed the fix/mutil-snapshottask-created branch from ab56813 to acb58c3 Compare August 16, 2024 09:34
Copy link

codecov bot commented Aug 16, 2024

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

ℹ️ You can also turn on project coverage checks and project coverage reporting on Pull Request comment

Thanks for integrating Codecov - We've got you covered ☂️

@yyzxw yyzxw dismissed stale reviews from kebe7jun and muma378 via ac23068 August 16, 2024 09:53
@yyzxw yyzxw force-pushed the fix/mutil-snapshottask-created branch from acb58c3 to ac23068 Compare August 16, 2024 09:53
@yyzxw yyzxw merged commit 39941b5 into BaizeAI:main Aug 16, 2024
5 checks passed
@muma378
Copy link
Collaborator

muma378 commented Aug 16, 2024

🥳

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

Successfully merging this pull request may close these issues.

3 participants