[in-progress] Protect datalog cache from early cancellation #657
+245
−34
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
View Vercel preview at instant-www-js-dont-cancel-jsv.vercel.app. |
stopachka
reviewed
Dec 24, 2024
| @@ -342,23 +347,112 @@ | |||
| ;; ------ | |||
| ;; datalog cache | |||
|
|
|||
| (defn swap-datalog-cache-delay! [conn app-id datalog-query delayed-call] | |||
| (defn swap-datalog-cache! [conn app-id datalog-query-fn ctx datalog-query] | |||
stopachka
reviewed
Dec 24, 2024
| ;; We added it, so we must execute it | ||
| (let [stmt-tracker (sql/make-top-level-statement-tracker) | ||
| result-promise (:promise @result-delay) | ||
| work-fut (binding [ua/*child-vfutures* nil ;; Move future to a new "call-stack" |
There was a problem hiding this comment.
Man, bind + statement-tracker makes for a really powerful abstraction.
stopachka
approved these changes
Dec 24, 2024
stopachka
reviewed
Dec 24, 2024
| q))] | ||
| @started | ||
| (future-cancel f1) | ||
| (is (instance? java.lang.InterruptedException (deref err 100 :timeout))))) |
There was a problem hiding this comment.
Ah interesting, so err is taking more than 100ms to deliver. 🤔
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 prevents in-progress datalog queries from getting cancelled when there are multiple sessions waiting on the query.
Previously, we could get into a situation like this:
This PR moves the query execution to a separate call stack where it won't get canceled if the parent is canceled.
To keep the behavior where we cancel queries that exceed the
handle-receive-timeout-ms, we add another future that is in the call stack. If it gets canceled and there are no other threads waiting on the query, then it will cancel the query.