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

Improve live blog bandwidth use #13458

Merged
merged 4 commits into from
Jul 4, 2016
Merged

Conversation

johnduffell
Copy link
Member

When a live blog is running, clients request updates every 5 seconds using an ajax url.

To do this, they ask for all blocks since the last block that was loaded onto the screen, and if the user reads those blocks (click toast or scroll up) then it uses the new latest block as the last block. This endpoint, like the live page itself, has a 5 second cache time regardless of which block is requested.

This causes a great deal of cache splitting, every extra block splits the cache again. With the 5 second cache time, this gets expensive. With multiple liveblogs running, if they're big, we're wasting resources.

This PR is the first (of probably 3) to solve the problem.

Part 1: this part: arguably a bug fix - make the clients always request from the last block they retrieved, and not wait for the user to read the block before using it. This will save the user repeatedly downloading the same blocks every 5 seconds, and possibly save us a little on not having to serve as many old blocks.
Part 2: I will make the cache time of anything other than the latest block (i.e. empty response) to be Very Long. This means requests for one block only (the latest) will be getting to the backend every 5 seconds, the others will all be left in the cache. The clients will only get one block at a time, so they'll have to poll again immediately after any successful update. This will take most of the requests off the backend. Note: if editorial add a block, and then delete it, we might get stuck in the past. The purger only clears theguardian.com, not api.nextgen. Actually, even now, deleting blocks would mess things up. Editing in general is basically not supported with auto upates. The timeline isn't auto updated either by the looks 😦
Part 3: finally 😊 use @cb372 's https://github.com/guardian/content-api/pull/1502 and only get the blocks we need for the endpoints that are only cached for 5 seconds. This will make the requests that we do process every 5 seconds much cheaper for us and capi.

@alexduf @TBonnin @JustinPinner comments comments comments!

val allPagesJson = Seq(
"timeline" -> timelineHtml,
"numNewBlocks" -> newBlocks.size,
"mostRecentBlockId" -> page.article.fields.blocks.headOption.map(block => s"block-${block.id}").getOrElse("block-9")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is block-9?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a block it definitely won't find in the list, that would only happen if a liveblog was somehow published without any blocks (which isn't possible). If we're looking since a certain block and it doesn't exist, we just return them all.
Having said all that, maybe it could be clearer. I'll take a look.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh and the 9 was a typo it was supposed to be 0, but block 9 is an area at glastonbury so I thought I'd leave it ;)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about block-does-not-exist? 🎯

@johnduffell
Copy link
Member Author

ok I've taken out the default value @TBonnin as it didn't make a lot of sense to force it

@johnduffell
Copy link
Member Author

is that a +1 for the whole PR or just the block change?

@alexduf
Copy link
Contributor

alexduf commented Jul 4, 2016

It looks good to me, unless @TBonnin has another comment 👍

@johnduffell johnduffell merged commit e396fb5 into master Jul 4, 2016
@johnduffell johnduffell deleted the liveblog-remember-blocks branch July 4, 2016 14:41
@TBonnin
Copy link
Contributor

TBonnin commented Jul 4, 2016

Sorry I forgot to give you a 👍
Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants