Skip to content
This repository was archived by the owner on Jan 28, 2021. It is now read-only.

Do not cancel context for async queries on success #859

Merged
merged 1 commit into from
Oct 28, 2019

Conversation

carlosms
Copy link
Contributor

Fix #857.

The code in #806 added a context cancel call when the query handle finishes,

defer cancel()

For an async index creation the driver is called in a goroutine

createIndex := func() {

And when the driver went to do the actual index creation in that goroutine, the query had already exited, and the context was cancelled

case <-ctx.Done():

The first option discussed in slack (private link) was to change the context of the createIndex function linked above with a ctx.WithContext(context.Background()), if the call was made in a goroutine.

This removes the original context cancellation, which fixes the problem, but it also prevents from doing a KILL QUERY of the async task afterwards. This would be a regression over the latest stable gitbase release.

The code I propose in this PR instead removes the deferred context cancellation in the handler if the query is async. It feels like the code is kind of mixing responsibilities, making the Handler aware of details about the query that should be handled by the Engine. But this was the cleanest approach I could think of.

I am still trying to figure out how this could be tested, any suggestion would be welcome.

@carlosms carlosms requested a review from a team October 25, 2019 15:45
@erizocosmico erizocosmico merged commit 5b68206 into src-d:master Oct 28, 2019
@carlosms carlosms deleted the fix-async-index branch October 28, 2019 09:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Async index creation is not working
3 participants