Skip to content

Register for cq avalanching when interceptors are going to be run#17806

Merged
yashykt merged 6 commits intogrpc:masterfrom
yashykt:interceptorcqavalanching
Jan 29, 2019
Merged

Register for cq avalanching when interceptors are going to be run#17806
yashykt merged 6 commits intogrpc:masterfrom
yashykt:interceptorcqavalanching

Conversation

@yashykt
Copy link
Copy Markdown
Member

@yashykt yashykt commented Jan 24, 2019

A bug was discovered today where if the completion queue is shutdown before some tags are returned, then in those cases gRPC ends up asserting inside gRPC Core.

E0122 15:02:57.424189 49066 call.cc:1512] assertion failed: grpc_cq_begin_op(call->cq, notify_tag)

This happened because the interception API needs to generate more batches that need to go through the completion queue and if the shutdown happens before the interception API is done generating all the batches, we will get this error.

This can be avoided by delaying completion queue shutdown till the point that we can generate additional batches.

@yashykt yashykt requested a review from vjpai January 24, 2019 03:18
@yashykt yashykt added the release notes: yes Indicates if PR needs to be in release notes label Jan 24, 2019
@vjpai
Copy link
Copy Markdown
Contributor

vjpai commented Jan 24, 2019

Nice catch! Does this patch fix the crash observed by the user, and does this test change fail without the code change and pass with the code change?

@yashykt
Copy link
Copy Markdown
Member Author

yashykt commented Jan 24, 2019

Yes, the test change fails without the code change, and also fixes the crash observed by the user

@yashykt
Copy link
Copy Markdown
Member Author

yashykt commented Jan 29, 2019

Known issues : #17831, #17846, #17847

@yashykt
Copy link
Copy Markdown
Member Author

yashykt commented Jan 29, 2019

Thanks for reviewing!

@yashykt
Copy link
Copy Markdown
Member Author

yashykt commented Jan 29, 2019

Created new issues for all of the failing tests -
#17846, #17847, #17850, #17851

@yashykt yashykt merged commit f41affc into grpc:master Jan 29, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Apr 29, 2019
@yashykt yashykt deleted the interceptorcqavalanching branch May 18, 2023 20:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

kind/bug lang/c++ priority/P1 release notes: yes Indicates if PR needs to be in release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants