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

deleteat! fails in sra_migrate! #77

Open
StevenWhitaker opened this issue Feb 2, 2024 · 2 comments
Open

deleteat! fails in sra_migrate! #77

StevenWhitaker opened this issue Feb 2, 2024 · 2 comments

Comments

@StevenWhitaker
Copy link

StevenWhitaker commented Feb 2, 2024

foreach(idx->deleteat!(from_refs, idx), reverse(to_delete))

The call to reverse above assumes to_delete is in ascending order, which isn't necessarily the case. I didn't thoroughly test, so I don't know what combinations of to_mem and sra.policy cause an error, but I got an error about trying to delete an index that didn't exist when using a LRU policy (don't know if to_mem was true or not). The problem did not occur when I switched to a MRU policy. Sorry I don't have a MWE, though I can reliably reproduce the issue.

My problem went away when I replaced reverse(to_delete) with sort(to_delete; rev = true).

@jpsamaroo
Copy link
Collaborator

Yeah that's a good find - an alternative to sort(to_delete; rev=true) is to call sort!(to_delete; rev=true), which would reuse the to_delete array, and should still return it too. Would you be willing to open a PR? I don't know how exactly to test this, aside from implementing a stress-test for MemPool's swap-to-disk (which I do want to do once I find the time).

@StevenWhitaker
Copy link
Author

Sure, I can open a PR when I have time over the next couple of days. I'll ping you when it's ready.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants