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

Cascade cache remove loops forever on cyclic references #6133

Closed
cvb941 opened this issue Aug 26, 2024 · 11 comments · Fixed by #6137
Closed

Cascade cache remove loops forever on cyclic references #6133

cvb941 opened this issue Aug 26, 2024 · 11 comments · Fixed by #6137
Milestone

Comments

@cvb941
Copy link
Contributor

cvb941 commented Aug 26, 2024

Version

4.0.0

Summary

Our schema:

image Screenshot 2024-08-26 at 14 50 36

When deleting the Record object using its CacheKey with cascade = true. It seems to loop forever.

Here's the remove method:

Just looking at the code, there is a recursive call which probably calls remove between those two objects indefinitely. The cascade remove should probably gather all unique references first and then delete them.

@martinbonnin
Copy link
Contributor

Thanks for sending this 🙏 Do you have a reproducer by any chance? The remove() call there should be mostly idempotent so calling it multiple times with the same cache key should be harmless I think (recordJournal should be null the 2nd time). But there could very well be something else. I'll keep digging but a reproducer would help a ton.

@cvb941
Copy link
Contributor Author

cvb941 commented Aug 26, 2024

Yes I missed that, you're right the entries should be removed from the journal and lruCache in the memory cache. For some reason the process gets stuck though. I will investigate more and maybe setup a reproduction.

@cvb941
Copy link
Contributor Author

cvb941 commented Aug 26, 2024

Okay I think I got it. The problem is in the SQL cache (I have the memory cache chained to SQL)

Here is the offending method:

It keeps trying to delete children before itself and thus create a loop.
image

@martinbonnin
Copy link
Contributor

Excelent catch! Do you want to submit a fix?

  • load all referenced ids
  • delete record
  • delete each referenced id. If any is self referential, the fact that it's now deleted should break the cycle.

@cvb941
Copy link
Contributor Author

cvb941 commented Aug 26, 2024

Sure, I'll see when I'll get to it though :)

@cvb941
Copy link
Contributor Author

cvb941 commented Sep 3, 2024

Okay I took a look at this and I stopped at having trouble with just opening the Apollo project in the IDE. I tried both Android Studio and IDEA but they fail at processing the source files.

I guess it would be faster for you to fix this properly, it should be very simple for you.

@martinbonnin
Copy link
Contributor

I tried both Android Studio and IDEA but they fail at processing the source files.

Interesting. This was an issue a couple of years ago but it's been a while I haven't seen any issue. Do you have any Gradle sync issue?

@cvb941
Copy link
Contributor Author

cvb941 commented Sep 3, 2024

Gradle sync passes, I found out the issue is caused by the SQLDelight plugin. Do you use it without any issues? Disabling it fixed both IDEs.

@martinbonnin
Copy link
Contributor

Ah good catch. Indeed, I do not have the SQLDelight plugin installed. Might be worth filing a bug if it's reproducible, I'll look into this. Do you still want me to do the patch?

@cvb941
Copy link
Contributor Author

cvb941 commented Sep 3, 2024

I reported the exceptions from IDEA. I'm working on the patch now 😎

Copy link
Contributor

github-actions bot commented Sep 3, 2024

Do you have any feedback for the maintainers? Please tell us by taking a one-minute survey. Your responses will help us understand Apollo Kotlin usage and allow us to serve you better.

@BoD BoD added this to the 4.0.1 milestone Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants