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

ParseObject saved multiple times on slow internet connection #764

Open
Kiwifed0r opened this issue Dec 7, 2017 · 15 comments · May be fixed by #1208
Open

ParseObject saved multiple times on slow internet connection #764

Kiwifed0r opened this issue Dec 7, 2017 · 15 comments · May be fixed by #1208
Labels
bounty:$20 Bounty applies for fixing this issue (Parse Bounty Program) type:feature New feature or improvement of existing feature

Comments

@Kiwifed0r
Copy link
Contributor

Kiwifed0r commented Dec 7, 2017

Hi,

I'm having an issue with parse and very slow network connection. This can be reproduced with the emulator when using cellular network and setting it to GSM and signal strength 'None'. Now, when saving a ParseObject, the call goes through and the ParseObject shows up in the dashboard but the client doesn't get the answer before the timeout. This means that Parse tries again and saves it another time and so on until the maximum number of retries is reached. And finally the callback is called reporting an error. Setting the network to LTE and signal strength 'Good' at some time results in a positive callback for the most recent try.

One solution which I tried is to set the timeouts to 60 seconds. This works and the answer from the server reaches the client on very slow internet before the timeout.
Another possible solution would be to tell Parse not to retry.

Does anyone have any other suggestions how to solve this issue?
Does it make sense for Parse to retry saving an object after a failure? There's no way to be sure whether or not the object was saved.
For fetching data on the other hand retrying is fine.

This only happens when using https.

Parse version: 1.16.3

@montymxb montymxb added type:bug Impaired feature or lacking behavior that is likely assumed needs investigation labels Dec 8, 2017
@montymxb
Copy link

montymxb commented Dec 8, 2017

We'll have to see about this one, it's a bit tough when working under very poor or interrupted network conditions to ensure much of anything. I wonder if maybe preventing retries would even help, as you still may not be able to tell if the object was even created or not. It would be nice to deliberate and see if there is a reasonable solution for this.

@Kiwifed0r
Copy link
Contributor Author

I ran some tests on iOS and I was able to reproduce the issue on iOS aswell. The only difference is that the timeouts are different. It seems that the default timeouts on iOS are a lot higher (60 seconds) compared to Android (okhttp, 10 seconds?). To reduce the chance of this happening I will set the timeouts to 60 seconds for now. The connection has to be incredibly poor for this to happen with 60 seconds timeout and it shouldn't happen very often but still not impossible.

I played around with setting number of retries to 0 and it also prevents saving the object multiple times but then the problem is that we still don't really know what's going on and the 10 seconds timeout is too less.

The best 'solution' for now is to increase the okhttp timeout.

@montymxb
Copy link

montymxb commented Dec 9, 2017

@Kiwifed0r hopefully you're able to keep the issue at bay with your timeout patch. It's not the best but if it works in your case it will have to do for the moment. Since, this could happen to any sdk I'm wondering if a server side fix should be considered...

We had a similar issue brought up in parse server where a response wasn't returning to the client, resulting in multiple retries and duplicate objects. Unfortunately if the server cannot return a response for some reason there's not a ton we can do about it :/. It's implied that we trust the connection to completion, but if becomes strained some undesired behavior (such as this) can become evident.

@lukyanov
Copy link

lukyanov commented Dec 18, 2017

@montymxb @Kiwifed0r That's why usually non-idempotent requests are not retried by default.

Due to the nature of HTTP protocol and client-server architecture, from the client point of view there are always 3 types of responses: 1) object has been successfully saved, 2) object has not been saved, 3) the status of object is unknown (it could be saved or not). This will always be like this. There will always be situations when the client doesn't know. You CANNOT fix this on the client or on the server side.

The only requests that can be safely retried are read requests. All the rest types are subject to business logic consideration. The app should decide, not SDK. So my proposal would be to remove retry logic from SDK altogether.

@Jawnnypoo
Copy link
Member

100% in agreement that the retry should be configurable by the client, disabled by default

@CoderickLamar
Copy link

If we pass our own okhttp Builder, does it override any other settings?

Basically, I'm deciding whether it makes more sense to override that actual timeout code by forking, or to pass my own okhttp Builder. When I passed my own OkHttpClient.Builder, I had some unintended issues, but I wasn't entirely sure if these were related.

@Jawnnypoo
Copy link
Member

The timeout unfortunately does not come from OkHttp but is deeper in the SDK.

@univ3rse
Copy link

Any news on this, or combined effort we can make to improve this?
We are very keen to get a solution for this, as we constantly run into timeouts for certain things.

@univ3rse
Copy link

I added the SDK locally so I could fool around a bit easier, I set the three timeouts to the internal member of OkHttpBuilder before build() is called and it seemed to work fine. Tried an external OkHttpBuilder and it seemed to work as well.
I do seem to remember that an external one did not work in the past for some reason, but have not traced the commits to see if there were any changes.

This should be fairly easy to add as an extension of the Parse.Configuration.Builder and check when the builder is being loaded if there are any custom timeouts to be set.(In ParsePlugins after line 109)

@thunderwin
Copy link

someone fixed this issue....?? please

@mtrezza
Copy link
Member

mtrezza commented Oct 27, 2019

@thunderwin The issue still exists. A workaround is to:
a) disable retries: http://parseplatform.org/Parse-SDK-Android/api/com/parse/Parse.Configuration.Builder.html
b) Implement logic yourself if possible in your use case; to prevent for example multiple writes you can set a UUID for the object so the server won’t create it multiple times.

@laibonn
Copy link

laibonn commented Dec 11, 2019

The Parse.Configuration.Builder class has a .clientBuilder() method to which you can pass a custom OkHttpClient.Builder whereby you have set custom timeout settings e.g.

OkHttpClient.Builder parseOkHttpClientBuilder  = new OkHttpClient.Builder();
parseOkHttpClientBuilder.callTimeout(30, TimeUnit.SECONDS)
                    .connectTimeout(15, TimeUnit.SECONDS)
                    .readTimeout(45, TimeUnit.SECONDS)
                    .writeTimeout(60, TimeUnit.SECONDS);

Parse.Configuration.Builder builder = new Parse.Configuration.Builder(this);
builder.applicationId("yourAppId")
                    .server("yourServerUrl")
                    .clientKey("yourClientKey")
                    .maxRetries(3) //whatever works for you
                    .clientBuilder(parseOkHttpClientBuilder); //The custom  OkHttpClient builder

The custom OkHttpClient builder will subsequently be used to create ParseHttpClient and your custom timeouts will be used.

The primary drawback I see here is that in most cases, there are specific server calls that a developer would expect to take longer than usual, so it would be ideal to be able to set the timeout and retry for specific calls, rather than for the entire Parse configuration.

@mtrezza mtrezza removed the discussion label Oct 7, 2021
@mtrezza mtrezza added type:feature New feature or improvement of existing feature and removed type:bug Impaired feature or lacking behavior that is likely assumed state:needs-investigation labels Nov 26, 2021
@mtrezza
Copy link
Member

mtrezza commented Nov 26, 2021

Changed this from bug to feature, as the idempotency feature has been added to Parse Server, but the SDK needs a feature PR to support it.

@mtrezza mtrezza added the bounty:$20 Bounty applies for fixing this issue (Parse Bounty Program) label Nov 26, 2021
@alexandersokol
Copy link

Any updates on this? Just reproduces this bug on 4.1.0.

@mtrezza
Copy link
Member

mtrezza commented Oct 31, 2022

Would you want to open a PR? It should be fairly easy, there are other SKD that already support it, you could look that these as a hint.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bounty:$20 Bounty applies for fixing this issue (Parse Bounty Program) type:feature New feature or improvement of existing feature
Projects
None yet
10 participants