When spider tries to find a partition matching a name passed from the
sql layer, it construct the partition name with NORMAL_PART_NAME.
However, the name passed from the sql layer could be constructed with
other types of name, such as TEMP_PART_NAME, which is a longer string.
Spider does handle TEMP_PART_NAME in other places of
spider_get_partition_info, but overall it is not able to handle
partition changes involving redistributing data to partitions which
can result in TEMP_PART_NAME. That is a more involved issue. In this
patch, we simply follow the existing intended logic and fix the MSAN
complaint.
We spare spider_table_remove_share_from_crd/sts_thread and
spider_table_add_share_to_crd/sts_thread because the function body has
too many fields with crd/sts in the name and macro affects
debuggability.
Each spider connection is identified with a connection key, which is
an encoding of the backend parameters.
The first byte of the key is by default 0, and in rare circumstances
it is changed to a different value: when semi_table_lock is set to 1;
and when using casual read. When this happens, often a new connection
is created with the new key. Neither case is useful: the description
of semi_table_lock has nothing to do with creation of new connections
and the parameter itself was deprecated for 10.7+ (MDEV-28829) and
marked for deletion (MDEV-28830); while new threads created by
non-zero spider_casual_read causes only threads to be idle, thus not
achieving any gain, see MDEV-26151, and the param has also been
deprecated in 11.5+ (MDEV-31789). The relevant code adds unnecessary
complexity to the spider code. This change does not reduce
parallelism, because already when bgs mode is on a background thread
is created per partition, and there is no evidence spider creates
multiple threads for one partition. If the needs of such cases arise
it will be a separate issue.
The conn_kind, which stands for "connection kind", is no longer useful
because the HandlerSocket support is deleted and Spider now has only
one connection kind, SPIDER_CONN_KIND_MYSQL. Remove conn_kind and
related code.
Signed-off-by: Yuchen Pei <yuchen.pei@mariadb.com>
Reviewed-by: Nayuta Yanagisawa <nayuta.yanagisawa@mariadb.com>
Partial commit of the greater MDEV-34348 scope.
MDEV-34348: MariaDB is violating clang-16 -Wcast-function-type-strict
Change the type of my_hash_get_key to:
1) Return const
2) Change the context parameter to be const void*
Also fix casting in hash adjacent areas.
Reviewed By:
============
Marko Mäkelä <marko.makela@mariadb.com>
Partial commit of the greater MDEV-34348 scope.
MDEV-34348: MariaDB is violating clang-16 -Wcast-function-type-strict
The functions queue_compare, qsort2_cmp, and qsort_cmp2
all had similar interfaces, and were used interchangable
and unsafely cast to one another.
This patch consolidates the functions all into the
qsort_cmp2 interface.
Reviewed By:
============
Marko Mäkelä <marko.makela@mariadb.com>
Remove the dead-code, in Spider, which is related to the Spider's
Oracle OCI support. The code has been disabled for a long time and
it is unlikely that the code will be enabled.
- document tmp_share, which are temporary spider shares with only one
link (no ha)
- simplify spider_get_sys_tables_connect_info() where link_idx is
always 0
Distinguish them in two place:
when constructing connection key in create_conn_key and
spider_create_conn for both ordinary queries and spider_direct_sql
For spider_create_conn and spider_udf_direct_sql_create_conn, the
created conn gets assigned a field from the source object if and only
if source->field is non-null.
For spider_create_conn_keys and spider_udf_direct_sql_create_conn_key,
we update the encoding so that absence of keyword and keyword with an
empty value result in different keys. More specifically, if the i-th
keyword has a value, the corresponding part in the conn key begins
with the char \i, followed by the possibly empty value and ends with a
\0. If the i-th keyword is not specified, then it does not get a
mention in the conn key.
As part of this change, we also update table param / option parsing to
recognise a singleton empty string when creating an string list,
instead of writing it off as NULL.
Remove the dead-code, in Spider, which is related to the Spider's
HandlerSocket support. The code has been disabled for a long time
and it is unlikely that the code will be enabled.
- rm all files under storage/spider/hs_client/ except hs_compat.h
- rm storage/spider/spd_db_handlersocket.*
- unifdef -UHS_HAS_SQLCOM -UHAVE_HANDLERSOCKET \
-m storage/spider/spd_* storage/spider/ha_spider.* storage/spider/hs_client/*
- remove relevant files from storage/spider/CMakeLists.txt
The two I_S plugins SPIDER_ALLOC_MEM and SPIDER_WRAPPER_PROTOCOL
only makes sense if the main SPIDER plugin is installed. Further,
SPIDER_ALLOC_MEM requires a mutex that requires SPIDER init to fill
the table.
We also update the spider init query to override
--transaction_read_only=on so that it does not affect the spider init.
Also fixed error handling in spider_db_init() so that failure in
spider table init does not result in memory leak
Same as MDEV-29579. For some reason, libodbc does not clean up
properly if unloaded too early with the dlclose() of spider. So we add
UNIQUE symbols to spider so the spider does not reload in dlclose().
This change, however, uncovers some hidden problems in the spider
codebase, for which we move the initialisation of some spider global
variables into the initialisation of spider itself.
Spider has some global variables. Their initialisation should be done
in the initialisation of spider itself, otherwise, if spider were
re-initialised without these symbol being unloaded, the values could
be inconsistent and causing issues.
One such issue is caused by the variables
spider_mon_table_cache_version and spider_mon_table_cache_version_req.
They are used for resetting the spider monitoring table cache and have
initial values of 0 and 1 respectively. We have that always
spider_mon_table_cache_version_req >= spider_mon_table_cache_version,
and when the relation is strict, the cache is reset,
spider_mon_table_cache_version is brought to be equal to
spider_mon_table_cache_version_req, and the cache is searched for
matching table_name, db_name and link_idx. If the relation is equal,
no reset would happen and the cache would be searched directly.
When spider is re-inited without resetting the values of
spider_mon_table_cache_version and spider_mon_table_cache_version_req
that were set to be equal in the previous cache reset action, the
cache was emptied in the previous spider deinit, which would result in
HA_ERR_KEY_NOT_FOUND unexpectedly.
An alternative way to fix this issue would be to call the spider udf
spider_flush_mon_cache_table(), which increments
spider_mon_table_cache_version_req thus making sure the inequality is
strict. However, there's no reason for spider to initialise these
global variables on dlopen(), rather than on spider init, which is
cleaner and "purer".
To reproduce this issue, simply revert the changes involving the two
variables and then run:
mtr --no-reorder spider.ha{,_part}
Partial documentation due to time constraints. Will improve over time.
Also removed a redundant parameter link_idx from
spider_get_sys_tables_connect_info().
And deleted some commented out code.
Since 0930eb86cb, system table creation
needed for spider init is delayed to the signal_ddl_recovery_done
callback. Since it is part of the init, failure should result in
spider deinit.
We also remove the call to spider_init_system_tables() from
spider_db_init(), as it was removed in the commit mentioned above and
accidentally restored in a merge.
use signal_ddl_recovery_done callback for that.
also make the server to call signal_ddl_recovery_done() when loading
plugins at run-time (so that plugins would't need to detect that
on their own)
