Description:
When renaming a table the NDB_SHARE is renamed. As part of renaming the NDB_SHARE a new "key" for the NDB_SHARE is allocated in NDB_SHARE::mem_root and stashed in NDB_SHARE::new_key. This memory has the size 2*strlen(<dbname>+<table_name>). The allocated memory is twice the size since it stores both the "key"(which is "db/tabname") as well as the the db and tabname as separate strings.
Affected variables:
struct NDB_SHARE {
MEM_ROOT mem_root;
char* new_key;
char *old_names; // for rename table
}
Code:
> share->old_names= old_key;
> // ToDo free old_names after ALTER EVENT
Furthermore, the code which allocates the "key" memory does not check return value.
How to repeat:
MCI since problem is not detected by valgrind. Replacing the usage of mem_root with my_malloc/my_free calls makes the problem clearly visible by valgrind.
There are old TODOs in the code pointing at this problem has been known since the functionality was originally implemented.
Problem was detected while working on "BUG#20479917 REMOVE MCP_BUG16021021" and suggestion is to fix this after or as part of that bug.
Suggested fix:
1) Remove NDB_SHARE::mem_root
- it's only used by this "key"
- you can't release memory in it.
- it's just an unnecessarily obfuscating the code.
2) Add functions to NDB_SHARE to allocate, store and commit free a "key".
- this encapsulates
3) Add functions to NDB_SHARE to install a prepared key
- this may be the new key or the old key when "undoing" a rename.
In short, remove usage of mem_root and encapsulate this variable size memory into it's own subclass of NDB_SHARE, giving NDB_SHARE functions to operate on the new class. Add checks for memory allocation failures. Remember that the rename is prepared also on schema distribution participants and in case the memory allocation fails on those there is no way to report the error back(yet). Perhaps a fixed size buffer can be used for those since we know that only one DDL is ongoing at a time? But at the same time there are quite a few other memory allocations which could go wrong for this case.
Alternatively use fixed width strings, but it feels to much memory is wasted that way. It's quite important to keep NDB_SHARE footprint as small as possible.
Description: When renaming a table the NDB_SHARE is renamed. As part of renaming the NDB_SHARE a new "key" for the NDB_SHARE is allocated in NDB_SHARE::mem_root and stashed in NDB_SHARE::new_key. This memory has the size 2*strlen(<dbname>+<table_name>). The allocated memory is twice the size since it stores both the "key"(which is "db/tabname") as well as the the db and tabname as separate strings. Affected variables: struct NDB_SHARE { MEM_ROOT mem_root; char* new_key; char *old_names; // for rename table } Code: > share->old_names= old_key; > // ToDo free old_names after ALTER EVENT Furthermore, the code which allocates the "key" memory does not check return value. How to repeat: MCI since problem is not detected by valgrind. Replacing the usage of mem_root with my_malloc/my_free calls makes the problem clearly visible by valgrind. There are old TODOs in the code pointing at this problem has been known since the functionality was originally implemented. Problem was detected while working on "BUG#20479917 REMOVE MCP_BUG16021021" and suggestion is to fix this after or as part of that bug. Suggested fix: 1) Remove NDB_SHARE::mem_root - it's only used by this "key" - you can't release memory in it. - it's just an unnecessarily obfuscating the code. 2) Add functions to NDB_SHARE to allocate, store and commit free a "key". - this encapsulates 3) Add functions to NDB_SHARE to install a prepared key - this may be the new key or the old key when "undoing" a rename. In short, remove usage of mem_root and encapsulate this variable size memory into it's own subclass of NDB_SHARE, giving NDB_SHARE functions to operate on the new class. Add checks for memory allocation failures. Remember that the rename is prepared also on schema distribution participants and in case the memory allocation fails on those there is no way to report the error back(yet). Perhaps a fixed size buffer can be used for those since we know that only one DDL is ongoing at a time? But at the same time there are quite a few other memory allocations which could go wrong for this case. Alternatively use fixed width strings, but it feels to much memory is wasted that way. It's quite important to keep NDB_SHARE footprint as small as possible.