PS-9148: Add caching of dictionary table for component_masking_functions #5275

oleksandr-kachan · 2024-03-27T14:43:51Z

https://perconadev.atlassian.net/browse/PS-9148

Added caching of mysql.masking_dictionaries table content.
Implemented masking_dictionaries_flush() UDF which flushes data from the masking dictionaries table to the memory cache.
Added component_masking.dictionaries_flush_interval_seconds system
variable.
Added actual flusher thread. It periodically rereads content of
dictionary table and updates in-memory cache.

github-actions

⚠️ Clang-Tidy found issue(s) with the introduced code (1/1)

github-actions · 2024-03-27T14:51:15Z

components/masking_functions/src/masking_functions/registration_routines.cpp

+// Flush the data from the masking dictionaries table to the memory cache.
+class masking_dictionaries_flush_impl {
+ public:
+  masking_dictionaries_flush_impl(mysqlpp::udf_context &ctx) {


⚠️ google-explicit-constructor ⚠️
single-argument constructors must be marked explicit to avoid unintentional implicit conversions

Suggested change

masking_dictionaries_flush_impl(mysqlpp::udf_context &ctx) {

explicit masking_dictionaries_flush_impl(mysqlpp::udf_context &ctx) {

github-actions · 2024-03-27T14:51:15Z

components/masking_functions/src/masking_functions/registration_routines.cpp

@@ -1229,6 +1237,7 @@
 DECLARE_STRING_UDF_AUTO(mask_uuid)
 DECLARE_STRING_UDF_AUTO(gen_blocklist)
 DECLARE_STRING_UDF_AUTO(gen_dictionary)
+DECLARE_STRING_UDF_AUTO(masking_dictionaries_flush)


⚠️ cppcoreguidelines-init-variables ⚠️
variable initid is not initialized

github-actions · 2024-03-27T14:51:15Z

components/masking_functions/src/masking_functions/registration_routines.cpp

@@ -1229,6 +1237,7 @@
 DECLARE_STRING_UDF_AUTO(mask_uuid)
 DECLARE_STRING_UDF_AUTO(gen_blocklist)
 DECLARE_STRING_UDF_AUTO(gen_dictionary)
+DECLARE_STRING_UDF_AUTO(masking_dictionaries_flush)


⚠️ misc-unused-parameters ⚠️
parameter initid is unused

github-actions · 2024-03-27T14:51:15Z

components/masking_functions/src/masking_functions/query_cache.cpp

+    return std::nullopt;
+  }
+
+  int random_step = rand() % std::distance(range.first, range.second);


⚠️ bugprone-narrowing-conversions ⚠️
narrowing conversion from long to signed type int is implementation-defined

components/masking_functions/include/masking_functions/query_cache.hpp

components/masking_functions/include/masking_functions/sql_context.hpp

components/masking_functions/src/masking_functions/query_cache.cpp

percona-ysorokin · 2024-03-27T17:31:14Z

components/masking_functions/src/masking_functions/query_cache.cpp

+bool query_cache::load_cache() {
+  auto query = global_query_builder::instance().select_all_from_dictionary();
+  auto result =
+      masking_functions::sql_context{global_command_services::instance()}


Should we make sql_context a member of query_cache to reuse it for different calls.
Still an open question, may not be the best decision when we add locks to support multithreading.

My impression was that this context is something specific to actual query execution (due to how it initializes and uses mysql_command_* services for each specific query) and it isn't a good idea to reuse same instance for different requests. And yes, reusing it may be an issue when multi-threading is involved.

components/masking_functions/src/masking_functions/query_cache.cpp

percona-ysorokin · 2024-03-27T17:38:24Z

components/masking_functions/src/masking_functions/sql_context.cpp

-  ulong *length = nullptr;
-  if ((*get_services().query_result->fetch_lengths)(mysql_res, &length) != 0)
-    throw std::runtime_error{"Couldn't fetch lenghts"};
+    result->emplace(std::make_pair(std::string{row[0], length[0]},


Use piecewise_construct here as well.

components/masking_functions/src/masking_functions/query_cache.cpp

github-actions

⚠️ Clang-Tidy found issue(s) with the introduced code (1/2)

components/masking_functions/include/masking_functions/dictionary_container.hpp

github-actions · 2024-04-08T10:58:39Z

components/masking_functions/include/masking_functions/dictionary_container.hpp

+  struct term_container {
+    explicit term_container(std::string term)
+        : term_mutex{}, term_list{std::move(term)} {}
+    mutable std::shared_mutex term_mutex;


⚠️ misc-non-private-member-variables-in-classes ⚠️
member variable term_mutex has public visibility

github-actions · 2024-04-08T10:58:39Z

components/masking_functions/include/masking_functions/dictionary_container.hpp

+    explicit term_container(std::string term)
+        : term_mutex{}, term_list{std::move(term)} {}
+    mutable std::shared_mutex term_mutex;
+    std::set<std::string> term_list;


⚠️ misc-non-private-member-variables-in-classes ⚠️
member variable term_list has public visibility

github-actions · 2024-04-08T10:58:39Z

components/masking_functions/src/component.cpp

@@ -174,6 +195,8 @@ BEGIN_COMPONENT_REQUIRES(CURRENT_COMPONENT_NAME)
  REQUIRES_SERVICE(mysql_string_substr),
  REQUIRES_SERVICE(mysql_string_compare),

+  REQUIRES_PSI_THREAD_SERVICE,


⚠️ cppcoreguidelines-pro-type-const-cast ⚠️
do not use const_cast

github-actions · 2024-04-08T10:58:39Z

components/masking_functions/src/component.cpp

@@ -187,6 +210,8 @@
  REQUIRES_SERVICE(mysql_current_thread_reader),
  REQUIRES_SERVICE(mysql_thd_security_context),
  REQUIRES_SERVICE(global_grants_check),
+  REQUIRES_SERVICE(component_sys_variable_register),


⚠️ cppcoreguidelines-pro-type-const-cast ⚠️
do not use const_cast

github-actions · 2024-04-08T10:58:39Z

components/masking_functions/src/component.cpp

@@ -187,6 +210,8 @@
  REQUIRES_SERVICE(mysql_current_thread_reader),
  REQUIRES_SERVICE(mysql_thd_security_context),
  REQUIRES_SERVICE(global_grants_check),
+  REQUIRES_SERVICE(component_sys_variable_register),
+  REQUIRES_SERVICE(component_sys_variable_unregister),


⚠️ cppcoreguidelines-pro-type-const-cast ⚠️
do not use const_cast

github-actions · 2024-04-08T10:58:39Z

components/masking_functions/src/masking_functions/sys_vars.cpp

+#include <mysql/components/services/component_sys_var_service.h>
+#include <mysql/components/services/log_builtins.h>
+
+#include <mysqld_error.h>


⚠️ clang-diagnostic-error ⚠️
mysqld_error.h file not found

github-actions · 2024-04-08T10:58:39Z

components/masking_functions/src/masking_functions/sys_vars.cpp

+  if (database_name == nullptr || strlen(database_name) == 0) {
+    LogComponentErr(ERROR_LEVEL, ER_LOG_PRINTF_MSG,
+                    "Bad masking_functions.masking_database value");
+    return false;
+  }
+
+  return true;


⚠️ readability-simplify-boolean-expr ⚠️
redundant boolean literal in conditional return statement

Suggested change

if (database_name == nullptr || strlen(database_name) == 0) {

LogComponentErr(ERROR_LEVEL, ER_LOG_PRINTF_MSG,

"Bad masking_functions.masking_database value");

return false;

}

return true;

return !(database_name == nullptr || strlen(database_name) == 0);

github-actions · 2024-04-08T10:58:39Z

components/masking_functions/src/masking_functions/query_cache.cpp

+
+#include <mysql/components/services/log_builtins.h>
+#include <mysql/psi/mysql_thread.h>
+#include <mysqld_error.h>


⚠️ clang-diagnostic-error ⚠️
mysqld_error.h file not found

github-actions · 2024-04-08T10:58:40Z

components/masking_functions/src/masking_functions/query_cache.cpp

+    masking_functions::primitive_singleton<masking_functions::query_builder>;
+
+void *run_dict_flusher(void *arg) {
+  auto *self = reinterpret_cast<masking_functions::query_cache *>(arg);


⚠️ cppcoreguidelines-pro-type-reinterpret-cast ⚠️
do not use reinterpret_cast

github-actions

⚠️ Clang-Tidy found issue(s) with the introduced code (2/2)

github-actions · 2024-04-08T10:58:51Z

components/masking_functions/include/masking_functions/query_cache.hpp

+  std::mutex m_flusher_mutex;
+  std::condition_variable m_flusher_condition_var;
+
+  PSI_thread_key m_psi_flusher_thread_key;


⚠️ cppcoreguidelines-pro-type-member-init ⚠️
constructor does not initialize these fields: m_psi_flusher_thread_key, m_flusher_thread_attr

Suggested change

PSI_thread_key m_psi_flusher_thread_key;

PSI_thread_key m_psi_flusher_thread_key{};

github-actions · 2024-04-08T10:58:51Z

components/masking_functions/include/masking_functions/query_cache.hpp

+
+  PSI_thread_key m_psi_flusher_thread_key;
+  my_thread_handle m_flusher_thread;
+  my_thread_attr_t m_flusher_thread_attr;


⚠️ cppcoreguidelines-pro-type-member-init ⚠️
constructor does not initialize these fields: m_psi_flusher_thread_key, m_flusher_thread_attr

Suggested change

my_thread_attr_t m_flusher_thread_attr;

my_thread_attr_t m_flusher_thread_attr{};

github-actions · 2024-04-08T10:58:51Z

components/masking_functions/src/masking_functions/query_cache.cpp

+}
+
+void query_cache::init_thd() noexcept {
+  auto *thd = new THD;


⚠️ cppcoreguidelines-owning-memory ⚠️
initializing non-owner THD * with a newly created gsl::owner<>

github-actions · 2024-04-08T10:58:51Z

components/masking_functions/src/masking_functions/query_cache.cpp

+}
+
+void query_cache::init_thd() noexcept {
+  auto *thd = new THD;


⚠️ bugprone-unhandled-exception-at-new ⚠️
missing exception handler for allocation failure at new

github-actions · 2024-04-08T10:58:51Z

components/masking_functions/src/masking_functions/query_cache.cpp

+  auto *thd = new THD;
+  my_thread_init();
+  thd->set_new_thread_id();
+  thd->thread_stack = reinterpret_cast<char *>(&thd);


⚠️ cppcoreguidelines-pro-type-reinterpret-cast ⚠️
do not use reinterpret_cast

github-actions · 2024-04-08T10:58:52Z

components/masking_functions/include/masking_functions/query_cache.hpp

+  bool load_cache();
+
+  void init_thd() noexcept;
+  void release_thd() noexcept;


⚠️ readability-convert-member-functions-to-static ⚠️
method release_thd can be made static

Suggested change

void release_thd() noexcept;

static void release_thd() noexcept;

github-actions · 2024-04-08T10:58:52Z

components/masking_functions/include/masking_functions/query_cache.hpp

+  bool remove(const std::string &dictionary_name);
+  bool remove(const std::string &dictionary_name, const std::string &term);
+  bool insert(const std::string &dictionary_name, const std::string &term);
+  bool load_cache();


⚠️ readability-convert-member-functions-to-static ⚠️
method load_cache can be made static

Suggested change

bool load_cache();

static bool load_cache();

https://perconadev.atlassian.net/browse/PS-9148 - Added caching of mysql.masking_dictionaries table content. - Implemented masking_dictionaries_flush() UDF which flushes data from the masking dictionaries table to the memory cache.

https://perconadev.atlassian.net/browse/PS-9148 The masking_functions.masking_database system variable for the masking_functions component specifies database used for data masking dictionaries.

https://perconadev.atlassian.net/browse/PS-9148 - Added component_masking.dictionaries_flush_interval_seconds system variable. - Added actual flusher thread. It periodically rereads content of dictionary table and updates in-memory cache.

github-actions

⚠️ Clang-Tidy found issue(s) with the introduced code (1/2)

github-actions · 2024-04-25T10:20:42Z

components/masking_functions/include/masking_functions/dictionary.hpp

+
+namespace masking_functions {
+
+class dictionary {


⚠️ cppcoreguidelines-special-member-functions ⚠️
class dictionary defines a copy constructor, a copy assignment operator, a move constructor and a move assignment operator but does not define a destructor

github-actions · 2024-04-25T10:20:42Z

components/masking_functions/include/masking_functions/query_cache.hpp

+  std::unique_ptr<THD> m_flusher_thd;
+
+  void init_thd() noexcept;
+  void release_thd() noexcept;


⚠️ readability-convert-member-functions-to-static ⚠️
method release_thd can be made static

Suggested change

void release_thd() noexcept;

static void release_thd() noexcept;

github-actions · 2024-04-25T10:20:42Z

components/masking_functions/src/masking_functions/query_cache.cpp

+}
+
+void *query_cache::run_dict_flusher(void *arg) {
+  auto *self = reinterpret_cast<masking_functions::query_cache *>(arg);


⚠️ cppcoreguidelines-pro-type-reinterpret-cast ⚠️
do not use reinterpret_cast

github-actions · 2024-04-25T10:20:42Z

components/masking_functions/src/masking_functions/dictionary.cpp

+
+namespace masking_functions {
+
+dictionary::dictionary(const std::string &term) : terms_{}, terms_mutex_{} {


⚠️ readability-redundant-member-init ⚠️
initializer for member terms_ is redundant

Suggested change

dictionary::dictionary(const std::string &term) : terms_{}, terms_mutex_{} {

dictionary::dictionary(const std::string &term) : , terms_mutex_{} {

github-actions · 2024-04-25T10:20:42Z

components/masking_functions/src/masking_functions/dictionary.cpp

+
+namespace masking_functions {
+
+dictionary::dictionary(const std::string &term) : terms_{}, terms_mutex_{} {


⚠️ readability-redundant-member-init ⚠️
initializer for member terms_mutex_ is redundant

Suggested change

dictionary::dictionary(const std::string &term) : terms_{}, terms_mutex_{} {

dictionary::dictionary(const std::string &term) : terms_{}, {

github-actions · 2024-04-25T10:20:42Z

components/masking_functions/src/masking_functions/dictionary.cpp

+  }
+
+  const auto random_index{random_number(0, terms_.size() - 1U)};
+  return *std::next(terms_.begin(), random_index);


⚠️ bugprone-narrowing-conversions ⚠️
narrowing conversion from std::size_t (aka unsigned long) to signed type typename iterator_traits<_Node_const_iterator<basic_string<char>, true, true>>::difference_type (aka long) is implementation-defined

github-actions · 2024-04-25T10:20:42Z

components/masking_functions/src/masking_functions/dictionary.cpp

+}
+
+bool dictionary::insert(const std::string &term) {
+  std::unique_lock terms_write_lock{terms_mutex_};


⚠️ clang-diagnostic-error ⚠️
no type named unique_lock in namespace std

github-actions · 2024-04-25T10:20:43Z

components/masking_functions/src/masking_functions/dictionary.cpp

+}
+
+bool dictionary::insert(const std::string &term) {
+  std::unique_lock terms_write_lock{terms_mutex_};


⚠️ cppcoreguidelines-init-variables ⚠️
variable terms_write_lock is not initialized

Suggested change

std::unique_lock terms_write_lock{terms_mutex_};

std::unique_lock terms_write_lock = 0{terms_mutex_};

github-actions · 2024-04-25T10:20:43Z

components/masking_functions/src/masking_functions/dictionary.cpp

+}
+
+bool dictionary::remove(const std::string &term) noexcept {
+  std::unique_lock terms_write_lock{terms_mutex_};


⚠️ clang-diagnostic-error ⚠️
no type named unique_lock in namespace std

github-actions · 2024-04-25T10:20:43Z

components/masking_functions/src/masking_functions/dictionary.cpp

+}
+
+bool dictionary::remove(const std::string &term) noexcept {
+  std::unique_lock terms_write_lock{terms_mutex_};


⚠️ cppcoreguidelines-init-variables ⚠️
variable terms_write_lock is not initialized

Suggested change

std::unique_lock terms_write_lock{terms_mutex_};

std::unique_lock terms_write_lock = 0{terms_mutex_};

github-actions

⚠️ Clang-Tidy found issue(s) with the introduced code (2/2)

github-actions · 2024-04-25T10:20:54Z

components/masking_functions/src/masking_functions/bookshelf.cpp

+}
+
+bool bookshelf::remove(const std::string &dictionary_name) noexcept {
+  std::unique_lock dictionaries_write_lock{dictionaries_mutex_};


⚠️ clang-diagnostic-error ⚠️
no type named unique_lock in namespace std

github-actions · 2024-04-25T10:20:54Z

components/masking_functions/src/masking_functions/bookshelf.cpp

+}
+
+bool bookshelf::remove(const std::string &dictionary_name) noexcept {
+  std::unique_lock dictionaries_write_lock{dictionaries_mutex_};


⚠️ cppcoreguidelines-init-variables ⚠️
variable dictionaries_write_lock is not initialized

Suggested change

std::unique_lock dictionaries_write_lock{dictionaries_mutex_};

std::unique_lock dictionaries_write_lock = 0{dictionaries_mutex_};

github-actions · 2024-04-25T10:20:54Z

components/masking_functions/src/masking_functions/bookshelf.cpp

+  // if no dictionary with such name alteady exist, we need to
+  // create it under a write lock
+  {
+    std::unique_lock dictionaries_write_lock{dictionaries_mutex_};


⚠️ clang-diagnostic-error ⚠️
no type named unique_lock in namespace std

github-actions · 2024-04-25T10:20:54Z

components/masking_functions/src/masking_functions/bookshelf.cpp

+  // if no dictionary with such name alteady exist, we need to
+  // create it under a write lock
+  {
+    std::unique_lock dictionaries_write_lock{dictionaries_mutex_};


⚠️ cppcoreguidelines-init-variables ⚠️
variable dictionaries_write_lock is not initialized

Suggested change

std::unique_lock dictionaries_write_lock{dictionaries_mutex_};

std::unique_lock dictionaries_write_lock = 0{dictionaries_mutex_};

github-actions · 2024-04-25T10:20:54Z

components/masking_functions/src/component.cpp

@@ -33,9 +35,13 @@
 #include <mysqld_error.h>


⚠️ clang-diagnostic-error ⚠️
mysqld_error.h file not found

github-actions · 2024-04-25T10:20:55Z

components/masking_functions/src/component.cpp

+    return 1;
+  }
+
+  std::string check_error_message;


⚠️ cppcoreguidelines-init-variables ⚠️
variable check_error_message is not initialized

Suggested change

std::string check_error_message;

std::string check_error_message = 0;

github-actions · 2024-04-25T10:20:55Z

components/masking_functions/include/masking_functions/bookshelf.hpp

+
+namespace masking_functions {
+
+class bookshelf {


⚠️ cppcoreguidelines-special-member-functions ⚠️
class bookshelf defines a copy assignment operator, a move constructor and a move assignment operator but does not define a destructor or a copy constructor

https://perconadev.atlassian.net/browse/PS-9148 Introduced 'dictionary' and 'bookshelf' classes for storing terms on per-dictionary level. Reworked 'query_cache' to utilize these two new classes.

https://perconadev.atlassian.net/browse/PS-9148 Introduced 'component_sys_variable_service_tuple' class for groupping comonent system variable registration services (supposed to be used with 'primitive_singleton' class template). 'query_cache' now expects 'query_builder' and 'flusher_interval_seconds' as its constructor's parameters. Eliminates custom MySQL types (like 'ulonglong') and its includes (like 'my_inttypes.h') from the publicly facing headers. 'query_cache' is now explicitly initialized / deinitialized in the component's 'init()'' / 'deinit()'' functions via 'primitive_singleton' interface. 'query_cache' helper thread-related methods made private.

https://perconadev.atlassian.net/browse/PS-9148 As std::string_view::data() is not guaranteed to be null-terminated, it is not safe to use it in old c-functions accepting 'const char *'. Some constants converted to arrays of char 'const char buffer[]{"value"}'.

https://perconadev.atlassian.net/browse/PS-9148 'command_service_tuple' struct extended with one more member - 'field_info' service. Reworked 'query_cache' class: instead of loading terms from the database in constructor, this operation is now performed in first attempt to access one of the dictionary methods ('contains()' / 'get_random()' / 'remove()' / 'insert()'). This is done in order to overcome a limitation that does not allow 'mysql_command_query' service to be used from inside the componment initialization function. Fixed problem with 'm_dict_cache' shared pointer updated concurrently from different threads. Exceptions thrown from the cache loading function no longer escape the flusher thread. De-coupled 'sql_context' and 'bookshelf' classes: 'sql_context' now accepts a generic insertion callback that can be used to populate any type of containers. 'component_masking_functions.dictionary_operations' MTR test case extended with additional checks for flushed / unflushed dictionary cache.

github-actions

⚠️ Clang-Tidy found issue(s) with the introduced code (1/1)

github-actions · 2024-04-30T11:05:14Z

components/masking_functions/src/component.cpp

  REQUIRES_SERVICE(mysql_command_query),
  REQUIRES_SERVICE(mysql_command_query_result),
+  REQUIRES_SERVICE(mysql_command_field_info),


⚠️ cppcoreguidelines-pro-type-const-cast ⚠️
do not use const_cast

github-actions · 2024-04-30T11:05:15Z

components/masking_functions/src/masking_functions/query_cache.cpp

+query_cache::query_cache(query_builder_ptr query_builder,
+                         std::uint64_t flusher_interval_seconds)
+    : m_query_builder{std::move(query_builder)},
+      m_dict_cache{},


⚠️ readability-redundant-member-init ⚠️
initializer for member m_dict_cache is redundant

Suggested change

m_dict_cache{},

,

github-actions · 2024-04-30T11:05:15Z

components/masking_functions/src/masking_functions/query_cache.cpp

+    masking_functions::sql_context sql_ctx{global_command_services::instance()};
+    auto query = m_query_builder->select_all_from_dictionary();
+    auto local_dict_cache{std::make_shared<bookshelf>()};
+    sql_context::row_callback<2> result_inserter{[&terms = *local_dict_cache](


⚠️ cppcoreguidelines-init-variables ⚠️
variable result_inserter is not initialized

Suggested change

sql_context::row_callback<2> result_inserter{[&terms = *local_dict_cache](

sql_context::row_callback<2> result_inserter = 0{[&terms = *local_dict_cache](

github-actions · 2024-04-30T11:05:15Z

components/masking_functions/src/masking_functions/sql_context.cpp

+                                     query.length()) != 0) {
+    return false;
+  }
+  std::uint64_t row_count = 0;


⚠️ clang-diagnostic-error ⚠️
no type named uint64_t in namespace std; did you mean simply uint64_t?

Suggested change

std::uint64_t row_count = 0;

uint64_t row_count = 0;

github-actions · 2024-04-30T11:05:15Z

components/masking_functions/src/masking_functions/sql_context.cpp

+                                     query.length()) != 0) {
+    return false;
+  }
+  std::uint64_t row_count = 0;


⚠️ cppcoreguidelines-init-variables ⚠️
variable row_count is not initialized

Suggested change

std::uint64_t row_count = 0;

std::uint64_t row_count = 0 = 0;

github-actions · 2024-04-30T11:05:15Z

components/masking_functions/src/masking_functions/sql_context.cpp

  if ((*get_services().query->query)(to_mysql_h(impl_.get()), query.data(),
                                     query.length()) != 0) {
    throw std::runtime_error{"Error while executing SQL query"};
  }

+  unsigned int actual_number_of_fields;


⚠️ cppcoreguidelines-init-variables ⚠️
variable actual_number_of_fields is not initialized

Suggested change

unsigned int actual_number_of_fields;

unsigned int actual_number_of_fields = 0;

github-actions · 2024-04-30T11:05:15Z

components/masking_functions/src/masking_functions/sql_context.cpp

@@ -107,7 +131,7 @@
      std::unique_ptr<mysql_res_type, decltype(mysql_res_deleter)>;

  mysql_res_ptr mysql_res_guard(mysql_res, std::move(mysql_res_deleter));
-  uint64_t row_count = 0;
+  std::uint64_t row_count = 0;


⚠️ clang-diagnostic-error ⚠️
no type named uint64_t in namespace std; did you mean simply uint64_t?

Suggested change

std::uint64_t row_count = 0;

uint64_t row_count = 0;

github-actions · 2024-04-30T11:05:15Z

components/masking_functions/src/masking_functions/sql_context.cpp

@@ -107,7 +131,7 @@
      std::unique_ptr<mysql_res_type, decltype(mysql_res_deleter)>;

  mysql_res_ptr mysql_res_guard(mysql_res, std::move(mysql_res_deleter));
-  uint64_t row_count = 0;
+  std::uint64_t row_count = 0;


⚠️ cppcoreguidelines-init-variables ⚠️
variable row_count is not initialized

Suggested change

std::uint64_t row_count = 0;

std::uint64_t row_count = 0 = 0;

https://perconadev.atlassian.net/browse/PS-9148 Both 'dictionary' and 'bookshelf' classes no longer include their own 'std::shared_mutex' to protect data. Instead, we now have a single 'std::shared_mutex' at the 'query_cache' level. The return value of the 'get_random()' method in both 'dictionary' and 'bookshelf' classes changed from 'optional_string' to 'std::string_view'. Empty (default constructed) 'std::string_view' is used as an indicator of an unsuccessful operation. 'get_random()' method in the 'query_cache' class still returns a string by value to avoid race conditions. Changed the behaviour of the 'sql_context::execute_dml()' method - it now throws when SQL errors (like "no table found", etc.) occur.

github-actions

⚠️ Clang-Tidy found issue(s) with the introduced code (1/1)

github-actions · 2024-05-02T14:55:51Z

components/masking_functions/src/masking_functions/dictionary.cpp

+  }
+
+  const auto random_index{random_number(0, terms_.size() - 1U)};
+  return *std::next(std::begin(terms_), random_index);


⚠️ bugprone-narrowing-conversions ⚠️
narrowing conversion from std::size_t (aka unsigned long) to signed type typename iterator_traits<_Node_const_iterator<basic_string<char>, true, true>>::difference_type (aka long) is implementation-defined

github-actions · 2024-05-02T14:55:51Z

components/masking_functions/include/masking_functions/bookshelf.hpp

+
+namespace masking_functions {
+
+class bookshelf {


⚠️ cppcoreguidelines-special-member-functions ⚠️
class bookshelf defines a non-default destructor, a copy assignment operator, a move constructor and a move assignment operator but does not define a copy constructor

github-actions · 2024-05-02T14:55:52Z

components/masking_functions/include/masking_functions/query_cache.hpp

+  std::mutex flusher_mutex_;
+  std::condition_variable flusher_condition_var_;
+
+  PSI_thread_key psi_flusher_thread_key_;


⚠️ cppcoreguidelines-pro-type-member-init ⚠️
constructor does not initialize these fields: psi_flusher_thread_key_, flusher_thread_attr_

Suggested change

PSI_thread_key psi_flusher_thread_key_;

PSI_thread_key psi_flusher_thread_key_{};

github-actions · 2024-05-02T14:55:52Z

components/masking_functions/include/masking_functions/query_cache.hpp

+
+  PSI_thread_key psi_flusher_thread_key_;
+  my_thread_handle flusher_thread_;
+  my_thread_attr_t flusher_thread_attr_;


⚠️ cppcoreguidelines-pro-type-member-init ⚠️
constructor does not initialize these fields: psi_flusher_thread_key_, flusher_thread_attr_

Suggested change

my_thread_attr_t flusher_thread_attr_;

my_thread_attr_t flusher_thread_attr_{};

github-actions · 2024-05-02T14:55:52Z

components/masking_functions/src/masking_functions/query_cache.cpp

+                         std::uint64_t flusher_interval_seconds)
+    : dict_query_builder_{std::move(query_builder)},
+      dict_cache_{},
+      dict_cache_mutex_{},


⚠️ readability-redundant-member-init ⚠️
initializer for member dict_cache_mutex_ is redundant

Suggested change

dict_cache_mutex_{},

,

github-actions · 2024-05-02T14:55:52Z

components/masking_functions/src/masking_functions/query_cache.cpp

+}
+
+bool query_cache::contains(const std::string &dictionary_name,
+                           const std::string &term) const {


⚠️ readability-convert-member-functions-to-static ⚠️
method contains can be made static

Suggested change

const std::string &term) const {

const std::string &term) {

github-actions · 2024-05-02T14:55:52Z

components/masking_functions/include/masking_functions/query_cache.hpp

+  bool contains(const std::string &dictionary_name,
+                const std::string &term) const;


⚠️ readability-convert-member-functions-to-static ⚠️
method contains can be made static

Suggested change

bool contains(const std::string &dictionary_name,

const std::string &term) const;

static bool contains(const std::string &dictionary_name,

const std::string &term) ;

github-actions · 2024-05-02T14:55:52Z

components/masking_functions/src/masking_functions/query_cache.cpp

+  return acquired_dict_cache.contains(dictionary_name, term);
+}
+
+std::string query_cache::get_random(const std::string &dictionary_name) const {


⚠️ readability-convert-member-functions-to-static ⚠️
method get_random can be made static

Suggested change

std::string query_cache::get_random(const std::string &dictionary_name) const {

std::string query_cache::get_random(const std::string &dictionary_name) {

github-actions · 2024-05-02T14:55:52Z

components/masking_functions/include/masking_functions/query_cache.hpp

+                const std::string &term) const;
+  // returns a copy of the string to avoid race conditions
+  // an empty string is returned if the dictionary does not exist
+  std::string get_random(const std::string &dictionary_name) const;


⚠️ readability-convert-member-functions-to-static ⚠️
method get_random can be made static

Suggested change

std::string get_random(const std::string &dictionary_name) const;

static std::string get_random(const std::string &dictionary_name) ;

oleksandr-kachan requested a review from percona-ysorokin March 27, 2024 14:43

github-actions bot reviewed Mar 27, 2024

View reviewed changes

oleksandr-kachan force-pushed the PS-9148-8.0 branch from 4f3e70f to 21035cd Compare March 27, 2024 14:51

percona-ysorokin reviewed Mar 27, 2024

View reviewed changes

oleksandr-kachan force-pushed the PS-9148-8.0 branch from 21035cd to e05258c Compare March 29, 2024 17:21

oleksandr-kachan force-pushed the PS-9148-8.0 branch 2 times, most recently from 2bdd7f5 to dd83250 Compare April 8, 2024 10:51

github-actions bot reviewed Apr 8, 2024

View reviewed changes

oleksandr-kachan added 3 commits April 10, 2024 11:53

PS-9148: Add masking_functions.masking_database sys var support

15cfaae

https://perconadev.atlassian.net/browse/PS-9148 The masking_functions.masking_database system variable for the masking_functions component specifies database used for data masking dictionaries.

oleksandr-kachan force-pushed the PS-9148-8.0 branch from dd83250 to 58517cf Compare April 10, 2024 08:57

oleksandr-kachan requested a review from percona-ysorokin April 10, 2024 08:57

github-actions bot reviewed Apr 25, 2024

View reviewed changes

percona-ysorokin added 3 commits April 25, 2024 15:26

PS-9148: Implemented hierarchical storage for dictionaries and terms

d505232

https://perconadev.atlassian.net/browse/PS-9148 Introduced 'dictionary' and 'bookshelf' classes for storing terms on per-dictionary level. Reworked 'query_cache' to utilize these two new classes.

oleksandr-kachan force-pushed the PS-9148-8.0 branch from d2def5c to 6fd4883 Compare April 25, 2024 15:32

github-actions bot reviewed Apr 30, 2024

View reviewed changes

github-actions bot reviewed May 2, 2024

View reviewed changes

	masking_dictionaries_flush_impl(mysqlpp::udf_context &ctx) {
	explicit masking_dictionaries_flush_impl(mysqlpp::udf_context &ctx) {

	PSI_thread_key m_psi_flusher_thread_key;
	PSI_thread_key m_psi_flusher_thread_key{};

	my_thread_attr_t m_flusher_thread_attr;
	my_thread_attr_t m_flusher_thread_attr{};

	void release_thd() noexcept;
	static void release_thd() noexcept;


		namespace masking_functions {

		dictionary::dictionary(const std::string &term) : terms_{}, terms_mutex_{} {

	std::unique_lock terms_write_lock{terms_mutex_};
	std::unique_lock terms_write_lock = 0{terms_mutex_};

	std::unique_lock dictionaries_write_lock{dictionaries_mutex_};
	std::unique_lock dictionaries_write_lock = 0{dictionaries_mutex_};

	std::string check_error_message;
	std::string check_error_message = 0;

	sql_context::row_callback<2> result_inserter{[&terms = *local_dict_cache](
	sql_context::row_callback<2> result_inserter = 0{[&terms = *local_dict_cache](

	std::uint64_t row_count = 0;
	std::uint64_t row_count = 0 = 0;

	unsigned int actual_number_of_fields;
	unsigned int actual_number_of_fields = 0;

	PSI_thread_key psi_flusher_thread_key_;
	PSI_thread_key psi_flusher_thread_key_{};

	my_thread_attr_t flusher_thread_attr_;
	my_thread_attr_t flusher_thread_attr_{};

		bool contains(const std::string &dictionary_name,
		const std::string &term) const;

	std::string query_cache::get_random(const std::string &dictionary_name) const {
	std::string query_cache::get_random(const std::string &dictionary_name) {

	std::string get_random(const std::string &dictionary_name) const;
	static std::string get_random(const std::string &dictionary_name) ;

PS-9148: Add caching of dictionary table for component_masking_functions #5275

Are you sure you want to change the base?

PS-9148: Add caching of dictionary table for component_masking_functions #5275

Conversation

oleksandr-kachan commented Mar 27, 2024 • edited Loading

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot Mar 27, 2024

Choose a reason for hiding this comment

github-actions bot Mar 27, 2024

Choose a reason for hiding this comment

github-actions bot Mar 27, 2024

Choose a reason for hiding this comment

github-actions bot Mar 27, 2024

Choose a reason for hiding this comment

percona-ysorokin Mar 27, 2024

Choose a reason for hiding this comment

oleksandr-kachan Apr 10, 2024

Choose a reason for hiding this comment

percona-ysorokin Mar 27, 2024

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot Apr 8, 2024

Choose a reason for hiding this comment

github-actions bot Apr 8, 2024

Choose a reason for hiding this comment

github-actions bot Apr 8, 2024

Choose a reason for hiding this comment

github-actions bot Apr 8, 2024

Choose a reason for hiding this comment

github-actions bot Apr 8, 2024

Choose a reason for hiding this comment

github-actions bot Apr 8, 2024

Choose a reason for hiding this comment

github-actions bot Apr 8, 2024

Choose a reason for hiding this comment

github-actions bot Apr 8, 2024

Choose a reason for hiding this comment

github-actions bot Apr 8, 2024

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot Apr 8, 2024

Choose a reason for hiding this comment

github-actions bot Apr 8, 2024

Choose a reason for hiding this comment

github-actions bot Apr 8, 2024

Choose a reason for hiding this comment

github-actions bot Apr 8, 2024

Choose a reason for hiding this comment

github-actions bot Apr 8, 2024

Choose a reason for hiding this comment

github-actions bot Apr 8, 2024

Choose a reason for hiding this comment

github-actions bot Apr 8, 2024

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot Apr 25, 2024

Choose a reason for hiding this comment

github-actions bot Apr 25, 2024

Choose a reason for hiding this comment

github-actions bot Apr 25, 2024

Choose a reason for hiding this comment

github-actions bot Apr 25, 2024

Choose a reason for hiding this comment

github-actions bot Apr 25, 2024

Choose a reason for hiding this comment

github-actions bot Apr 25, 2024

Choose a reason for hiding this comment

github-actions bot Apr 25, 2024

Choose a reason for hiding this comment

github-actions bot Apr 25, 2024

Choose a reason for hiding this comment

github-actions bot Apr 25, 2024

Choose a reason for hiding this comment

github-actions bot Apr 25, 2024

Choose a reason for hiding this comment

github-actions bot left a comment

oleksandr-kachan commented Mar 27, 2024 •

edited

Loading