-
Notifications
You must be signed in to change notification settings - Fork 23
Conversation
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]>
fix Also, in the worst case would just one more call to Get would done? |
cache/auto_refresh.go
Outdated
@@ -235,7 +235,8 @@ func (w *autoRefresh) enqueueBatches(ctx context.Context) error { | |||
|
|||
for _, batch := range batches { | |||
b := batch | |||
w.workqueue.Add(&b) | |||
logger.Debugf(ctx, "Enqueuing batch with id: %v", b[0].GetID()) | |||
w.workqueue.Add(b[0].GetID()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This assumes that batches only have one item... I don't think this is true in all usage of AutoRefresh cache... if it's, I would rather deprecate the whole batching mechanism...
This implementation won't work for any batches > 1 item.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In flytepropeller, batches always only have one item.
flytestdlib/cache/auto_refresh.go
Lines 133 to 140 in 1285ff3
func SingleItemBatches(_ context.Context, snapshot []ItemWrapper) (batches []Batch, err error) { | |
res := make([]Batch, 0, len(snapshot)) | |
for _, item := range snapshot { | |
res = append(res, Batch{item}) | |
} | |
return res, nil | |
} |
Will clean up the code.
Signed-off-by: Kevin Su <[email protected]>
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #165 +/- ##
==========================================
- Coverage 67.83% 67.79% -0.04%
==========================================
Files 69 69
Lines 4113 4124 +11
==========================================
+ Hits 2790 2796 +6
- Misses 1159 1163 +4
- Partials 164 165 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
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]>
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]>
Hi, we are moving all Flyte development to a monorepo. In order to help the transition period, we're moving open PRs to monorepo automatically and your PR was moved to flyteorg/flyte#4138. Notice that if there are any conflicts in the resulting PR they most likely happen due to the change in the import path of the flyte components. |
TL;DR
I found that Propeller keeps sending the request to the agent even workflow is done.
The work queue only has unique items, so we add
itemID
to it. The workers won't process the item again after the task is done.Before:
After:
100 workflows, each has 100 tasks.
Before:
After:
Type
Are all requirements met?
Complete description
^^^
Tracking Issue
NA
Follow-up issue
NA