Skip to content
This repository has been archived by the owner on Oct 9, 2023. It is now read-only.

Only enqueue non-terminal tasks #164

Merged
merged 14 commits into from
Sep 14, 2023
Merged

Only enqueue non-terminal tasks #164

merged 14 commits into from
Sep 14, 2023

Conversation

pingsutw
Copy link
Member

@pingsutw pingsutw commented Sep 12, 2023

TL;DR

I find that Propeller tries to process the completed task from the workqueue because we enqueue the completed item. we should check the item's status before we enqueue them.

Problem:

Time WorkQueue Worker node Evaluation
T1 Add item1
T2 Process item1 (job finish)
T3 Add item1
T4 Process item1 again
T5 Update the node status (running -> succeed)

After:

Time WorkQueue Worker node Evaluation
T1 Add item1
T2 Process item1 (job finish)
T3
T4
T5 Update the node status (running -> succeed)

Type

  • Bug Fix
  • Feature
  • Plugin

Are all requirements met?

  • Code completed
  • Smoke tested
  • Unit tests added
  • Code documentation added
  • Any pending items have an associated Issue

Complete description

^^^

Tracking Issue

NA

Follow-up issue

NA

Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
@pingsutw pingsutw changed the title Enqueue only running tasks only Enqueue non-terminal task tasks Sep 12, 2023
@pingsutw pingsutw changed the title only Enqueue non-terminal task tasks Only enqueue non-terminal task tasks Sep 12, 2023
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
@codecov
Copy link

codecov bot commented Sep 13, 2023

Codecov Report

Patch coverage: 58.33% and project coverage change: -0.27% ⚠️

Comparison is base (4b950ec) 67.88% compared to head (15416e0) 67.61%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #164      +/-   ##
==========================================
- Coverage   67.88%   67.61%   -0.27%     
==========================================
  Files          69       69              
  Lines        4110     4113       +3     
==========================================
- Hits         2790     2781       -9     
- Misses       1157     1167      +10     
- Partials      163      165       +2     
Flag Coverage Δ
unittests 67.61% <58.33%> (-0.27%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
cache/auto_refresh.go 70.39% <58.33%> (-7.46%) ⬇️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Kevin Su <[email protected]>
@pingsutw pingsutw requested review from kumare3 and removed request for kumare3 September 13, 2023 00:34
@pingsutw pingsutw changed the title Only enqueue non-terminal task tasks Only enqueue non-terminal tasks Sep 13, 2023
cache/auto_refresh.go Outdated Show resolved Hide resolved
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
@pingsutw pingsutw merged commit 8f40575 into master Sep 14, 2023
5 of 7 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants