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

Upward sync push discarded due to payload size limit exceeded #506

Closed
sumairasaeed opened this issue Mar 28, 2017 · 10 comments
Closed

Upward sync push discarded due to payload size limit exceeded #506

sumairasaeed opened this issue Mar 28, 2017 · 10 comments
Assignees
Labels

Comments

@sumairasaeed
Copy link
Contributor

Discussed this issue with @sojharo that the push response received from server in response to upward sync API call sometimes become large if pending data was large. In this case push is discarded and not sent.

Here is the screenshot:
screen shot 2017-03-28 at 4 25 27 pm

Also, here is screenshot of large payload:
screen shot 2017-03-28 at 4 25 03 pm

@sojharo
Copy link
Contributor

sojharo commented Mar 29, 2017

Cloudkibo/Android#561 (comment)

As discussed in above comment, I would work here to avoid sending push notifications in response of http request of upward sync. The http response would also contain this payload which was first being sent as push notification and was increasing payload size limit. This way client would handle it in response of http instead of push.

@jekram
Copy link
Contributor

jekram commented Mar 29, 2017

Thanks. Should we close this as this is already getting fixed in 561?

@sojharo
Copy link
Contributor

sojharo commented Mar 29, 2017

We should close Cloudkibo/Android#561 as I would be working only on server side.

@jekram
Copy link
Contributor

jekram commented Mar 29, 2017

Ok.

@jekram
Copy link
Contributor

jekram commented Mar 29, 2017

@sojharo

There were two issues on #561

I am assuming that we are talking care of both.

So we can do can do # 2 and # 4 and then do #3

I think you missed understood my question. I was asking why we need to send Push Notification to the Sender. Since we are going Rest Call to send the and if get success or failure why do we need notification in this case.

sojharo added a commit that referenced this issue Mar 30, 2017
sojharo added a commit that referenced this issue Mar 30, 2017
@sojharo
Copy link
Contributor

sojharo commented Mar 30, 2017

I worked on this and now instead of sending response in of upward sync in push notifications I send it using http response. It was little bit difficult as lots of callbacks of mongodb were giving me responses and I had to aggregate them in a single response. Therefore it took me time. I need to keep this task open as now I need to do work on android side in response to these changes on server.

@sojharo
Copy link
Contributor

sojharo commented Mar 30, 2017

Also I have thought of logic to avoid messages going in unordered way. I would also fix that in this same issue. I discussed this problem with sumaira again today.

@jekram
Copy link
Contributor

jekram commented Mar 30, 2017

Thanks

sojharo added a commit that referenced this issue Mar 31, 2017
sojharo added a commit that referenced this issue Apr 1, 2017
@sojharo
Copy link
Contributor

sojharo commented Apr 1, 2017

I have completed the work on this. I also worked on android side to reflect changes on server.

@sojharo sojharo self-assigned this Apr 1, 2017
@jekram
Copy link
Contributor

jekram commented Apr 1, 2017

Thanks

@jekram jekram closed this as completed Apr 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants