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.

Sign up for GitHub

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

Remove Android network retries #1579

Merged
merged 1 commit into from
Jan 13, 2025
Merged

Remove Android network retries #1579

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 8 additions & 45 deletions olp-cpp-sdk-core/src/http/android/HttpClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2019 HERE Europe B.V.
* Copyright (C) 2019-2025 HERE Europe B.V.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -103,8 +103,7 @@ public Request(
byte[] postData,
String proxyServer,
int proxyPort,
int proxyType,
int maxRetries) {
int proxyType) {
this.url = url;
this.verb = verb;
this.requestId = requestId;
Expand Down Expand Up @@ -140,7 +139,6 @@ public Request(
this.proxyType = Proxy.Type.HTTP;
break;
}
this.maxRetries = maxRetries;
}

public final String url() {
Expand Down Expand Up @@ -191,10 +189,6 @@ public final boolean noProxy() {
return (hasProxy() && this.proxyServer.equals("No")) || this.proxyType == Proxy.Type.DIRECT;
}

public final int maxRetries() {
return this.maxRetries;
}

private final String url;
private final long requestId;
private final int connectionTimeout;
Expand All @@ -205,7 +199,6 @@ public final int maxRetries() {
private final String proxyServer;
private final int proxyPort;
private final Proxy.Type proxyType;
private final int maxRetries;
}

/**
Expand All @@ -232,7 +225,6 @@ protected Void doInBackground(Request... requests) {
boolean downloadContentSizePresent = false;

try {
int retryCount = 0;
boolean isDone = false;
final URL url = new URL(request.url());

Expand All @@ -242,13 +234,6 @@ protected Void doInBackground(Request... requests) {
return null;
}

// TODO: replace with more elegant and controlled approach
if (retryCount > 0) {
Thread.sleep(100);

cleanup(httpConn);
}

URLConnection conn;
if (request.hasProxy()) {
if (request.noProxy()) {
Expand Down Expand Up @@ -352,21 +337,8 @@ protected Void doInBackground(Request... requests) {
try {
status = httpConn.getResponseCode();
error = httpConn.getResponseMessage();

if ((status > 0)
&& ((status < HttpURLConnection.HTTP_OK)
|| (status >= HttpURLConnection.HTTP_SERVER_ERROR))) {
if (retryCount++ < request.maxRetries()) {
continue;
}
}
} catch (SocketTimeoutException | UnknownHostException e) {
if (retryCount++ < request.maxRetries()) {
httpClient.resetRequest(request.requestId());
continue;
} else {
throw e;
}
throw e;
}
}

Expand Down Expand Up @@ -458,17 +430,12 @@ protected Void doInBackground(Request... requests) {
throw e;
}
} catch (SocketTimeoutException e) {
if (retryCount++ < request.maxRetries()) {
httpClient.resetRequest(request.requestId());
continue;
} else {
throw e;
}
throw e;
}

checkCancelled();

// The request is completed, not cancelled or retried
// The request is completed and not cancelled
// Notifies the native (C++) side that request was completed
isDone = true;
httpClient.completeRequest(request.requestId(), status, uploadedContentSize, downloadContentSize, error, contentType);
Expand Down Expand Up @@ -616,8 +583,7 @@ public HttpTask send(
byte[] postData,
String proxyServer,
int proxyPort,
int proxyType,
int maxRetries) {
int proxyType) {
final Request request =
new Request(
url,
Expand All @@ -629,8 +595,7 @@ public HttpTask send(
postData,
proxyServer,
proxyPort,
proxyType,
maxRetries);
proxyType);
final HttpTask task = new HttpTask(this);
task.executeOnExecutor(executor, request);
return task;
Expand All @@ -647,6 +612,4 @@ private synchronized native void completeRequest(
private synchronized native void dateAndOffsetCallback(long requestId, long date, long offset);
// Callback set date and offset
private synchronized native void headersCallback(long requestId, String[] headers);
// Reset request for retry
private synchronized native void resetRequest(long requestId);
}
}
41 changes: 3 additions & 38 deletions olp-cpp-sdk-core/src/http/android/NetworkAndroid.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2019-2021 HERE Europe B.V.
* Copyright (C) 2019-2025 HERE Europe B.V.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -148,22 +148,6 @@ Java_com_here_olp_network_HttpClient_completeRequest(
downloaded_bytes, error, content_type);
}

/*
* Reset request upon retry
*/
extern "C" OLP_SDK_NETWORK_ANDROID_EXPORT void JNICALL
Java_com_here_olp_network_HttpClient_resetRequest(JNIEnv* env, jobject obj,
jlong request_id) {
auto network = olp::http::GetNetworkAndroidNativePtr(env, obj);
if (!network) {
OLP_SDK_LOG_WARNING(
kLogTag,
"ResetRequest failed - network is invalid, request_id=" << request_id);
return;
}
network->ResetRequest(env, request_id);
}

NetworkAndroid::NetworkAndroid(size_t max_requests_count)
: java_self_class_(nullptr),
jni_send_method_(nullptr),
Expand Down Expand Up @@ -396,10 +380,7 @@ bool NetworkAndroid::Initialize() {
&Java_com_here_olp_network_HttpClient_dataCallback)},
{"completeRequest", "(JIIILjava/lang/String;Ljava/lang/String;)V",
reinterpret_cast<void*>(
&Java_com_here_olp_network_HttpClient_completeRequest)},
{"resetRequest", "(J)V",
reinterpret_cast<void*>(
&Java_com_here_olp_network_HttpClient_resetRequest)}};
&Java_com_here_olp_network_HttpClient_completeRequest)}};

env->RegisterNatives(java_self_class_, methods,
sizeof(methods) / sizeof(methods[0]));
Expand Down Expand Up @@ -715,21 +696,6 @@ void NetworkAndroid::CompleteRequest(JNIEnv* env, RequestId request_id,
run_thread_ready_cv_.notify_all();
}

void NetworkAndroid::ResetRequest(JNIEnv* env, RequestId request_id) {
std::lock_guard<std::mutex> lock(requests_mutex_);
if (!started_) {
return;
}

auto req = requests_.find(request_id);
if (req == requests_.end()) {
OLP_SDK_LOG_WARNING(kLogTag,
"ResetRequest of unknown request_id=" << request_id);
return;
}
req->second->Reinitialize();
}

jobjectArray NetworkAndroid::CreateExtraHeaders(
JNIEnv* env,
const std::vector<std::pair<std::string, std::string> >& extra_headers) {
Expand Down Expand Up @@ -970,7 +936,6 @@ SendOutcome NetworkAndroid::Send(NetworkRequest request,
.count());
const jint jproxy_port = static_cast<jint>(proxy_settings.GetPort());
const jint jproxy_type = static_cast<jint>(proxy_settings.GetType());
const jint jmax_retries = 1;
// Do sending
{
std::lock_guard<std::mutex> lock(requests_mutex_);
Expand All @@ -989,7 +954,7 @@ SendOutcome NetworkAndroid::Send(NetworkRequest request,
auto task_obj = env.GetEnv()->CallObjectMethod(
obj_, jni_send_method_, jurl, jhttp_verb, jrequest_id,
jconnection_timeout, jtransfer_timeout, jheaders, jbody, jproxy,
jproxy_port, jproxy_type, jmax_retries);
jproxy_port, jproxy_type);
if (env.GetEnv()->ExceptionOccurred() || !task_obj) {
OLP_SDK_LOG_ERROR(
kLogTag, "Send failed - HttpClient error, url=" << request.GetUrl());
Expand Down
6 changes: 0 additions & 6 deletions olp-cpp-sdk-core/src/http/android/NetworkAndroid.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,12 +104,6 @@ class NetworkAndroid : public Network {
int uploaded_bytes, int downloaded_bytes, jstring error,
jstring content_type);

/**
* @brief Reset the given message if retry attempt invoked
* @param id - Unique Id of the message
*/
void ResetRequest(JNIEnv* env, RequestId request_id);

/**
* @brief Setup JavaVM pointer
* This must be done before calling initialize
Expand Down
Loading