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

Support DeleteDelayed operation in AutoRefresh cache #98

Merged
merged 1 commit into from
Sep 21, 2021
Merged

Conversation

EngHabu
Copy link
Contributor

@EngHabu EngHabu commented Sep 21, 2021

Signed-off-by: Haytham Abuelfutuh [email protected]

TL;DR

Support DeleteDelayed operation in AutoRefresh cache. This allows callers to safely enqueue a delete operation for a key and ensure it gets deleted even if the cache is in the middle of being refreshed.

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

Tracking Issue

flyteorg/flyte#1379

@EngHabu EngHabu marked this pull request as ready for review September 21, 2021 23:08
@EngHabu EngHabu requested a review from katrogan as a code owner September 21, 2021 23:08
@codecov
Copy link

codecov bot commented Sep 21, 2021

Codecov Report

Merging #98 (e3b48f1) into master (3191899) will decrease coverage by 0.01%.
The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #98      +/-   ##
==========================================
- Coverage   66.78%   66.77%   -0.02%     
==========================================
  Files          59       60       +1     
  Lines        2824     2847      +23     
==========================================
+ Hits         1886     1901      +15     
- Misses        793      800       +7     
- Partials      145      146       +1     
Flag Coverage Δ
unittests 65.15% <61.11%> (-0.05%) ⬇️

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

Impacted Files Coverage Δ
cache/auto_refresh.go 78.51% <20.00%> (-5.42%) ⬇️
cache/sync_set.go 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3191899...e3b48f1. Read the comment docs.

@wild-endeavor
Copy link
Contributor

Do you remember why we didn't have this to begin with? I remember we had some concerns about deletion but I don't remember why.

@EngHabu
Copy link
Contributor Author

EngHabu commented Sep 21, 2021

Yeah the concern was that if we delete from the cache while a sync operation is happening, the item will show right up as soon as the sync loop finishes... this change mitigates that by not deleting immediately but instead queuing the deletion operation and only perform it as part of the sync loop

@EngHabu EngHabu merged commit dcc4d89 into master Sep 21, 2021
@kumare3
Copy link
Contributor

kumare3 commented Sep 22, 2021

@EngHabu this is slowly becoming the controller workqueue and the informer cache, we should find a way to re-use

eapolinario pushed a commit that referenced this pull request Sep 6, 2023
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