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.

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

middleware tracing #5

Closed
wants to merge 116 commits into from
Closed

middleware tracing #5

wants to merge 116 commits into from

Conversation

benlangfeld
Copy link
Member

  • These methods can all be private
  • Three ways to close a client socket are unnecessary
  • Documents the MessageBus::Client interface
  • ConnectionManager#stats is unused
  • Documents ConnectionManager
  • Much of MessageBus::Rack::Middleware can be private
  • Documents MessageBus::Rack::Middleware
  • Much of the diagnostics implementation can be private
  • Documents diagnostics interface
  • Puts existing documentation in the right place
  • These methods can be private
  • Documents MessageBus::Implementation interface
  • Use Client#close rather than Client#cancel
  • don't start message bus in rake tasks
  • fix crashes when destroying message bus
  • Makes this an opt-in change
  • Formatting
  • Test without mocks
  • Allow destroying a bus that never had a backend
  • Raises rubocop limit on Metrics/ModuleLength
  • Simple usage documentation
  • Keep a running changelog of changes on master since the last release
  • Mention changelog entries in contributing docs
  • FIX: stop leaking redis keys used for tracking per channel id
  • release 2.2.0.pre
  • FIX: private method should not be invoked on self
  • Re-generate rubocop todo list
  • Inherit rubocop off discourse
  • Generate backend-specific spec tasks automatically
  • Benchmark publication performance
  • Run variations on publish benchmark
  • Notes on performance tests in the README
  • Make it easy to manually test the diagnostics UI
  • Re-writes diagnostics UI using React
  • More diagnostics documentation
  • FEATURE: Ruby MessageBus HTTP client.
  • Release 2.2.0.pre.2.
  • Specify units for Ruby HTTP Client configs.
  • FIX: Ruby HTTP Client not backing off polling on error.
  • Release 2.2.0.
  • update README
  • on_middleware_error only for internal errors
  • Removes trailing whitespace
  • Tests for handling of downstream errors
  • bump version for release
  • Print the full stack when message bus callback fails.
  • Fix to make the is_readonly? method compatible with the redis gem both pre and post v4.0 when the client attribute was removed - closes Use of Redis#client method which was removed in redis v4.0 discourse/message_bus#208
  • Run prettier on file so it confirms to a consistent style
  • FEATURE: support status 429 from server and Retry-After header
  • FIX: Protect Redis config from being manipulated
  • Update changelog and bump version
  • FIX: remove deperecated Minitest global assertions
  • FIX: Magic comment to satisfy rubocop
  • FIX: Code adjustment to unfreeze strings
  • Revert "FIX: remove deperecated Minitest global assertions"
  • FIX: Use minitest-global_expectations
  • README.md: Doc adding extra response headers
  • README: Link to generated docs
  • FEATURE: add optional shouldLongPollCallback
  • bump version
  • DEV: Add missing Rubocop gem dependencies due to changes in discourse config
  • DEV: Fix broken rubocop rules
  • Run tests against Ruby 2.6 on CI.
  • Support for Ruby 2.3 has ended
  • FIX: Don't publish message to intersection of user_ids and group_ids
  • DEV: cut a new release
  • FEATURE: MessageBus#register_client_message_filter
  • DEV: bump version
  • FIX: compatibility with Rails 6.0.3
  • README: Add missing date to CHANGELOG
  • FEATURE: MessageBus.base_route
  • Use start_with to check route
  • FIX: Remove regex, use substring
  • Memoize the bus routes
  • Clean up Javascript (Clean up Javascript discourse/message_bus#225)
  • FEATURE: Support UMD and NPM (FEATURE: Support UMD and NPM discourse/message_bus#226)
  • Version bump for mew UMD js
  • Add npm publish to release task
  • Update Changelog
  • FIX: Avoid leaking Redis connections. (FIX: MessageBus backend can leak Redis connection. discourse/message_bus#229)
  • Release 3.3.1.
  • FIX: Disconnect Redis conn when rescuing errors in global subscribe.
  • Bump version to 3.3.2.
  • Bump version to 3.3.2.
  • Revert release of 3.3.1 and 3.3.2.
  • Follow up to c0ad280.
  • Release 3.3.1.
  • PERF: optimize performance of message generation
  • remove confusing comment
  • FEATURE: raise when attempting to publish to invalid targets
  • BREAKING Make JS client throw if lastId not number
  • Prettify error backtrace when logging.
  • Log when DistributedCache encounters an error when publishing.
  • Release 3.3.2.
  • Add Ruby 2.7 to Travis and remove Ruby 2.4.
  • Fix failing specs due when stopping MessageBus::TimerThread.
  • FIX: queue_in_memory option not being passed to the backends.
  • FIX: MessageBus::DistributedCache#publish should raise on error.
  • Add missing changelog for e91d414.
  • FEATURE: Add publish_queue_in_memory option to distributed cache manager.
  • Release 3.3.3.
  • FIX: Remove trailing comma incorrectly added in ec60d88.
  • Release 3.3.4.
  • DEV: removes dead code
  • FEATURE: Drop support for ruby 2.3 (FEATURE: Drop support for ruby 2.3 discourse/message_bus#239)
  • DEV: Switch from Travis to Actions (DEV: Switch from Travis to Actions discourse/message_bus#240)
  • DEV: Fix Rubocop runs on Github Actions (DEV: Fix Rubocop runs on Github Actions discourse/message_bus#241)
  • FIX: force a poll more consistently when visibility changes
  • Remove live demo, it has been removed
  • Allow tracing the performance of middleware

benlangfeld and others added 30 commits November 27, 2018 18:27
* turn off message bus by rails railtie when running rake
* don't initialize middleware subscribed when off
* method to detect if message bus is off
* do not crash when calling after_fork on destroyed bus
* do not crash on calling destroy twice
This puts the burden of writing change-logs on the contributor rather than the release manager, reducing the effort to cut a release and improving the accuracy of the changelog. It also means that changelog entries from now on are more likely to be useful in git blame when looking for the actual change that produced them.
tgxworld and others added 29 commits June 8, 2020 10:27
Pending permission to bump gem.
```
# frozen_string_literal: true

require_relative '../rerunner/rerunner'
require 'benchmark/ips'

def append(global_id,message_id,channel,data)
  global_id.to_s << "|" << message_id.to_s << "|" << channel.gsub("|", "$$123$$") << "|" << data
end

def interpolate(global_id,message_id,channel,data)
  "#{global_id}|#{message_id}|#{channel.gsub("|", "$$123$$")}|#{data}"
end

if interpolate(1,2,"a|b","data") != append(1,2,"a|b","data")
  raise "bad implementation"
end


Benchmark.ips do |x|
  x.warmup = 1
  x.time = 10

  x.report("<<") do |times|
    while times > 0
      append(1,2,"three","four")
      times -= 1
    end
  end

  x.report("interpolate") do |times|
    while times > 0
      interpolate(1,2,"three","four")
      times -= 1
    end
  end

  x.compare!
end
```

```
Calculating -------------------------------------
                  <<      2.254M (± 1.0%) i/s -     22.619M in  10.036563s
         interpolate      2.398M (± 0.9%) i/s -     24.143M in  10.067638s

Comparison:
         interpolate:  2398259.8 i/s
                  <<:  2253830.6 i/s - 1.06x  slower
```

Also fixed discourse#227
When a user attempts to publish to a an empty group/user or client id list
odds are a mistake happened.

Erring on the side of safety we should publish nothing, previously everyone
would get the message.
The thread may be dead before `Thread#wakeup` is called.
On the redis backend, any errors encountered during `MessageBus#publish`
will add the message into an in memory queue and silently swallow the
error. While this is behavior is OK for normal message_bus usage, it may
lead to inconsistency when using `DistributedCache`. If a process
doesn't publish successfully to another process, it will still update
its in memory cache leaving the other processes unaware. As such, the
distributed cache is out of sync and will require another successful
write to the cache to resync all the caches.
…nager.

This is a partial revert of e91d414.
Instead of changing the default behavior of what
`MessageBus::DistributedCache::Manager#publish` should do when it is
called on a Readonly redis, this commit introduces an option to change
the behavior.
Also added test to prevent this regression.
Previously a visibility change was only inconsistently forcing a poll.

This happened cause "delayPollTimeout" may have been in effect during the change.

This should make the polling after returning to a tab more consistent. However we may go even a bit further later on by binding to scroll or mouse click.
turns out that unmoderated public chat is a bad idea.
In a consuming application, when debugging slow responses to subscribing clients, it is useful to know where time is being spent in MessageBus middleware. This functionality allows hooking in a consumer-application-specific tracing mechanism, such as [NewRelic's `#trace_execution_scoped`](https://www.rubydoc.info/github/newrelic/newrelic-ruby-agent/NewRelic/Agent/MethodTracer#trace_execution_scoped-instance_method).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
10 participants