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.

Sign up for GitHub

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

Jump to bottom

We should not use blockingCallFromThread in the composer #3236

Closed
bowlofeggs opened this issue May 17, 2019 · 1 comment
Closed

We should not use blockingCallFromThread in the composer #3236

bowlofeggs opened this issue May 17, 2019 · 1 comment
Assignees
@bowlofeggs
Labels
Composer Issues related to the composer Crash Issues related to an unhandled crash Critical We can't go on living in this sqalor, drop everything and fix it!

Comments

@bowlofeggs
Copy link
Contributor

I spoke with @jeremycline today and he informed me that we should not be using twisted.internet.threads.blockingCallFromThread to wrap Bodhi's calls to publish(). The reason is that though we are running in a thread, we are not a thread that is running the event loop and thus we should not be using any twisted code. We can treat all our code as if it were synchronous here.

Thus, I think we want to revert #3145

This seems to be related to the crash we saw in fedora-infra/fedora-messaging#175
@bowlofeggs bowlofeggs added Critical We can't go on living in this sqalor, drop everything and fix it! Composer Issues related to the composer Crash Issues related to an unhandled crash labels May 17, 2019
@jeremycline
Copy link
Member

jeremycline commented May 17, 2019 •
edited
Loading

To expand on this a little bit:

fedora_messaging.api.publish uses the Pika blocking connection, not a connection managed by Twisted. Thus, it doesn't need to run inside the reactor thread and it, in fact, blocks the reactor loop so things like heartbeating can't occur. If you were calling the Twisted version of publish, you would need to use blockingCallFromThread.

@bowlofeggs bowlofeggs self-assigned this May 17, 2019
bowlofeggs added a commit to bowlofeggs/bodhi that referenced this issue May 17, 2019
@bowlofeggs
Revert "Handle thread safety wrt the messaging API in callbacks"
66ac53e 
We are not running Bodhi in Twisted's event loop thread, and we
are not using Twisted's Pika's connections, so we do not need to
use blockingCallFromThread on our publish calls.

This reverts commit e07fc25.

fixes fedora-infra#3236
bowlofeggs added a commit to bowlofeggs/bodhi that referenced this issue May 17, 2019
@bowlofeggs
Revert "Handle thread safety wrt the messaging API in callbacks"
ccfabd0 
We are not running Bodhi in Twisted's event loop thread, and we
are not using Twisted's Pika's connections, so we do not need to
use blockingCallFromThread on our publish calls.

This reverts commit e07fc25.

fixes fedora-infra#3236

Signed-off-by: Randy Barlow <randy@electronsweatshop.com>
bowlofeggs added a commit to bowlofeggs/bodhi that referenced this issue May 20, 2019
@bowlofeggs
Revert "Handle thread safety wrt the messaging API in callbacks"
5e10d4c 
We are not running Bodhi in Twisted's event loop thread, and we
are not using Twisted's Pika's connections, so we do not need to
use blockingCallFromThread on our publish calls.

This reverts commit e07fc25.

fixes fedora-infra#3236

Signed-off-by: Randy Barlow <randy@electronsweatshop.com>
bowlofeggs added a commit to bowlofeggs/bodhi that referenced this issue May 23, 2019
@bowlofeggs
Revert "Handle thread safety wrt the messaging API in callbacks"
6941780 
We are not running Bodhi in Twisted's event loop thread, and we
are not using Twisted's Pika's connections, so we do not need to
use blockingCallFromThread on our publish calls.

This reverts commit e07fc25.

fixes fedora-infra#3236

Signed-off-by: Randy Barlow <randy@electronsweatshop.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bowlofeggs bowlofeggs

Labels
Composer Issues related to the composer Crash Issues related to an unhandled crash Critical We can't go on living in this sqalor, drop everything and fix it!
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@bowlofeggs @jeremycline