Skip to content

Commit 977327f

Browse files
authored
ip tagging: refactor ip tagging to make it lock free (envoyproxy#45166)
Commit Message: ip tagging: refactor ip tagging to make it lock free Additional Description: Make the ip tagging lock free and removed unnecessary update callback of data source. Risk Level: mid. Testing: unit. Docs Changes: n/a. Release Notes: n/a. Platform Specific Features: n/a. --------- Signed-off-by: wbpcode/wangbaiping <wbphub@gmail.com>
1 parent c3ab9e5 commit 977327f

5 files changed

Lines changed: 217 additions & 301 deletions

File tree

source/common/config/datasource.h

Lines changed: 16 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,6 @@ absl::optional<std::string> getPath(const envoy::config::core::v3::DataSource& s
6868

6969
template <class DataType>
7070
using DataTransform = std::function<absl::StatusOr<std::shared_ptr<DataType>>(absl::string_view)>;
71-
using DataUpdateCb = std::function<void()>;
7271

7372
struct ProviderOptions {
7473
// Use an empty string if no DataSource case is specified.
@@ -94,11 +93,9 @@ template <class DataType> class DynamicData {
9493
ThreadLocal::SlotAllocator& tls, Api::Api& api,
9594
DataTransform<DataType> data_transform_cb, const ProviderOptions& options,
9695
std::shared_ptr<DataType> initial_data, uint64_t initial_hash,
97-
absl::AnyInvocable<void()> cleanup, absl::Status& creation_status,
98-
absl::optional<std::function<void()>> data_update_cb = absl::nullopt)
96+
absl::AnyInvocable<void()> cleanup, absl::Status& creation_status)
9997
: dispatcher_(main_dispatcher), api_(api), options_(options), filename_(source.filename()),
100-
data_transform_cb_(data_transform_cb), data_update_cb_(data_update_cb), hash_(initial_hash),
101-
cleanup_(std::move(cleanup)) {
98+
data_transform_cb_(data_transform_cb), hash_(initial_hash), cleanup_(std::move(cleanup)) {
10299
slot_ =
103100
ThreadLocal::TypedSlot<typename DynamicData<DataType>::ThreadLocalData>::makeUnique(tls);
104101
slot_->set([initial_data = std::move(initial_data)](Event::Dispatcher&) {
@@ -163,14 +160,11 @@ template <class DataType> class DynamicData {
163160
hash_ = new_hash;
164161
}
165162

166-
slot_->runOnAllThreads([new_data = std::move(transformed_new_data_or_error.value()),
167-
this](OptRef<typename DynamicData<DataType>::ThreadLocalData> obj) {
163+
slot_->runOnAllThreads([new_data = std::move(transformed_new_data_or_error.value())](
164+
OptRef<typename DynamicData<DataType>::ThreadLocalData> obj) {
168165
if (obj.has_value()) {
169166
obj->data_ = new_data;
170167
}
171-
if (data_update_cb_.has_value()) {
172-
(*data_update_cb_)();
173-
}
174168
});
175169
return absl::OkStatus();
176170
}
@@ -180,7 +174,6 @@ template <class DataType> class DynamicData {
180174
const ProviderOptions options_;
181175
const std::string filename_;
182176
DataTransform<DataType> data_transform_cb_;
183-
absl::optional<DataUpdateCb> data_update_cb_;
184177
uint64_t hash_;
185178
absl::AnyInvocable<void()> cleanup_;
186179
ThreadLocal::TypedSlotPtr<ThreadLocalData> slot_;
@@ -211,7 +204,6 @@ template <class DataType> class DataSourceProvider {
211204
* @param data_transform_cb transforms content of the DataSource (type std::string)
212205
* to the desired `DataType` type.
213206
* @param max_size max size limit of file to read, default 0 means no limit.
214-
* @param data_update_cb optional callback that can be invoked upon data update in the DataSource.
215207
* @return absl::StatusOr<DataSourceProvider> with DataSource contents. or an error
216208
* status if any error occurs.
217209
* NOTE: If file watch is enabled and the new file content does not meet the
@@ -220,17 +212,15 @@ template <class DataType> class DataSourceProvider {
220212
static absl::StatusOr<DataSourceProviderPtr<DataType>>
221213
create(const ProtoDataSource& source, Event::Dispatcher& main_dispatcher,
222214
ThreadLocal::SlotAllocator& tls, Api::Api& api, bool allow_empty,
223-
DataTransform<DataType> data_transform_cb, uint64_t max_size,
224-
absl::optional<DataUpdateCb> data_update_cb = absl::nullopt) {
215+
DataTransform<DataType> data_transform_cb, uint64_t max_size) {
225216
return create(source, main_dispatcher, tls, api, data_transform_cb,
226-
{.allow_empty = allow_empty, .max_size = max_size}, {}, data_update_cb);
217+
{.allow_empty = allow_empty, .max_size = max_size}, {});
227218
}
228219

229220
static absl::StatusOr<DataSourceProviderPtr<DataType>>
230221
create(const ProtoDataSource& source, Event::Dispatcher& main_dispatcher,
231222
ThreadLocal::SlotAllocator& tls, Api::Api& api, DataTransform<DataType> data_transform_cb,
232-
const ProviderOptions& options, absl::AnyInvocable<void()> cleanup = {},
233-
absl::optional<DataUpdateCb> data_update_cb = absl::nullopt) {
223+
const ProviderOptions& options, absl::AnyInvocable<void()> cleanup = {}) {
234224
uint64_t max_size = options.max_size;
235225
auto initial_data_or_error = read(source, options.allow_empty, api, max_size);
236226
RETURN_IF_NOT_OK_REF(initial_data_or_error.status());
@@ -253,11 +243,11 @@ template <class DataType> class DataSourceProvider {
253243

254244
absl::Status creation_status = absl::OkStatus();
255245
const uint64_t hash = options.hash_content ? HashUtil::xxHash64(*initial_data_or_error) : 0;
256-
auto ret = std::unique_ptr<DataSourceProvider>(
257-
new DataSourceProvider<DataType>(std::make_unique<DynamicData<DataType>>(
258-
source, main_dispatcher, tls, api, data_transform_cb, options,
259-
std::move(transformed_data_or_error).value(), hash, std::move(cleanup), creation_status,
260-
data_update_cb)));
246+
auto ret = std::unique_ptr<DataSourceProvider>(new DataSourceProvider<DataType>(
247+
std::make_unique<DynamicData<DataType>>(source, main_dispatcher, tls, api,
248+
data_transform_cb, options,
249+
std::move(transformed_data_or_error).value(), hash,
250+
std::move(cleanup), creation_status)));
261251
RETURN_IF_NOT_OK(creation_status);
262252
return std::move(ret);
263253
}
@@ -301,16 +291,15 @@ class ProviderSingleton : public Singleton::Instance,
301291
public:
302292
ProviderSingleton(Event::Dispatcher& main_dispatcher, ThreadLocal::SlotAllocator& tls,
303293
Api::Api& api, DataTransform<DataType> data_transform_cb,
304-
const ProviderOptions& options,
305-
absl::optional<DataUpdateCb> data_update_cb = absl::nullopt)
294+
const ProviderOptions& options)
306295
: dispatcher_(main_dispatcher), tls_(tls), api_(api), data_transform_cb_(data_transform_cb),
307-
data_update_cb_(data_update_cb), options_(options) {}
296+
options_(options) {}
308297

309298
absl::StatusOr<DataSourceProviderSharedPtr<DataType>> getOrCreate(const ProtoDataSource& source) {
310299
ASSERT_IS_MAIN_OR_TEST_THREAD();
311300
if (!usesFileWatching(source, options_)) {
312-
return DataSourceProvider<DataType>::create(
313-
source, dispatcher_, tls_, api_, data_transform_cb_, options_, {}, data_update_cb_);
301+
return DataSourceProvider<DataType>::create(source, dispatcher_, tls_, api_,
302+
data_transform_cb_, options_, {});
314303
}
315304
const size_t config_hash = MessageUtil::hash(source);
316305
auto it = dynamic_providers_.find(config_hash);
@@ -347,7 +336,6 @@ class ProviderSingleton : public Singleton::Instance,
347336
ThreadLocal::SlotAllocator& tls_;
348337
Api::Api& api_;
349338
DataTransform<DataType> data_transform_cb_;
350-
absl::optional<DataUpdateCb> data_update_cb_;
351339
const ProviderOptions options_;
352340
absl::flat_hash_map<size_t, std::weak_ptr<DataSourceProvider<DataType>>> dynamic_providers_;
353341
};

0 commit comments

Comments
 (0)