Skip to content

Conversation

@alexeykopytov
Copy link

@alexeykopytov alexeykopytov commented Apr 20, 2025

Add a new system variable mydb_dedicated_server with the functionality described in the Github issue.

The variable is ON by default, which creates some challenges for MTR testing:

  • the amount of resources allocated by mysqld with that variable set to ON is incompatible with the typical use case of MTR running multiple jobs and servers in parallel;

  • values of other variables that are auto-tuned by mydb_dedicated_server may depend on hardware resoures and OS limits

Changing a certain variable from its default value for MTR runs is not as simple as it may seem. Test suites or individual tests may override whatever was set up by MTR. I tried a few changes to MTR code or its default .cnf files, and none of them was reliable enough. I ended up introducing a new environment variable `MTR_TEST_MODE``.

When set to any value, it makes certain variables change their default values to the ones that make more sense in the MTR testing context. For now, the only affected variable is mydb_dedicated_server, but more variables may be added later.

Closes GH-1.

@alexeykopytov alexeykopytov force-pushed the mydb_dedicated_server branch from e97ad69 to 11a4950 Compare April 20, 2025 19:35
@ihanick
Copy link

ihanick commented Apr 20, 2025

This change will be problematic for MyDB installed in containers (docker/kubernetes) if the server has huge amount of RAM, but just few GB is granted for MyDB.

Can we consider cgroups memory limit check in my_physical_memory?

    std::ifstream file("/sys/fs/cgroup/memory.max");
    if (!file.is_open()) {
        file.open("/sys/fs/cgroup/memory/memory.limit_in_bytes");
        if (!file.is_open()) {
            return 0;
        }
    }
    std::string line;
    std::getline(file, line);
    
    if (line == "max") {
        return 0;
    }

@alexeykopytov alexeykopytov force-pushed the mydb_dedicated_server branch from 11a4950 to 41223b4 Compare April 20, 2025 20:46
@alexeykopytov
Copy link
Author

This change will be problematic for MyDB installed in containers (docker/kubernetes) if the server has huge amount of RAM, but just few GB is granted for MyDB.

Can we consider cgroups memory limit check in my_physical_memory?

    std::ifstream file("/sys/fs/cgroup/memory.max");
    if (!file.is_open()) {
        file.open("/sys/fs/cgroup/memory/memory.limit_in_bytes");
        if (!file.is_open()) {
            return 0;
        }
    }
    std::string line;
    std::getline(file, line);
    
    if (line == "max") {
        return 0;
    }

Right, but this is technically an innodb_dedicated_server issue, which was addressed upstream in 9.3.0:

InnoDB now supports container-aware resource allocation, allowing it to adhere to the restrictions imposed by the container. The default values of InnoDB configurations are now calculated based on the logical CPUs and physical memory allocated by the container, rather than relying on system-wide resources.

For now, we can consider containers a "non-dedicated" server. Backporting container-aware allocation from 9.3.0 looks reasonable.

Add a new system variable `mydb_dedicated_server` with the functionality
described in the Github issue.

The variable is ON by default, which creates some challenges for MTR
testing:

- the amount of resources allocated by mysqld with that variable set to
ON is incompatible with the typical use case of MTR running multiple
jobs and servers in parallel;

- values of other variables that are auto-tuned by
`mydb_dedicated_server` may depend on hardware resoures and OS limits

Changing a certain variable from its default value for MTR runs is not
as simple as it may seem. Test suites or individual tests may override
whatever was set up by MTR. I tried a few changes to MTR code or its
default .cnf files, and none of them was reliable enough. I ended up
introducing a new environment variable `MTR_TEST_MODE`.

When set to any value, it makes certain variables change their default
values to the ones that make more sense in the MTR testing context. For
now, the only affected variable is `mydb_dedicated_server`, but more
variables may be added later.

Closes GH-1.
@alexeykopytov alexeykopytov force-pushed the mydb_dedicated_server branch from 41223b4 to dcfb512 Compare April 21, 2025 16:36
@alexeykopytov alexeykopytov merged commit 6ff7cde into 8.4 Apr 22, 2025
1 of 2 checks passed
@alexeykopytov alexeykopytov deleted the mydb_dedicated_server branch April 23, 2025 07:04
alexeykopytov pushed a commit that referenced this pull request Jul 26, 2025
…tion fault

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

Problem
-------
When changing binlog_transaction_dependency_tracking in high load
workload, MySQL can get a segmentation fault.

Analysis
--------
Address sanitizer runs exposed the heap-use-after-free.

READ of size 8 at 0x6030002c3298 thread T52
    #0 _M_hash_code()
    #1 _M_bucket_index()
    ..
    #7 std::unordered_map::insert()
    #8 Writeset_trx_dependency_tracker::get_dependency()
    #9 Transaction_dependency_tracker::get_dependency()
    #10 MYSQL_BIN_LOG::write_transaction()
    #11 binlog_cache_data::flush()
    #12 binlog_cache_mngr::flush()
    #13 MYSQL_BIN_LOG::flush_thread_caches()
    #14 MYSQL_BIN_LOG::process_flush_stage_queue()
    #15 MYSQL_BIN_LOG::ordered_commit()
    #16 MYSQL_BIN_LOG::commit()

freed by thread T49 here:
    #0 operator delete()
    ...
    #7 std::unordered_map::clear()
    #8 Writeset_trx_dependency_tracker::rotate(long)
    #9 Transaction_dependency_tracker::tracking_mode_changed()
    #10 update_binlog_transaction_dependency_tracking
    #11 sys_var::update()

- The Writeset_trx_dependency_tracker uses std::unordered_map for
  storing depdendency information.
- When a transaction is committing, the committing thread inserts the
  dependency information to this map in through get_dependency().
- When the tracking mode is changed, then the map is cleared by
  Writeset_trx_dependency_tracker::rotate(). Note that no lock/mutex is
  taken during the rotation.
- As the rotate() and get_dependency() operations can be concurrently
  called from different threads and there is no mutex protection to
  handle it, it can result in segmentation fault when the get_dependency()
  tries to insert to the already deleted map.

Solution
--------
Use std::shared_ptr with atomic load/store for safer dependency tracker map rotation.

- Replaced direct usage of of map with std::shared_ptr in the
  Writeset_trx_dependency_tracker class.
- Modified the implementation of rotate() to used std::atomic_load and
  std::atomic_store to enable thread-safe reads and rotations.

With the new solution the rotation happens in an atomic manner. So that
transactions calling get_dependency() always use the object returned by
shared_ptr. So, even if rotate() happens in parallel, the memory won't
be freed until all readers are done.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement mydb_dedicated_server

4 participants