RIP Celluloid, Hello concurrent-ruby/Thread pool #291
Conversation
|
@mariokostelac this is what I was talking about. I do need to review all tests, but so far, it's working, only with a simple ThreadPool + Atomic Integer and Boolean. Could you have a look at it? |
| module Shoryuken | ||
| module Middleware | ||
| module Server | ||
| class AutoExtendVisibility | ||
| EXTEND_UPFRONT_SECONDS = 5 | ||
|
|
||
| def call(worker, queue, sqs_msg, body) | ||
| timer = auto_visibility_timer(worker, queue, sqs_msg, body) | ||
| # timer = auto_visibility_timer(worker, queue, sqs_msg, body) |
There was a problem hiding this comment.
I still need to figure out how to implement this.
Hard shutdown needs to use `sleep` IO blocking not `after`
That was way easier than I expected. |
calls from processor_done, which can be called from multiple threads
|
@mariokostelac I'm adding some "performance" results in the PR description 🍻 |
lib/shoryuken/fetcher.rb
Outdated
| limit = available_processors > FETCH_LIMIT ? FETCH_LIMIT : available_processors | ||
|
|
||
| sqs_msgs = Array(receive_messages(queue, limit)) | ||
| logger.info { "Found #{sqs_msgs.size} messages for '#{queue.name}'" } |
There was a problem hiding this comment.
As an aside, have you considered not emitting this when the sqs_msgs are empty?
There was a problem hiding this comment.
@tjsingleton I haven't, but it seems to be a good idea. The only one issue is because the process will may look like isn't running/moving without that message, in case all queues are empty. WDYT?
There was a problem hiding this comment.
I thought about this for awhile and as long as the process lifecycle has good log messages, I don't think it's typically expected to "heartbeat" via the log. For instance, do you trust that puma is up and processing requests despite that it's doesn't log?
|
@tjsingleton makes sense. Done
93bb041
…On Thu, Jan 12, 2017 at 4:37 PM, TJ Singleton ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In lib/shoryuken/fetcher.rb <#291>:
> - logger.debug { "Fetcher for '#{queue}' completed in #{elapsed(started_at)} ms" }
- sqs_msgs
- rescue => ex
- logger.error { "Error fetching message: #{ex}" }
- logger.error { ex.backtrace.first }
- []
- end
+ started_at = Time.now
+
+ logger.debug { "Looking for new messages in '#{queue}'" }
+
+ begin
+ limit = available_processors > FETCH_LIMIT ? FETCH_LIMIT : available_processors
+
+ sqs_msgs = Array(receive_messages(queue, limit))
+ logger.info { "Found #{sqs_msgs.size} messages for '#{queue.name}'" }
I thought about this for awhile and as long as the process lifecycle has
good log messages, I don't think it's typically expected to "heartbeat" via
the log. For instance, do you trust that puma is up and processing requests
despite that it's doesn't log?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#291>, or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAGctAFJkdXdWJ2l8QCU9b_4ly1bdC6qks5rRnLWgaJpZM4LQU1y>
.
--
Pablo Cantero
pablocantero.com
|
Fixes #306 See also: #291 (comment)
Fixes #306 See also: #291 (comment)
|
Congrats on getting this merged! 👍 |
|
@tjsingleton thanks! I merged it to a |
|
Great news! Thank you. |
I think Shoryuken can be simplified, it feels like just a Thread pool would be enough for Shoryuken, currently it's completed inspired by Sidekiq, but thinking more on it, doesn't it feel like just a manager that calls fetcher then calls processors (thread pool) should be enough?!?!
Fixes #185
Related to #280, #88
Time to process 1k messages/PutsReq worker