bbc / alephant-broker

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

Cache Dynamo Read/Write #51

Open revett opened 9 years ago

revett commented 9 years ago

Caching Dynamo read/write may help with hitting limits with Dynamo. Had issues in Elections.

This should be a very small cache time.

Now part of: https://jira.dev.bbc.co.uk/browse/CONNPOL-2821

samfrench commented 8 years ago

dynamodb cache

I have 2 options:

I think the simplest option is to reuse the Cache.rb object in the Alephant-Broker. At the moment, the Cache object is in front of S3 to store components. A block is passed to this, which gets executed if the component is not in the cache.

I could use this for the Lookup and the Sequencer to look in the Cache first, and if not, then run a block to fetch the item from Alephant-Lookup, or Alephant-Sequencer as it currently does. The additional caches will all be independant, so they each get items from the cache or read from Dynamo separately. There will be no dependency between the Lookup cache and the Sequencer cache.

I also need to add a method to set the TTL of the Cache.rb object for the new instances. At the moment it either uses Cosmos config or the default of 30 days.

Integralist commented 8 years ago

Hi @samfrench, I agree that option 1 indeed seems the easier solution, but option 2 is (IMO) a better, less coupled, design which helps keep a separation of logic/behaviour to the respective gems. So I personally would probably have gone for option 2.

Unless @zygotic99 has any strong opinions about the placement of the code itself (I also assume the potentially obvious, which is that the cache cluster will be shared and not a new ElastiCache setup just for sequencer/lookup).

samfrench commented 8 years ago

For option 2, I was thinking of creating an Alephant-Cache object, which is probably just the Alephant-Cache broker.rb file, moved into a gem, to be reused by the other gems.

But, there is already an Alephant-Cache gem (soon to be renamed Alephant-Storage, for S3 abstraction...), so I thought that would get even more confusing with the naming. If option 2 was to be used, I'd have to come up with another name.

It would be worth doing it properly, as it's not too much more time, as I'd be writing tests and just moving the caching to another gem (I assume). But, it depends if the next version of Alephant would have all of these features, and then I'd be writing something to be thrown away.

bbc-fossbot commented 8 years ago

Sorry for the stupid gem name. I think originally the intention was to do something like Sam describes but I backed off from the renaming exercise. On Tue, 16 Feb 2016 at 08:44, Sam French notifications@github.com wrote:

For option 2, I was thinking of creating an Alephant-Cache object, which is probably just the Alephant-Cache broker.rb file moved into a gem, to be reused by the other gems.

But, there is already an Alephant-Cache gem (soon to be renamed Alephant-Storage, for S3 abstraction...), so I thought that would get even more confusing with the naming. If option 2 was to be used, I'd have to come up with another name.

It would be worth doing it properly, as it's not too much more time, as I'd be writing tests and just moving the caching to another gem (I assume). But, it depends if the next version of Alephant would have all of these features, and then I'd be writing something to be thrown away.

Reply to this email directly or view it on GitHub https://github.com/BBC-News/alephant-broker/issues/51#issuecomment-184577808 .

kenoir commented 8 years ago

@Integralist you've probably already thought it through, but i'd be careful about caching the get_last_seen call as you will need to be sure it really is the last seen to avoid getting out of order components.

BBCMarkMcDonnell commented 8 years ago

@kenoir yup :-) it's definitely a bit of a concern

kenoir commented 8 years ago

Is simply scaling Dynamo read/write provision for an event not feasible?

samfrench commented 8 years ago

@zygotic99 mentioned yesterday that he wondered if the sequencer and lookup should be cached in-sync. (I don't this is his exact wording!).

If the sequencer was in the cache, then would we expect the lookup to be? I had them down as being cached independently of each other.

If the sequencer was in the cache, and the lookup wasn't, then we would fetch the lookup from the DB. The same the other way around. So I don't think this would be an issue.

samfrench commented 8 years ago

@kenoir, we would ideally get the last seen for a batch request. So all of the components from the same queue, should be in-sync with each other. Does a cache for this value, really add much more concern about getting items out of order? It just means there may be more delay in getting the newer version. The cache should be quite short, so it's not like we're expecting to wait a minute to get the next one.

We also expect to have polling, so we would update the page with newer components soon after.

samfrench commented 8 years ago

Last time, Dynamo was forgotten about, so it wasn't scaled as we expected. This work is to help with this issue, if it ever happened again...

BBCMarkMcDonnell commented 8 years ago

@samfrench

Well an atomic operation is a nice idea but I'm not sure if it's worth the hassle if we're already not concerned that the sequence in the cache is indeed the latest or not (e.g. caching the sequence inherently supports the possibility of the non-latest version being returned). But that's a product decision though as far as I'm concerned (e.g. do we absolutely want the latest version or do we want to improve dynamo performance without considering automatic throughput scaling in ddb).

re: "next version of Alephant would have all of these features" <- to be decided still :-)

BBCMarkMcDonnell commented 8 years ago

Things like https://aws.amazon.com/blogs/aws/auto-scale-dynamodb-with-dynamic-dynamodb/ could be considered in order to reduce the need for code and push the problem back into infrastructure @zygotic99

kenoir commented 8 years ago

@samfrench Afraid I can't speak to the details (been away from the code too long). Dynamo definitely needs monitoring and pre-scaling (and anything else your AWS account rep suggests).

@Integralist My concern with that project is that it would have the responsiveness to scaling demand that you require (i.e. does it catch the spike in use fast enough to scale).

Good luck with the polling!

BBCMarkMcDonnell commented 8 years ago

@kenoir exactly, needs to be reviewed and perf tested to make sure it fits our requirements

samfrench commented 8 years ago

@BBCMarkMcDonnell and @zygotic99, how long do you expect these values to be cached for? If we are talking about less than 1-2 seconds, @jimjohnsonrollings has said that he expects this to may not be the latest value and is fine with it. We would get the latest value when the cache value expires. He also pointed out that we were doing this, because we would have the private broker and the public (aka polling) broker hitting the DynamoDB this time. So the cache was to help with the multiple brokers reading/writing from the DB.

Integralist commented 8 years ago

@samfrench OK. Understood.

I'm not sure what work is involved with that dynamic dynamo setup, so maybe this would be a quicker short term win if product (Jim) is happy with what's been explained to him with regards to 'latest version'

samfrench commented 8 years ago

As agreed:

Non caching work, i.e. Alephant-Storage: