Skip to content

Commit be2513d

Browse files
Don't cancel merged requests (#1706)
Cancellation is not an error and shouldn't be propagated to parallel requests Relates-To: DATASDK-101 Signed-off-by: Andrey Kashcheev <ext-andrey.kashcheev@here.com>
1 parent 8fdc782 commit be2513d

3 files changed

Lines changed: 82 additions & 3 deletions

File tree

.github/workflows/psv_pipelines.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,7 @@ jobs:
274274

275275
psv-ios-arm64-xcode-26-build:
276276
name: PSV.iOS.MacOS15.Xcode26.ARM64
277-
runs-on: macos-latest
277+
runs-on: macos-15
278278
steps:
279279
- name: Check out repository
280280
uses: actions/checkout@v6

olp-cpp-sdk-dataservice-read/src/repositories/NamedMutex.cpp

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (C) 2020-2023 HERE Europe B.V.
2+
* Copyright (C) 2020-2026 HERE Europe B.V.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -85,6 +85,12 @@ std::mutex& NamedMutexStorage::Impl::GetLockMutex(const std::string& resource) {
8585

8686
void NamedMutexStorage::Impl::SetError(const std::string& resource,
8787
const client::ApiError& error) {
88+
// Ignore cancellation since this is not an error and parallel requests should
89+
// not be canceled because of it.
90+
if (error.GetErrorCode() == client::ErrorCode::Cancelled) {
91+
return;
92+
}
93+
8894
std::lock_guard<std::mutex> lock(mutex_);
8995
auto mutex_it = mutexes_.find(resource);
9096
if (mutex_it != mutexes_.end()) {

olp-cpp-sdk-dataservice-read/tests/DataRepositoryTest.cpp

Lines changed: 74 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (C) 2019-2024 HERE Europe B.V.
2+
* Copyright (C) 2019-2026 HERE Europe B.V.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -375,6 +375,79 @@ TEST_F(DataRepositoryTest, GetBlobDataSimultaniousFailedCalls) {
375375
second_request_thread.join();
376376
}
377377

378+
TEST_F(DataRepositoryTest, GetBlobDataSimultaniousCancelCalls) {
379+
EXPECT_CALL(*network_mock_, Send(IsGetRequest(kUrlLookup), _, _, _, _))
380+
.WillRepeatedly(
381+
ReturnHttpResponse(olp::http::NetworkResponse().WithStatus(
382+
olp::http::HttpStatusCode::OK),
383+
kUrlResponseLookup));
384+
385+
std::promise<void> network_request_started_promise;
386+
std::promise<void> finish_network_request_promise;
387+
388+
auto wait = [&]() {
389+
network_request_started_promise.set_value();
390+
finish_network_request_promise.get_future().wait();
391+
};
392+
393+
testing::InSequence sequence;
394+
EXPECT_CALL(*network_mock_, Send(IsGetRequest(kUrlBlobData269), _, _, _, _))
395+
.WillOnce(testing::DoAll(
396+
testing::InvokeWithoutArgs(wait),
397+
ReturnHttpResponse(
398+
olp::http::NetworkResponse().WithStatus(
399+
static_cast<int>(olp::http::ErrorCode::CANCELLED_ERROR)),
400+
"Cancelled")));
401+
402+
EXPECT_CALL(*network_mock_, Send(IsGetRequest(kUrlBlobData269), _, _, _, _))
403+
.WillOnce(ReturnHttpResponse(olp::http::NetworkResponse().WithStatus(
404+
olp::http::HttpStatusCode::OK),
405+
"someData"));
406+
407+
olp::client::CancellationContext context;
408+
olp::dataservice::read::repository::NamedMutexStorage storage;
409+
410+
olp::dataservice::read::model::Partition partition;
411+
partition.SetDataHandle(kUrlBlobDataHandle);
412+
413+
olp::client::HRN hrn(GetTestCatalog());
414+
ApiLookupClient lookup_client(hrn, *settings_);
415+
DataRepository repository(hrn, *settings_, lookup_client, storage);
416+
417+
// Start first request in a separate thread
418+
std::thread first_request_thread([&]() {
419+
auto response = repository.GetBlobData(
420+
kLayerId, kService, partition,
421+
olp::dataservice::read::FetchOptions::OnlineIfNotFound,
422+
olp::porting::none, context, false);
423+
EXPECT_FALSE(response.IsSuccessful());
424+
ASSERT_EQ(response.GetError().GetErrorCode(),
425+
olp::client::ErrorCode::Cancelled);
426+
});
427+
428+
// Wait until network request processing started
429+
network_request_started_promise.get_future().wait();
430+
431+
// Get a mutex from the storage. It guarantees that when the second thread
432+
// acquires the mutex, the stored error will not be cleaned up in scope of
433+
// ReleaseLock call from the first thread
434+
olp::dataservice::read::repository::NamedMutex mutex(
435+
storage, hrn.ToString() + kService + kUrlBlobDataHandle, context);
436+
437+
// Start second request in a separate thread
438+
std::thread second_request_thread([&]() {
439+
auto response = repository.GetBlobData(
440+
kLayerId, kService, partition,
441+
olp::dataservice::read::FetchOptions::OnlineIfNotFound,
442+
olp::porting::none, context, false);
443+
EXPECT_TRUE(response.IsSuccessful());
444+
});
445+
446+
finish_network_request_promise.set_value();
447+
first_request_thread.join();
448+
second_request_thread.join();
449+
}
450+
378451
TEST_F(DataRepositoryTest, GetVersionedDataTile) {
379452
EXPECT_CALL(*network_mock_, Send(IsGetRequest(kUrlLookup), _, _, _, _))
380453
.WillOnce(ReturnHttpResponse(olp::http::NetworkResponse().WithStatus(

0 commit comments

Comments
 (0)