ably-cocoa icon indicating copy to clipboard operation
ably-cocoa copied to clipboard

Published 20 hours ago • verified publisher icon ably

Implement RTP18a

Open maratal opened this issue 10 months ago • 6 comments

RTP18a: The client library determines that a new sync has started whenever a SYNC ProtocolMessage is received with a channel attribute and a new sync sequence identifier in the channelSerial attribute. The channelSerial is used as the sync cursor and is a two-part identifier :. If a new sequence identifier is sent from Ably, then the client library must consider that to be the start of a new sync sequence and any previous in-flight sync should be discarded

Now library doesn't support this behavior (in-flight sync will not be discarded, instread new sync will be ignored).

This issue is a part of a bigger work (https://github.com/ably/ably-cocoa/pull/1891).

┆Issue is synchronized with this Jira Task by Unito

maratal avatar Apr 14 '24 11:04 maratal

I’m setting this as a medium priority but it’s a bit of a guess — I don't know how often the described scenario occurs in reality.

lawrence-forooghian avatar Apr 22 '24 20:04 lawrence-forooghian

I’m setting this as a medium priority but it’s a bit of a guess — I don't know how often the described scenario occurs in reality.

I agree! All other libraries are working fine without implementing this. Even if ably-js supports this spec, being single threaded, there's no issue of synchronization or multithreading unlike other languages.

sacOO7 avatar Apr 25 '24 17:04 sacOO7

I’m setting this as a medium priority but it’s a bit of a guess — I don't know how often the described scenario occurs in reality.

I agree! All other libraries are working fine without implementing this. Even if ably-js supports this spec, being single threaded, there's no issue of synchronization or multithreading unlike other languages.

There are no threading issue in this PR though (all happens in the single realtime background thread). Should I close it now or just leave it as-is?

maratal avatar Apr 26 '24 10:04 maratal

It seems like something we should implement; why would we close it?

lawrence-forooghian avatar Apr 26 '24 11:04 lawrence-forooghian

It seems like something we should implement; why would we close it?

Sorry, I've meant the PR, not clear to me what to do with it now. Shouldn't be permanently opened, right? If you can help in reviewing it I would appreciate.

maratal avatar Apr 26 '24 12:04 maratal

@maratal few points to mention

  1. Even though this spec is not implemented in other SDKs, we haven't faced any issues due to lack of it.
  2. Spec is a bit complicated and involves a very rare scenario that's unlikely to happen.
  3. If we do not implement this spec correctly, I'm afraid, it will break existing spec instead of improving it.

Because of this, I think we should hold on to merging it into the main. Maybe in the near future, we can think about merging it. Let's see the feedback from customers about recently merged PR's first 👍

sacOO7 avatar Apr 29 '24 05:04 sacOO7