UI hang when Receiver gets internet and get chat through partial sync

[sumairasaeed]

Repro steps 1- I switched off Internet on Device A 2- I sent lot of messages from Device B to Device A 3- For all these messages, status was shown as "sent" but not "delivered" as Device A's internet was off

4. Later I opened internet on Device A and launched application
5. At this time Device A gets all of the chat messages which were sent from Device B.
6. Device B become hanged at this point
7. Device B was getting multiple status updates of "delivered" at a time
8. At each status update, it was reloading UI so remained hanged.

[jekram]

Before we work on this please send me the design document in this area.

Send me the link to the page number where we discuss this

[sumairasaeed]

Here is the link to heading which discussed this use case: https://docs.google.com/document/d/1BR4cBUZoAIWnIZzQiuZFTZVORsXaEyLEUV5EDM8ByfI/edit#heading=h.89bx99leke9n

On Mon, Dec 19, 2016 at 7:10 AM, Cloudkibo notifications@github.com wrote:

Before we work on this please send me the design document in this area.

Send me the link to the page number where we discuss this

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Cloudkibo/iOS/issues/453#issuecomment-267866720, or mute the thread https://github.com/notifications/unsubscribe-auth/AKbhpxdEpZCgdhUhQ9v_Tezz3ubmFE23ks5rJeeWgaJpZM4LQTAC .

[jekram]

@sumairasaeed

This is cut and paste from your design document: Case # 2: Sender is online but receiver is not:

"This is the case when sender(Client A) is online but receiver(Client B) is offline and not conneceted to internet. In this scenario, Client A sends a chat message(or multiple messages) to Client B. Each chat message is sent to cloudkibo server using HTTP network request. At this point, chat message status is saved locally as “pending” and UI gets reloaded. When we get API response as success then we update this chat status as “sent” and also we reload UI with updated status

As Client B was offline, so it didnot receive any of the chat message. When Client B comes online, it performs partial sync of chat, assuming that application was already installed on device of Client B. Partial chat sync API returns those message from server whose status was NOT “delivered” or “seen”. Client B now gets all the chat messages sent by Client A. For each message received, Client B sends a status update of “delivered” to Client B.

When Client A opens chat page, it sends status of “seen” for all messages to Client A. Client A gets all status updates. For each status update(received as silent push notification) , Client A updates the database and UI."

1. This is not accurate:

"This is the case when sender(Client A) is online but receiver(Client B) is offline and not conneceted to internet. In this scenario, Client A sends a chat message(or multiple messages) to Client B. Each chat message is sent to cloudkibo server using HTTP network request. At this point, chat message status is saved locally as “pending” and UI gets reloaded.

The message should not be in "pending" state but "sent" stage and then it should to "delivered" and "seen" stage.

2. Your logic does not show Server sending notifications to B. Would Server not sending the notifications anyway regardless of the status of B. Those notifications would be queued by notification service and delivered to B when it comes back up.

3. When B is back up those notifications would be delivered to B. At that time B should Call the API to get the messages? Correct?

4. So are we getting undelivered messages from notification service and partial chat synch?

5. Are we saying that if they are, 10 messages to be sent from the server to device B. Device B would 10 via notification service and 10 via partial chat synch logic and device B would send 20 status back?

6. It is not clear how this partial chat synch get triggered?

7. Please update the flow to show the role of notification service also.

@sojharo Can you share the design document on how this is done in Android.

[sojharo]

I have no document on this exact issue. However, when Internet comes back I automatically receive the push notification for message which was sent when I was not on Internet. I had set the time in azure push notification settings that push notification should remain alive as long as possible. There was exact number of days till which push notification is alive. So no matter if i am offline for hours, when I come back to Internet I get push notification for the messages.

[jekram]

@sojharo Can you share Andriod Design Document of Andriod internet recovery. I want to make sure the Internet recovery is the same for both the clients.

[jekram]

@sumairasaeed I had sent feedback on your design document. Have you made those changes in the design document? I would like to get an updated design document before proceeding to next step. What is the ETA for an updated design document

[sumairasaeed]

Answer to queries: Discussed with @sojharo . Internet recovery logic is same for Android and iOS.

Here are answers to above questions:

** This is not accurate: "This is the case when sender(Client A) is online but receiver(Client B) is offline and not conneceted to internet. In this scenario, Client A sends a chat message(or multiple messages) to Client B. Each chat message is sent to cloudkibo server using HTTP network request. At this point, chat message status is saved locally as “pending” and UI gets reloaded.

The message should not be in "pending" state but "sent" stage and then it should to "delivered" and "seen" stage. **

Ans: This is the logic which is consistent on Android and iOS. These are the terms we decided when designing logic for chat. Chat will be in 'pending' state unless it reaches server. When it reaches server, status becomes "sent" . When Receiver receives message, status becomes "delivered". When user reads, it changes to "seen"

Your logic does not show Server sending notifications to B. Would Server not sending the notifications anyway regardless of the status of B. Those notifications would be queued by notification service and delivered to B when it comes back up.

Yes, server will send notifications anyway regardless of the status of B. Discussed this with @sojharo also. Both on Android and iOS , notification hub does not send queued push notifications for 1-1 chats. On Android, it receives for group-chat but not for 1-1 chat. We are unable to comprehend the reason behind it.

When B is back up those notifications would be delivered to B. At that time B should Call the API to get the messages? Correct?

Ans- We are not receiving notifications for 1-1 chat when internet is back.

So are we getting undelivered messages from notification service and partial chat synch?

Ans- No, we are currently getting messages from partial sync only for 1-1 chat.

Are we saying that if they are, 10 messages to be sent from the server to device B. Device B would 10 via notification service and 10 via partial chat synch logic and device B would send 20 status back?

Ans- No, we will get 10 messages via partial sync and get 10 status back for 'delivered' . Then when user reads all those messages, we get 10 more statuses of "seen"

It is not clear how this partial chat synch get triggered? Please update the flow to show the role of notification service also.

Ans- Partial chat sync API returns those message from server whose status was NOT “delivered” or “seen”. For 1-1 chat, Notification Hub is currently paying no role when internet is restored. We donot get any push.

Solution : The issue is UI hang due to multiple status updates. for each update there is a seperate push. @sojharo has recommended that we send all statuses once in form of array so UI gets updated once only. This will require some server-side and client-side changes. We will open tasks on server-side and client side after your feedback.

[jekram]

Opening new task is not the solution.We need to fix this now.

Also it is a complete waste of time to have these discussions after three months. When we work on a issue we need to take it to a logical conclusion.

This was marked High and we chose to work on low priority issues.

[sojharo]

I will open the tasks on server side for this. Please give me an hour or two. I need to think through the design as it would be slight change in the logic of how we do sync today.

[sojharo]

I have created the design document for how it is done currently and how it will happen after changes.

[sojharo]

https://docs.google.com/document/d/1OuH-Fr3b2TjBNDRpIzDMdI7dzf4exd15EWAxiWyU5k4/edit

[sumairasaeed]

Completed code for sending upward sync details. Needs to code part for processing push notification responses from server

[sumairasaeed]

Completed coding and testing for upward and downward sync. It is working fine and we are getting all pending information. Also, made little modification and updating UI on main thread only if we are on chat window. Tested between iOS to iOS. There is still a glitch on UI. One solution for this is that we also merge our push notifications for message status as we have merged out HTTP requests for upward and downward sync. We can define a limit e.g. we will integrate push for every 5 status updates. So if user fetches 5 unread messages from downward sync, so single push should be sent to sender indicating status "delivered" for all those 5 messages. And we will update our UI once only instead of updating 5 times. @sojharo FYI

[jekram]

I am interested in when this will get resolved and not in any glitch.

[jekram]

all the server work is complete by sojharo and both the send and client work on Android is also done. When can we have this done?

I do not understand what you are saying here? Is this different from the design document?

"There is still a glitch on UI. One solution for this is that we also merge our push notifications for message status as we have merged out HTTP requests for upward and downward sync. We can define a limit e.g. we will integrate push for every 5 status updates. So if user fetches 5 unread messages from downward sync, so single push should be sent to sender indicating status "delivered" for all those 5 messages. And we will update our UI once only instead of updating 5 times. "

[jekram]

@sumairasaeed Are you saying that we get 1) incoming messages from push notification 2) incoming message when we call the resyn code? Correct?

[sumairasaeed]

yes sir correct.

On Thu, Mar 23, 2017 at 10:16 AM, Cloudkibo notifications@github.com wrote:

@sumairasaeed https://github.com/sumairasaeed Are you saying that we get 1) incoming messages from push notification 2) incoming message when we call the resyn code? Correct?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Cloudkibo/iOS/issues/453#issuecomment-288619732, or mute the thread https://github.com/notifications/unsubscribe-auth/AKbhp7HGs9XvAltpbg4MNjbsxTeUK0cCks5rogAWgaJpZM4LQTAC .

[jekram]

so why we should get the message twice?

[sumairasaeed]

Sir, our sync logic is incremental. We only get those messages from sync which are not on device. It is useful for the case if we had previously missed the push of that message for some reason.

[jekram]

We should be able to handle individual push notification messages and we do not need another redesign. We are not talking about 100's os messages coming in. We need to queue the message and process it, we should not expect the server to do that work for the client.

[sumairasaeed]

Worked on this. Coded to update/insert only the required rows instead of reloading whole table for 1-1 chat. This has significantly improved the performance and solved UI glitch issue. However, there is some issue in tableview scrolling. It scrolls table to wrong row on iOS 9. Working fine on iOS 10. Next,need to do same for group chat and need to fix scroll to wrong row error.

[jekram]

This has been only open since Mid Dec when we would resolve this?

[sumairasaeed]

Will wrap it up by today IA

[jekram]

This you have been saying for weeks

[jekram]

What is the status here? We need to be updating active task on a daily basis.

[sumairasaeed]

This has been done and pushed

[jekram]

How many pending messages it was tested with and what was the response time?

[jekram]

@sumairasaeed

This issue is not fixed. See you own comments on https://github.com/Cloudkibo/Android/issues/561

[sumairasaeed]

I tested with 15 messages and it took around 2 seconds after internet is back. This was UI issue that UI was previously freezed/unresponsive for long time. This issue is not same as Cloudkibo/Android#561 sir. The issue Cloudkibo/Android#561 is related to disturbed message ordering. Server sends messages push in wrong order.

[jekram]

@sumairasaeed

We made the change that went from UI not being hung to be UI not to be correct? Out of order?

You think it is ok to fix one problem at the expense on another problem?

[sumairasaeed]

UI hang issue was fixed. We can close this please