Engine Interface optimisations

[banks]

With #52 we improved throughput enough to keep up with our publish volume for now.

However publishing messages that are almost all not actually delivered to anyone nr stored (thanks to #51) is using a lot of CPU and causing delivery latency to be slower than it could be in some cases.

To fix this I suggest we consider a few different optimisations:

1. remove separate addHistory method and instead pass history options to publish().
   • this will allow redis engine to implement publish + addHistory as a lua script for example which will require only a single round trip to redis per publish call - even with the drop_inactive optimisation.
2. support publishing a batch of messages (possibly each to different channels) at once
   • engines that don't have a way to optimise this can just call publish in a loop and decide on semantics for partial failure (broadcast API call already has same problem)
   • redis engine can make use of pipelining for example to send all the request and then reap them without waiting for synchronous round-trip for each one.

Batch publishing in 2. will require changes further up the stack though - the application itself will need to support batch publish API requests (should we make new API call? Or breaking change to current API params?).

This is becoming more important as we increase publish load and see CPU usage climb.

[FZambia]

Could you explain what you mean under:

the application itself will need to support batch publish API requests (should we make new API call? Or breaking change to current API params?

I supposed that this must be transparent for applications i.e. apps just send the same requests as before but they batched in Centrifugo before publishing. Seems I miss something?

[banks]

I meant that for it to be useful, batching must be exposed right through to the client API. Even if we only choose to expose it via redis API, right now that just makes internal calls to API commands. I think we should support it on http API too though as it will make that much more powerful for medium to high throughput cases.

I don't think backward incompatible changes are the best way though, I was just pointing out the options.

My vote would be to make a new API command called 'publish_batch'. Internally we can just make existing publish and broadcast commands use the internal batch publish method.

Make sense. Apologies I wasn't clear.

On 30 Dec 2015, at 22:45, Alexandr Emelin notifications@github.com wrote:

Could you explain what you mean under:

the application itself will need to support batch publish API requests (should we make new API call? Or breaking change to current API params?

I supposed that this must be transparent for applications i.e. apps just send the same requests but they batched in Centrifugo before publishing. Seems I miss something?

—
Reply to this email directly or view it on GitHub.

[FZambia]

Part one done via #61

[FZambia]

Part two. Yesterday I made proof of concept showing that we can actually batch publish requests to Redis over pipeline and this makes things more performant as we save time on RTT to Redis. But the hardest part is how to seamlessly add batching support to existing API.

1. Option 1. Make redis_batch_* options - to batch all published messages in time interval (or configured amount reached).
2. Option 2. Add batch option to internal API, so only use it when needed - for example we don't really need batching when single messages come to HTTP API - because they will use different connections to Redis from pool. But we need batching when lots of messages to publish come in one HTTP request. Most probably some changes in engine will be required.
3. Option 3. Add batch option to external API, so developers can decide when they need batching. I like this option less than others because it's in general more to do for our users.
4. ?

Also we should keep the behaviour in which call to history after publish always return correct history containing messages just published. So we can't just publish asynchronously - we need a way to only return response to API client when publish has been done.

[banks]

There is another option which I think is best.

Why can't we just make whole engine async internally? I.e.:

• engine interface methods just build request object with a reply channel (like we do already for subscribe)
• they build object push it to pending chan
• one goroutine runs in a loop reading from that chan and writing to redis pipeline, then reading from pipeline and sending result on the next in-flight request object
• interface method then just waits on response chan and returns the result or error

This keeps the external synchronous interface but allows full use of pipelining with no extra config nor any trade off between latency and throughput.

If for some reason we don't want to do that then we should at least use dynamic batching rather than fixed interval. I was going to write an article about this technique as its so simple and yet is about the only design choice in IO handling space that optimises both latency and throughput at same time: http://mechanical-sympathy.blogspot.co.uk/2011/10/smart-batching.html?m=1

On 15 Mar 2016, at 08:13, Alexandr Emelin notifications@github.com wrote:

Part two. Yesterday I made proof of concept showing that we can actually batch publish requests to Redis over pipeline and this makes things more performant as we save time on RTT to Redis. But the hardest part is how to seamlessly add batching support to existing API.

1. Option 1. Make redis_batch_* options - to batch all published messages in time interval (or configured amount reached).
2. Option 2. Add batch option to internal API, so only use it when needed - for example we don't really need batching when single messages come to HTTP API - because they will use different connections to Redis from pool. But we need batching when lots of messages to publish come in one HTTP request.

Also we should keep the behaviour in which call to history after publish always return correct history containing messages just published. So we can't just publish asynchronously - we need a way to only return response to API client when publish has been done.

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub:
#59 (comment)

[FZambia]

Actually I thought about this option (I meant this as solution if choose option 1). But I thought to do this just for publish operation – do you mean that we should try to do all engine operations over pipeline - i.e. including history, presence etc?

[banks]

Didn't think too hard about it yet but I don't see why we wouldn't use the same mechanism for everything if we built it.

On 15 Mar 2016, at 10:27, Alexandr Emelin notifications@github.com wrote:

Actually I thought about this option (I meant this as solution if choose option 1). But I thought to do this just for publish operation – do you mean that we should try to do all engine operations over pipeline - i.e. including history, presence etc?

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub:
#59 (comment)

[FZambia]

Part two released with v1.4.2

@banks do you have any thoughts about what should be done in next iteration? Maybe special async flag in publish message to tell Centrifugo not to wait for response so batching could be used when messages published over publish command. So as far as I can see at moment batching in Redis engine will be used if:

• broadcast command used
• for parallel requests over HTTP API
• for parallel processing of API messages coming from different API Redis queues.

Requests like this (two publish commands in one request):

[
    {
        "method": "publish",
        "params": {
            "channel": "channel1",
            "data": {}
        }
    },
    {
        "method": "publish",
        "params": {
            "channel": "channel2",
            "data": {}
        }
    },
]

Will wait for at least 2 round-trips to Redis anyway, because Centrifugo will wait for response from publishing into channel1 before starting publishing into channel2, so sth like async option should help. But in general I personally don't need this at moment - so maybe we should wait to do this if no one needs this.

[FZambia]

As #90 merged I think we are done here (as far as I can see we've made all proposals and much more with that refactoring).