Skip to content

Commit

Permalink
New client option to propagate cache errors (#1519)
Browse files Browse the repository at this point in the history
Previously, some client APIs may ignore cache errors as they were
not critical for the API to operate correctly. New option allows
to enforce errors propagation in all cases when cache consistency
is a key.

Relates-To: DATASDK-10

Signed-off-by: Andrey Kashcheev <[email protected]>
  • Loading branch information
andrey-kashcheev committed Jun 4, 2024
1 parent 16c1952 commit bdb4845
Show file tree
Hide file tree
Showing 7 changed files with 267 additions and 21 deletions.
12 changes: 10 additions & 2 deletions olp-cpp-sdk-core/include/olp/core/client/OlpClientSettings.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2019-2023 HERE Europe B.V.
* Copyright (C) 2019-2024 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 @@ -232,12 +232,20 @@ struct CORE_API OlpClientSettings {
* This setting only applies to the mutable cache and to the in-memory cache,
* but should not affect the protected cache as no entries are added to the
* protected cache in read-only mode. Set to std::chrono::seconds::max() to
* disable expiration. By default expiration is disabled.
* disable expiration. By default, expiration is disabled.
*
* @note This only makes sense for data that has an expiration limit, e.g.
* volatile or versioned, and which is stored in cache.
*/
std::chrono::seconds default_cache_expiration = std::chrono::seconds::max();

/**
* @brief The flag to enable or disable the propagation of all cache errors.
*
* When set to `false` only critical cache errors are propagated to the user.
* By default, this setting is set to `false`.
*/
bool propagate_all_cache_errors = false;
};

} // namespace client
Expand Down
6 changes: 4 additions & 2 deletions olp-cpp-sdk-dataservice-read/src/VersionedLayerClientImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,8 @@ client::CancellationToken VersionedLayerClientImpl::GetData(

repository::DataRepository repository(catalog_, settings_, lookup_client_,
mutex_storage_);
return repository.GetVersionedData(layer_id_, request, version, context);
return repository.GetVersionedData(layer_id_, request, version, context,
settings_.propagate_all_cache_errors);
};

return task_sink_.AddTask(std::move(data_task), std::move(callback),
Expand Down Expand Up @@ -841,7 +842,8 @@ client::CancellationToken VersionedLayerClientImpl::GetAggregatedData(
repository::DataRepository data_repository(catalog_, settings_,
lookup_client_, mutex_storage_);
auto data_response = data_repository.GetVersionedData(
layer_id_, data_request, version, context);
layer_id_, data_request, version, context,
settings_.propagate_all_cache_errors);

const auto aggregated_network_statistics =
partition_response.GetPayload() + data_response.GetPayload();
Expand Down
3 changes: 2 additions & 1 deletion olp-cpp-sdk-dataservice-read/src/VolatileLayerClientImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,8 @@ client::CancellationToken VolatileLayerClientImpl::GetData(
auto task = [=](client::CancellationContext context) {
repository::DataRepository repository(catalog_, settings_, lookup_client_,
mutex_storage_);
return repository.GetVolatileData(layer_id_, request, context);
return repository.GetVolatileData(layer_id_, request, std::move(context),
settings_.propagate_all_cache_errors);
};

return task_sink_.AddTask(std::move(task), std::move(callback),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ DataResponse DataRepository::GetVersionedTile(

auto data_response =
GetBlobData(layer_id, kBlobService, partition, request.GetFetchOption(),
request.GetBillingTag(), std::move(context));
request.GetBillingTag(), std::move(context),
settings_.propagate_all_cache_errors);
network_statistics += data_response.GetPayload();

if (data_response) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,18 +50,18 @@ class DataRepository final {
const DataRequest& request,
int64_t version,
client::CancellationContext context,
bool fail_on_cache_error = false);
bool fail_on_cache_error);

BlobApi::DataResponse GetVolatileData(const std::string& layer_id,
const DataRequest& request,
client::CancellationContext context,
bool fail_on_cache_error = false);
bool fail_on_cache_error);

BlobApi::DataResponse GetBlobData(
const std::string& layer, const std::string& service,
const model::Partition& partition, FetchOptions fetch_option,
const boost::optional<std::string>& billing_tag,
client::CancellationContext context, bool fail_on_cache_error = false);
client::CancellationContext context, bool fail_on_cache_error);

private:
client::HRN catalog_;
Expand Down
121 changes: 109 additions & 12 deletions olp-cpp-sdk-dataservice-read/tests/DataRepositoryTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include <thread>

#include <matchers/NetworkUrlMatchers.h>
#include <mocks/CacheMock.h>
#include <mocks/NetworkMock.h>
#include <olp/core/cache/CacheSettings.h>
#include <olp/core/client/ApiLookupClient.h>
Expand Down Expand Up @@ -127,7 +128,7 @@ TEST_F(DataRepositoryTest, GetBlobData) {
auto response = repository.GetBlobData(
kLayerId, kService, partition,
olp::dataservice::read::FetchOptions::OnlineIfNotFound, boost::none,
context);
context, false);

ASSERT_TRUE(response.IsSuccessful());
}
Expand All @@ -149,7 +150,7 @@ TEST_F(DataRepositoryTest, GetBlobDataApiLookupFailed403) {
auto response = repository.GetBlobData(
kLayerId, kService, partition,
olp::dataservice::read::FetchOptions::OnlineIfNotFound, boost::none,
context);
context, false);

ASSERT_FALSE(response.IsSuccessful());
}
Expand All @@ -164,7 +165,7 @@ TEST_F(DataRepositoryTest, GetBlobDataNoDataHandle) {
auto response = repository.GetBlobData(
kLayerId, kService, olp::dataservice::read::model::Partition(),
olp::dataservice::read::FetchOptions::OnlineIfNotFound, boost::none,
context);
context, false);

ASSERT_FALSE(response.IsSuccessful());
}
Expand All @@ -191,7 +192,7 @@ TEST_F(DataRepositoryTest, GetBlobDataFailedDataFetch403) {
auto response = repository.GetBlobData(
kLayerId, kService, partition,
olp::dataservice::read::FetchOptions::OnlineIfNotFound, boost::none,
context);
context, false);

ASSERT_FALSE(response.IsSuccessful());
}
Expand Down Expand Up @@ -220,7 +221,7 @@ TEST_F(DataRepositoryTest, GetBlobDataCache) {
auto response = repository.GetBlobData(
kLayerId, kService, partition,
olp::dataservice::read::FetchOptions::OnlineIfNotFound, boost::none,
context);
context, false);

ASSERT_TRUE(response.IsSuccessful());

Expand All @@ -229,7 +230,7 @@ TEST_F(DataRepositoryTest, GetBlobDataCache) {
response = repository.GetBlobData(
kLayerId, kService, partition,
olp::dataservice::read::FetchOptions::OnlineIfNotFound, boost::none,
context);
context, false);

ASSERT_TRUE(response.IsSuccessful());
}
Expand Down Expand Up @@ -260,7 +261,7 @@ TEST_F(DataRepositoryTest, GetBlobDataImmediateCancel) {
auto response = repository.GetBlobData(
kLayerId, kService, partition,
olp::dataservice::read::FetchOptions::OnlineIfNotFound, boost::none,
context);
context, false);

ASSERT_EQ(response.GetError().GetErrorCode(),
olp::client::ErrorCode::Cancelled);
Expand Down Expand Up @@ -295,7 +296,7 @@ TEST_F(DataRepositoryTest, GetBlobDataInProgressCancel) {
auto response = repository.GetBlobData(
kLayerId, kService, partition,
olp::dataservice::read::FetchOptions::OnlineIfNotFound, boost::none,
context);
context, false);

ASSERT_EQ(response.GetError().GetErrorCode(),
olp::client::ErrorCode::Cancelled);
Expand Down Expand Up @@ -339,7 +340,7 @@ TEST_F(DataRepositoryTest, GetBlobDataSimultaniousFailedCalls) {
auto response = repository.GetBlobData(
kLayerId, kService, partition,
olp::dataservice::read::FetchOptions::OnlineIfNotFound, boost::none,
context);
context, false);
EXPECT_FALSE(response.IsSuccessful());
});

Expand All @@ -357,7 +358,7 @@ TEST_F(DataRepositoryTest, GetBlobDataSimultaniousFailedCalls) {
auto response = repository.GetBlobData(
kLayerId, kService, partition,
olp::dataservice::read::FetchOptions::OnlineIfNotFound, boost::none,
context);
context, false);
EXPECT_FALSE(response.IsSuccessful());
});

Expand Down Expand Up @@ -590,7 +591,7 @@ TEST_F(DataRepositoryTest, GetBlobDataCancelParralellRequest) {
auto response = repository.GetBlobData(
kLayerId, kService, partition,
olp::dataservice::read::FetchOptions::OnlineIfNotFound, boost::none,
context);
context, false);

EXPECT_FALSE(response);
EXPECT_EQ(response.GetError().GetErrorCode(),
Expand All @@ -604,7 +605,7 @@ TEST_F(DataRepositoryTest, GetBlobDataCancelParralellRequest) {
auto response = repository.GetBlobData(
kLayerId, kService, partition,
olp::dataservice::read::FetchOptions::OnlineIfNotFound, boost::none,
context);
context, false);

EXPECT_FALSE(response);
EXPECT_EQ(response.GetError().GetErrorCode(),
Expand All @@ -631,4 +632,100 @@ TEST_F(DataRepositoryTest, GetBlobDataCancelParralellRequest) {
// Compare time spanding for waiting for threads to finish
EXPECT_LT(end - start, wait_time);
}

TEST_F(DataRepositoryTest, GetBlobDataFailedToCache) {
auto cache_mock = std::make_shared<CacheMock>();

settings_->propagate_all_cache_errors = true;
settings_->cache = cache_mock;

EXPECT_CALL(*cache_mock, Put(testing::HasSubstr("::api"), _, _, _))
.WillRepeatedly(testing::Return(true));
EXPECT_CALL(*cache_mock, Put(testing::HasSubstr("::Data"), _, _))
.WillRepeatedly(testing::Return(false));

EXPECT_CALL(*cache_mock, Get(_, _))
.WillRepeatedly(testing::Return(boost::any()));
EXPECT_CALL(*cache_mock, Get(_)).WillRepeatedly(testing::Return(nullptr));

EXPECT_CALL(*network_mock_, Send(IsGetRequest(kUrlLookup), _, _, _, _))
.WillOnce(ReturnHttpResponse(olp::http::NetworkResponse().WithStatus(
olp::http::HttpStatusCode::OK),
kUrlResponseLookup));

EXPECT_CALL(*network_mock_, Send(IsGetRequest(kUrlBlobData269), _, _, _, _))
.WillOnce(ReturnHttpResponse(olp::http::NetworkResponse().WithStatus(
olp::http::HttpStatusCode::OK),
"someData"));

olp::client::CancellationContext context;

olp::dataservice::read::model::Partition partition;
partition.SetDataHandle(kUrlBlobDataHandle);

olp::client::HRN hrn(GetTestCatalog());
ApiLookupClient lookup_client(hrn, *settings_);
DataRepository repository(hrn, *settings_, lookup_client);
auto response = repository.GetBlobData(
kLayerId, kService, partition,
olp::dataservice::read::FetchOptions::OnlineIfNotFound, boost::none,
context, true);

ASSERT_FALSE(response);
ASSERT_EQ(response.GetError().GetErrorCode(),
olp::client::ErrorCode::CacheIO);
}

TEST_F(DataRepositoryTest, GetVersionedDataTileFailedToCache) {
auto cache_mock = std::make_shared<CacheMock>();

settings_->propagate_all_cache_errors = true;
settings_->cache = cache_mock;

EXPECT_CALL(*cache_mock, Put(testing::HasSubstr("::api"), _, _, _))
.WillRepeatedly(testing::Return(true));
EXPECT_CALL(*cache_mock, Put(testing::HasSubstr("::quadtree"), _, _))
.WillRepeatedly(testing::Return(true));
EXPECT_CALL(*cache_mock, Put(testing::HasSubstr("::Data"), _, _))
.WillRepeatedly(testing::Return(false));

EXPECT_CALL(*cache_mock, Get(_, _))
.WillRepeatedly(testing::Return(boost::any()));
EXPECT_CALL(*cache_mock, Get(_)).WillRepeatedly(testing::Return(nullptr));

EXPECT_CALL(*network_mock_, Send(IsGetRequest(kUrlLookup), _, _, _, _))
.WillRepeatedly(
ReturnHttpResponse(olp::http::NetworkResponse().WithStatus(
olp::http::HttpStatusCode::OK),
kUrlResponseLookup));

EXPECT_CALL(*network_mock_,
Send(IsGetRequest(kUrlQueryTreeIndex), _, _, _, _))
.WillRepeatedly(
ReturnHttpResponse(olp::http::NetworkResponse().WithStatus(
olp::http::HttpStatusCode::OK),
kSubQuads));

EXPECT_CALL(*network_mock_,
Send(IsGetRequest(kUrlBlobData5904591), _, _, _, _))
.WillRepeatedly(
ReturnHttpResponse(olp::http::NetworkResponse().WithStatus(
olp::http::HttpStatusCode::OK),
"someData"));

olp::client::HRN hrn(GetTestCatalog());
const auto kVersion = 4u;

auto request = olp::dataservice::read::TileRequest().WithTileKey(
olp::geo::TileKey::FromHereTile("5904591"));
olp::client::CancellationContext context;
ApiLookupClient lookup_client(hrn, *settings_);
DataRepository repository(hrn, *settings_, lookup_client);
auto response =
repository.GetVersionedTile(kLayerId, request, kVersion, context);

ASSERT_FALSE(response);
ASSERT_EQ(response.GetError().GetErrorCode(),
olp::client::ErrorCode::CacheIO);
}
} // namespace
Loading

0 comments on commit bdb4845

Please sign in to comment.