-
Notifications
You must be signed in to change notification settings - Fork 10
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
Outbox latency improvements #60
base: outbox
Are you sure you want to change the base?
Conversation
… record is added to the messages table, the producer is notified
There is one concern I had in the way this is implemented, specifically around interrupting the Thread from sleeping. Is it safe to assume any other part of the code executed by the MM Producer will not have any sleep anywhere? I'm afraid we might inadvertently interrupt a sleep that is not the 2 second sleep. Say, if there was a sleep in bunny, for example. |
re: postgres NOTIFY/LISTEN benchmarks, I just ran some quick/dirty ones on a local postgres instance with rails console -> postgres. Since it was over localhost, network delay was close to 0ms.
Given that I don't think any of our services come even close to that 6000 notifies per second, I'm of the opinion that the performance is perfectly acceptable. That said, since NOTIFY works as more of a multicast than it does rabbitmq (for every incoming message, it sends a copy to ALL listeners, instead of round robin'ing the messages), this is not scalable once you hit that limit. There would be no way to horizontally scale this solution. Given that it would have to be an incredible load to produce that many messages, is that even a worry? |
@@ -14,6 +14,11 @@ def self.push_record(record, operation, options) | |||
set_name: MultipleMan::RoutingKey.model_name(routing_key) | |||
).save! | |||
end | |||
|
|||
after_save do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in case anyone else has the same thought, I just looked up this:
Firstly, if a NOTIFY is executed inside a transaction, the notify events are not delivered until and unless the transaction is committed. This is appropriate, since if the transaction is aborted, all the commands within it have had no effect, including NOTIFY.
@@ -11,6 +11,12 @@ def run_producer | |||
require_relative '../outbox/db' | |||
require_relative '../outbox/message/sequel' | |||
|
|||
@producer_thread = Thread.new { producer_loop } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not a threading expert, but what happens if this producer_loop throws an exception? I'm just thinking about db connections, notifications etc
This MR covers two main performance improvements:
Assume that if there was at least one record in the messages table last time, that there will be more this time. So, don't sleep and immediately try and get those message(s) from the db. If there were no messages in the table, sleep per usual until the next polling time.
This will mainly help when the table gets backed up with many messages, so we aren't waiting ~2 seconds to get the next set of data from the messages table.
To get around the delay associated with polling, I see only a few options.
As is below, I implemented NOTIFY/LISTEN such that: