Skip to content
This repository has been archived by the owner on Jun 30, 2022. It is now read-only.

Defer creating all users on connection. #26

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

stevenharman
Copy link
Contributor

If we have A LOT of users, and especially if we're leveraging the :slack_user_created event triggered by UserCreator.create_user, the Slack Websocket can "time out." We only have 30 seconds to use a
connection once it's given to us.

This change defers the blocking, and possibly long-ish running creation of all uses to a background EM thread.

If we have A LOT of users, and especially if we're leveraging the
:slack_user_created event triggered by UserCreator.create_user, the
Slack Websocket can "time out." We only have 30 seconds to use a
connection once it's given to us.

This change defers the blocking, and possibly long-ish running creation
of all uses to a background EM thread.
@stevenharman
Copy link
Contributor Author

FWIW, the TravisCI build is failing due to coveralls, but the tests all pass.

@jimmycuadra
Copy link
Contributor

I see a test failure. Did it pass locally?

@stevenharman
Copy link
Contributor Author

Doh, sorry about that. Yes, it did pass locally.

It's been a while since I've worked with EventMachine, but my suspicion is there's a race condition that this change might be triggering. It seems to fail locally about 1 in 3 runs. Any ideas where I should look?

We need to be sure the event loop is configured and starting before
trying to defer a block. If deferred outside the #run block, we can get
`a eventmachine not initialized: evma_signal_loopbreak` error if we try
to stop the event loop before it's started.

We might also consider stubbing and yielding the `EventLoop.defer`
method in a general `before` block for this entire set of specs, to make
sure we're not accidentally deferring work we don't intend to.
@stevenharman
Copy link
Contributor Author

Under Ruby 2.0, I now see a new error coming from EventMachine: eventmachine not initialized: evma_stop_machine. On that version of Ruby, a spec also prints the following to STDOUT: Attempt to unlock a mutex which is locked by another thread.

On Ruby 2.1 and 2.2 the tests pass, every time. And I don't see the mutex message printed to STDOUT. I've added Ruby 2.1 and 2.2 to the TravisCI build matrix so we can reproduce the error on CI.

Update: Locally, this error seems to happen ~ 1 of every 10 times I run the isolated spec.

@jimmycuadra
Copy link
Contributor

Frustrating. I won't have time to look at it myself for a few days, but I will report back with anything I figure out once I do.

@stevenharman
Copy link
Contributor Author

Interestingly, stubbing the EventLoop::defer call (allow(Lita::Adapters::Slack::EventLoop).to receive(:defer)) seems to allow the test to pass, every time.

@jimmycuadra
Copy link
Contributor

That makes sense, since that removes the asynchronousness from the equation.

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.

2 participants