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

Revert "Merge pull request #1033 from cloudflare/bcaimano/actor-cache-no-state #1155

Merged
merged 1 commit into from
Sep 8, 2023

Conversation

jp4a50
Copy link
Collaborator

@jp4a50 jp4a50 commented Sep 8, 2023

This reverts commit 3a49006, reversing changes made to 812f78b.

…-no-state"

This reverts commit 3a49006, reversing
changes made to 812f78b.
@jp4a50 jp4a50 merged commit 2182afd into main Sep 8, 2023
7 checks passed
bcaimano added a commit that referenced this pull request Sep 12, 2023
…1033-main"

This reverts commit 2182afd, reversing
changes made to e4fe978.
bcaimano added a commit that referenced this pull request Sep 14, 2023
…r ActorCache::Entries with bugfix (#1162)

* Revert "Merge pull request #1155 from cloudflare/jphillips/revert-PR-1033-main"

This reverts commit 2182afd, reversing
changes made to e4fe978.

* Drive-by: Convert nullptr to kj::none

There are also a few spots where we should have been comparing size or using default init on strings or arrays!

* Drive-by: Convert actor cache uses of KJ_IF_MAYBE to KJ_IF_SOME

* Add test for eviction of list with known gaps

We weren't testing when we were evicting the last entry in a list with a known gap until an UNKNOWN (the end marker) entry. This matters because we had a hack in place to clean up the UNKNOWN entry which happens to have invalidated the cache list. This will be fixed in a following commit.

* Bugfix: Don't double evict for UNKNOWN entries

This was causing list iteration issues because UNKNOWN entries were now in the clean list. The fix here simply stops trying to clean up UNKNOWN entries after known empty gaps. They will be the oldest/stalest entries because if they were not then they would be dirty new entries. (We will not return a cached UNKNOWN result.)
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.

2 participants