Server: Asynchronous AMCP

[jesperstarkar]

AMCP being asynchronous, we need a way of identifying responses as they get back. FIFOin commands out of the client(s) just takes too long time.

2.1 proposed a REQ/RES syntax:
https://github.com/CasparCG/server/blob/e0933dc7f65c15a8122c97e64f38cb3afced91e0/src/protocol/amcp/AMCPProtocolStrategy.cpp#L171

https://github.com/CasparCG/server/blob/82430b96342f803e8e8b17a83291c9730bc05535/protocol/amcp/AMCPCommandsImpl.cpp#L2998

I don't know if this is the best approach, but to me, this syntax is brilliant - being backwards compatible.

[jesperstarkar]

Ref: #475, #568

[ronag]

I don't understand this... at all... also as far as I can see I haven't modified or removed this. The AMCP code needs an overhaul which is in the future.

[jesperstarkar]

Sorry! My bad.

[ronag]

I don't think it works and should be removed for something else along the same lines but implemented differently.

[ronag]

Sorry, but basically what you are asking for is something like AMCP 3.0 with support for asynchronous commands. It's been discussed with a positive response with a roadmap sometime in the future :).

[jesperstarkar]

Well, the REQ/RES syntax worked very well, and it is AMCP 2.x compatible. Making a breaking 3.x change for AMCP seems rather drastic, if we don't start a job of overhauling the complete protocol, looking into more advanced features like macros/shceduling etc.

So in my opinion, keeping REQ/RES for >2.2 makes perfect sense, and is a simple and really powerful solution to a major problem.

[ronag]

@jesperstarkar goes back to the question whether we should be implementing hacks to solve short term issues or not...

The problem here is that REQ/RES won't actually make any difference since AMCP still needs to execute all the commands in FIFO order. It's just an illusion. You already have command ids... it's the index of the command i.e.

REQ 1 PLAY 1-1rnREQ 2 PLAY 1-2rnREQ 3 PLAY 1-3rnRES 1rnRES 2rnRES 3rn
OKrnOKrnOKrn

achieves the exact same thing as

PLAY 1-1rnPLAY 1-2rnPLAY 1-3rn
OKrnOKrnOKrn

You need to fundamentally change how AMCP works for there to be any use, i.e. commands need to be able to execute out of order and then you need some way to enforce barriers for commands that actually need to be executed in order.

It's a much more complicated problem than one would initially think.

*EDITED: @jesperstarkar is actually right if the commands go to different channels as my original example did.

[jesperstarkar]

I don't think that's so Robert? There's no guarantee that responses come back in the same order as you sent them?

For years I thought the response order was guaranteed, and worked out from that. Until I understood it wasn't so, doing extensive testing implementing my connection library. @HellGore confirmed that you can't rely on the order of responses. Hence the REQ/RES implementation, which I wouldn't call a hack.

[ronag]

There's no guarantee that responses come back in the same order as you sent them?

Yes, there is. There are however several queues depending on some rules, e.g.

LOAD 1-1
LOAD 1-2
PLAY 1-1
PLAY 1-2

Are guaranteed to execute in order.

While

LOAD 1-1
LOAD 2-1
PLAY 1-1
PLAY 2-1

Are not since each channel has it's own amcp queue.

Hence the REQ/RES implementation, which I wouldn't call a hack.

I would :). Sorry.

You can achieve the same thing MUCH easier on the client side by making one connection for each AMCP queue. That's what we do at nxtedition, i.e. one connection for each channel and then one connection for query commands.

[jesperstarkar]

If you have multiple queues, unique pr. channel, the protocol is asynchronous by nature. 200 PLAY OK doesn't say which channel it executes on, so the client can't know which internal queue to move along.

What you call hack I call a graceful retro-fit for a problem, maintaining full backwards compatibility.

But hey, it seems to still be in there, and as long as it stays (or similar functionality), I'm happy!

[ronag]

If you have multiple queues, unique pr. channel, the protocol is asynchronous by nature. 200 PLAY OK doesn't say which channel it executes on, so the client can't know which internal queue to move along.

If you create one connection for each channel then you do.

[jesperstarkar]

Good point. But that smells more of an hack than REQ/RES to me? Subjectively of course.

Do you want to actively remove REQ/RES?

[ronag]

If you (for some reason don't want to have multiple connections). A much more graceful solution would be to simply send the queue index in all responses.

[ronag]

Do you want to actively remove REQ/RES?

Yes, it's a hack. And there are other hacks that don't involve server code if you need the functionality near term.

[ronag]

Just as a general note. We want to keep the server as simple as possible. If we can move complexity to client that's going to be preferred. There are not that many people that currently can maintain the server... We need to focus our efforts.

[jesperstarkar]

I just need a way of identifying responses. if there's no other options left other than having different connections, that's the way going forward. And I still need to FIFO all non-channel-related commands.

[ronag]

@jesperstarkar maybe we should have this in some wiki, readme, faq or best practice thing?

[ronag]

@jesperstarkar actually, I hadn't realized how big of an issue this is... you basically have to use multiple connections or the AMCP protocol is semantically broken since it's not enforcing total order. There are a lot of broken clients out there that assume total order... including the official client I believe @dotarmin? @dotarmin could you bring this up with Niklas P?

[jesperstarkar]

Does that mean that internally, with the 2.1 implementation of REQ/RES, you still can't guarantee matching responses with requests? Because, if REQ/RES can guarantee it, I will again (maybe for the last time, but no promises) strongly suggest, to not remove that 5-line-or-so implementation.

[ronag]

I'm sorry to say... I have no idea... I don't understand the implementation... I won't be touching it for now...

[ronag]

In theory REQ/RES should do what you think it does and work like that. I would still recommend not using it.

[jesperstarkar]

Let it stay, educate people of using it, and case closed?

[jesperstarkar]

There are a lot of broken clients out there that assume total order... including the official client I believe @dotarmin? @dotarmin could you bring this up with Niklas P?

From what I know, SVT tends to FIFO everything out of a single queue (the java and as3.0 libraries does so). Don't know if the official Client does.

If the client sends a single command at the time, waiting for response until the next one, everything works - of course. But it is slow, really slow.

[ronag]

It stays for now.

[ronag]

If the client sends a single command at the time, waiting for response until the next one, everything works - of course. But it is slow, really slow.

Hm, good point.

[5opr4ni]

@jesperstarkar No the CasparCG client don't. When I used the java lib I removed that silly thing, the same for the AS3 lib. It was a looooong time since the need for that was the case. Haven't you tried the regular client.
In the normal client at FORUM I stack over 100 lines in one group, and there are no problem at all.
Choff it says! I think there is a thread about that in the forums.
I can agree that it should have made its way to the wiki.

[jesperstarkar]

@5opr4ni Sending commands isn't the problem. Problem is receiving all responses and keeping track of every response code to each request.

[5opr4ni]

Responded to this:

From what I know, SVT tends to FIFO everything out of a single queue (the java and as3.0 libraries does so). Don't know if the official Client does.

If the client sends a single command at the time, waiting for response until the next one, everything works - of course. But it is slow, really slow.

[5opr4ni]

@jesperstarkar Are we missing something that you do, that we don't. We do use CasparCG heavily where I work.
;)

[jesperstarkar]

@5opr4ni This problem caught my attention in 2015, and I discussed it with Helge at the time. He told me the only way to maintain sync between requests and response was to send a single command at the time. He told me the Java library did so, and I looked into the AS3.0 library at the moment seeing the same thing there.

Working in-depth with the AMCP connection library, I had 3 possible strategies for command queue execution:
1) FIFO/Single queue (similar to the AS3.0 and apparently Java lib of yours – back in the days at least).
2) Fire-and-forget, similar to what you describe the Client does (in some cases).
3) Smart filtering by response code: ensuring only one command pr. possible response code family is sent at the time. This was my proposed approach until #475 was raised and swiftly implemented with full backwards compatibility by Helge.

475 allows for perfect async operation at full speed, and this issue was just meant to keep things so.

[5opr4ni]

@jesperstarkar We definitely use 2. for all our work, and has always done, except for maybe Helge and his java lib then.

1. Fire-and-forget, similar to what you describe the Client does (in some cases).

I personally have never seen the need in waiting for a response. If I should check that all commands has ben accepted and executed from Caspar and had to worry about that I would have abandon CCG a long time ago.

But at the same time I can understand that there are other kind of people that has other needs.

I think you simplify the ease of implementing this in a solid way, we have decided to build a rock solid CCG and then all function has to be that. One way for you at the moment is to do as Nagy suggest, and establish different work on different sockets.

After this we can start a new discussion about how we want the AMCP protocol to be in the future.

/olle

[Julusian]

So in my opinion, keeping REQ/RES for >2.2 makes perfect sense

No matter what the opinions are on REQ/RES, I really think it needs to stay for 2.2, as it is rather late in the dev cycle to make a breaking change that will kill most of the 3rd party client libraries that have released a version for 2.1. If it is to removed, I strongly feel that it should happen early on in 3.0.

Yes, it's a hack

@ronag why do you think it is a hack? I have seen other async protocols that use a nonce for identifying responses, and for the user this is a simple to understand and an optional method of doing so. It also means that the AMCP can support true async commands without a breaking protocol change.
How would you prefer to identify responses in a true async AMCP?

You can achieve the same thing MUCH easier on the client side

It also really isn't much code to implement this. Theres a couple of lines in /src/protocol/amcp/AMCPProtocolStrategy.cpp to pull the tokens off the stack, and then a couple in /protocol/amcp/AMCPCommand.h to add it back to responses, it is not done in a very nice way and could easily be made immutable and such, but it really isnt much code to maintain.
Also I wouldn't call it easier to do client side, as then the client has to maintain multiple connections which isn't difficult but neither is the amount of code taken to do it in the server.

There's no guarantee that responses come back in the same order

@jesperstarkar From what I have seen from adding a true async response, then within a queue it is guaranteed to be the same order. I haven't checked what has seperate amcp queues, but if your commands span multiple then it is likely that order will be a problem.

@ronag on a related note, does this not make doing a SWAP between channels unpredictable? as if you run PLAY 1-10 AMBrn PLAY 2-10 AMBrn SWAP 1-10 2-10rn then depending on how busy the channel 2 queue is, that second play could happen post swap occuring? If that is correct I havent verified it) then it is probably worth noting this on the wiki.

I personally have never seen the need in waiting for a response.

I normally don't listen to the response codes, but I can see times when it is useful to. One of my control programs completely falls over if it fails to load an asset (depending on which one failed to load), as it will trigger things on a certain frame of a video being hit, and if that video is defined as a runtime variable, then it is possible that it could be missing and cause the timeline to completely stall midway.
It is also useful during dev, to be able to easily identify which command returned an error when you send multiple. Sure you can count (unless you are using the system log and multiple channels), but matching an id is more reliable and means you can search

[ronag]

No matter what the opinions are on REQ/RES, I really think it needs to stay for 2.2,

It's going to be flagged as deprecated for removal in future release. I don't want to touch AMCP in this release.

but it really isnt much code to maintain

I disagree.

Also I wouldn't call it easier to do client side, as then the client has to maintain multiple connections which isn't difficult but neither is the amount of code taken to do it in the server.

Again I disagree. If something can be achieved with reasonable effort in a client then it has in my opinion nothing to do in the server.

@ronag why do you think it is a hack?

It's complicated. My main concern is the implementation which is not tracking dependencies properly, i.e. it allows some things out of order which it shouldn't and a lot of things that could be out of order aren't. I haven't fully analyzed this, e.g. the SWAP example.

It's both slower than it could be and has a lot of edge cases which can cause weird, intermittent failures and can be implemented in clients achieving the exact same functionality (with the same problems).

Also, there is no way to perform proper transactions, batching and cancellations which I would expect from an asynchronous API.

How would you prefer to identify responses in a true async AMCP?

I don't know how I would like this to work at the moment. It requires a lot of thought which I don't have time for now.

[ronag]

I guess one could do this similar to OpenGL and simply have a fence, sync and barrier AMCP API which would move the whole dependency tracking to the client... but that would make clients A LOT more complicated and would need a lot of knowledge about how commands are implemented internally.

[Julusian]

It sounds like you are talking about the protocol implementation in general, whereas I was referring specifically to the REQ/RES syntax. I know that the current architecture of the commands isn't great especially when it comes to wanting real async commands (eg 79ac76ffe19a761a7eff750f44ce96bb06ba0e38) so am definitely onboard with an overhaul of the internals of AMCP in Caspar.

Also, there is no way to perform proper transactions

Yes, transactions and more would be good, but I don't think it has to break the current API.

It requires a lot of thought which I don't have time for now.

Seeing as it isn't going to change this release, I am happy for a discussion to happen nearer to the time it will be implemented.