Ensure upserted cache entries always get written #4768
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR:
upsertQueryEntries
where updates to an existing cache entry would not actually get completed, leaving the entry in an incorrectpending
state indefinitelyFixes #4749
Investigation
Made a Replay that shows the repro in action:
Looks like there was a small flaw in the logic. We already had a check for the
fulfilled
case that bails out of writing the result ifsubstate.requestId !== meta.requestId
, so that we don't end up with conflicting request results being written. However, it also had a check for the older forced upsert logic as well, and it wasn't accounting for this newer approach.Meanwhile, the
pending
logic actually keeps around the existing request ID:So, what was happening was roughly:
fulfilled
action for ID 2 is written, with its request IDupsertQueryEntries
for ID 2 is dispatchedpending
write logic for the upsert sees there's an existing entry, and keeps the old request ID from the actual API response, instead of overwriting it with the fake request ID from the upsertfulfilled
write logic for the upsert tries to update the entry... but the existing and incoming request IDs don't match, and it wasn't being detected as a forced upsert because that logic only was checking for the olderupsertQueryData
approachand so we're left with the existing entry, updated to be in a
pending
state, and without the upserted data written and marked as fulfilled.