bbc / alephant-broker

Brokers requests for alephant components
MIT License
4 stars 3 forks source link

Loading components in parallel (Request::Batch) #16

Closed revett closed 10 years ago

revett commented 10 years ago

Problem

This functionality was removed within https://github.com/BBC-News/alephant-broker/pull/14 as we were seeing errors, most likely due to race conditions. It may also be due to using Peach which is now quite an old gem.

Solution

Refactor Request::Batch so that the following method loads components in parallel.

def components_for(env)
  env.data['components'].map do |c|
    @component_factory.create(
      c['component'],
      batch_id,
      c['options']
    )
  end
end
darnould commented 10 years ago

The S3 loading strategy is not thread-safe. Here's a snippet showing why:

        def initialize
          @cached = true
        end

        def load(id, batch_id, options)
          @id, @batch_id, @options = id, batch_id, options
          create_component(cache_object)
        rescue
          create_component(
            cache.set(cache_key, retrieve_object)
          )
        end

A single strategy object is used by multiple components, and holds mutable state (id, etc.) peculiar to the particular component being loaded at the time. Sequentially, there is no problem: but parallelised, the shared mutable state between the components get confused.

Here is a brief clip explaining how this works: https://pbs.twimg.com/tweet_video/BzCPLp5IIAArEb1.mp4

revett commented 10 years ago

This was re-implemented and fixed in https://github.com/BBC-News/alephant-broker/pull/14.