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

executor: avoid goroutine leak in TableReader along with memory leak #18057

Closed
4 tasks
fzhedu opened this issue Jun 16, 2020 · 2 comments · Fixed by #18445
Closed
4 tasks

executor: avoid goroutine leak in TableReader along with memory leak #18057

fzhedu opened this issue Jun 16, 2020 · 2 comments · Fixed by #18445
Labels

Comments

@fzhedu
Copy link
Contributor

fzhedu commented Jun 16, 2020

Bug Report

Please answer these questions before submitting your issue. Thanks!

1. Minimal reproduce step (Required)

The source code only generates go routines, but not handle errors if some exceptions happens.
To to reproduce this issue using failpoint.

On the other hand, if the main thread is cancelled, those generated goroutines should stop correctly.

detail problems:

  • TableReader->Next() is blocked, how to stop the execution once cancel the query manually?
  • When call TableReader->Close(), how to stop coprocessor workers?
  • When coprocessor workes panic, how to cancel the query correctly?
  • what about IndexReader?

2. What did you expect to see? (Required)

all goroutine stops and return errors.

3. What did you see instead (Required)

other goroutine runs normally and occupy many memory .

4. Affected version (Required)

v4.0.x, v3

5. Root Cause Analysis

lack handing exceptions.

@fzhedu fzhedu added the type/bug The issue is confirmed as a bug. label Jun 16, 2020
@fzhedu fzhedu changed the title executor: avoid goroutine leak in TableReader executor: avoid goroutine leak in TableReader along with memory leak Jun 17, 2020
@g1eny0ung
Copy link

/label component/executor

@tiancaiamao
Copy link
Contributor

Good questions, I'm glad to answer them:

  1. TableReader->Next() is blocked, how to stop the execution once cancel the query manually?

Everytime executor.Next is called, the killed flag is checked.
If the code is blocked within TableReader->Next() and never run to the Next() function, we should check why it's blocked and check the flag there.

For example, we have already check the pessimistic lock waiting, each backoff and coprocessor finish one region.

  1. When call TableReader->Close(), how to stop coprocessor workers?

The correct order should be coprocessor throw an error (maybe ErrInterrupted) to TableReader, then TableReader call Close() function.

We should not let TableReader peceive kill during the coprocessor worker is working, exception handling is hard that way.
Instead, only one place handle the kill flag and turn it into ErrInterrupted.
Exception handling is turned into error handling which is much easier.

  1. When coprocessor workes panic, how to cancel the query correctly?

When the coprocessor workes panic, it should throw error to the caller -- TableReader, then the TableReader handle the error.

  1. what about IndexReader?

The same with TableReader.

It's important to understand that there is no real cancel operation.
It's wrong to think like a caller preceive the killed flag and cancel the worker.
killed flag is checked only when some code runs to check it, in this way exception handling is turned to error handling.

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

Successfully merging a pull request may close this issue.

5 participants