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

Cancel pending requests when didChange & et all appear. #6131

Closed
mkaput opened this issue Aug 2, 2024 · 1 comment · Fixed by #6263
Closed

Cancel pending requests when didChange & et all appear. #6131

mkaput opened this issue Aug 2, 2024 · 1 comment · Fixed by #6263
Assignees

Comments

@mkaput
Copy link
Member

mkaput commented Aug 2, 2024

Then any of the textDocument/did* requests come, we need to cancel all operations that are holding snapshots of the AnalysisDatabase in order to unlock mutations to it.

This task is spin off #5355

@mkaput mkaput added this to CairoLS Aug 2, 2024
@mkaput mkaput converted this from a draft issue Aug 2, 2024
@mkaput mkaput added the ide label Aug 2, 2024
@mkaput mkaput added this to the LS: The Great Rewrite milestone Aug 2, 2024
@mkaput mkaput moved this from Triage to Todo in CairoLS Aug 2, 2024
@mkaput mkaput moved this from Todo to Backlog in CairoLS Aug 5, 2024
@mkaput
Copy link
Member Author

mkaput commented Aug 6, 2024

We have researched this with @Draggu. Conclusions:

Cancellation is well-defined in the following Salsa RFC: https://murek.dev/salsa-book-0.16/rfcs/RFC0007-Opinionated-Cancelation.html

When you do a write to the salsa database, that write will block until any queries running in background threads have completed. You really want those queries to complete quickly, though, because they are now operating on stale data and their results are therefore not meaningful. To expedite the process, salsa will cancel those queries. That means that the queries will panic as soon as they try to execute another salsa query.

Running cancellation is not dependent on any work on tower-lsplsp-server migration or any significant changes to LS state. Salsa should automatically initiate cancellation of running tasks, our job is just to catch cancelled task execution and properly report this fact to the client.

The problem is, this RFC is not implemented in Salsa 0.16 😭 We will have to upgrade to Salsa 0.17; probably the fork maintained by Rust Analyzer: https://crates.io/crates/ra_ap_salsa

@Draggu Draggu linked a pull request Aug 26, 2024 that will close this issue
Draggu added a commit that referenced this issue Aug 27, 2024
fix #6131

commit-id:c6b3800a
Draggu added a commit that referenced this issue Aug 27, 2024
fix #6131

commit-id:c6b3800a
Draggu added a commit that referenced this issue Aug 27, 2024
fix #6131

commit-id:c6b3800a
@github-project-automation github-project-automation bot moved this from In Progress to Done in CairoLS Aug 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants