Skip to content

Commit 9462ce3

Browse files
nacarvalhonacarvalho
committed
BUG#21755890: DO NOT ALLOCATE MEMORY ON GR PLUGIN P_S INTERFACE IMPLEMENTATION
On the previous interface between Group Replication plugin and Performance Tables there was memory allocation which was being passed to server side. This memory allocation should be avoided, also it can cause problems when it is released from a different entity that allocated it. To avoid that issues, now the interface uses callbacks that are invoked by plugin to send information to Performance Schema tables, or other callers. The callback implementations are the ones that are responsible to allocate new memory, if needed. We also changed the definition of performance_schema.replication_group_members table member_state column to be a arbitrary string, being now the plugin the full responsible of its values. The values continue to be: ONLINE, OFFLINE and RECOVERING.
1 parent c44f992 commit 9462ce3

9 files changed

+372
-238
lines changed

‎include/mysql/plugin_group_replication.h

+69-37
Original file line numberDiff line numberDiff line change
@@ -19,42 +19,67 @@
1919
/* API for Group Peplication plugin. (MYSQL_GROUP_REPLICATION_PLUGIN) */
2020

2121
#include <mysql/plugin.h>
22-
#define MYSQL_GROUP_REPLICATION_INTERFACE_VERSION 0x0100
22+
#define MYSQL_GROUP_REPLICATION_INTERFACE_VERSION 0x0101
2323

24-
enum enum_member_state {
25-
MEMBER_STATE_ONLINE= 1,
26-
MEMBER_STATE_OFFLINE,
27-
MEMBER_STATE_RECOVERING
28-
};
24+
/*
25+
Callbacks for get_connection_status_info function.
2926
30-
typedef struct st_group_replication_connection_status_info
31-
{
32-
char* channel_name;
33-
char* group_name;
34-
bool service_state;
35-
} GROUP_REPLICATION_CONNECTION_STATUS_INFO;
27+
context field can have NULL value, plugin will always pass it
28+
through all callbacks, independent of its value.
29+
Its value will not be used by plugin.
3630
37-
typedef struct st_group_replication_group_members_info
31+
All callbacks are mandatory.
32+
*/
33+
typedef struct st_group_replication_connection_status_callbacks
34+
{
35+
void* const context;
36+
void (*set_channel_name)(void* const context, const char& value, size_t length);
37+
void (*set_group_name)(void* const context, const char& value, size_t length);
38+
void (*set_source_uuid)(void* const context, const char& value, size_t length);
39+
void (*set_service_state)(void* const context, bool state);
40+
} GROUP_REPLICATION_CONNECTION_STATUS_CALLBACKS;
41+
42+
/*
43+
Callbacks for get_group_members_info function.
44+
45+
context field can have NULL value, plugin will always pass it
46+
through all callbacks, independent of its value.
47+
Its value will not be used by plugin.
48+
49+
All callbacks are mandatory.
50+
*/
51+
typedef struct st_group_replication_group_members_callbacks
3852
{
39-
char* channel_name;
40-
char* member_id;
41-
char* member_host;
42-
unsigned int member_port;
43-
enum enum_member_state member_state;
44-
} GROUP_REPLICATION_GROUP_MEMBERS_INFO;
45-
46-
typedef struct st_group_replication_member_stats_info
53+
void* const context;
54+
void (*set_channel_name)(void* const context, const char& value, size_t length);
55+
void (*set_member_id)(void* const context, const char& value, size_t length);
56+
void (*set_member_host)(void* const context, const char& value, size_t length);
57+
void (*set_member_port)(void* const context, unsigned int value);
58+
void (*set_member_state)(void* const context, const char& value, size_t length);
59+
} GROUP_REPLICATION_GROUP_MEMBERS_CALLBACKS;
60+
61+
/*
62+
Callbacks for get_group_member_stats_info function.
63+
64+
context field can have NULL value, plugin will always pass it
65+
through all callbacks, independent of its value.
66+
Its value will not be used by plugin.
67+
68+
All callbacks are mandatory.
69+
*/
70+
typedef struct st_group_replication_member_stats_callbacks
4771
{
48-
char* channel_name;
49-
char* view_id;
50-
char* member_id;
51-
unsigned long long int transaction_in_queue;
52-
unsigned long long int transaction_certified;
53-
unsigned long long int transaction_conflicts_detected;
54-
unsigned long long int transactions_in_validation;
55-
char* committed_transactions;
56-
char* last_conflict_free_transaction;
57-
} GROUP_REPLICATION_GROUP_MEMBER_STATS_INFO;
72+
void* const context;
73+
void (*set_channel_name)(void* const context, const char& value, size_t length);
74+
void (*set_view_id)(void* const context, const char& value, size_t length);
75+
void (*set_member_id)(void* const context, const char& value, size_t length);
76+
void (*set_transactions_committed)(void* const context, const char& value, size_t length);
77+
void (*set_last_conflict_free_transaction)(void* const context, const char& value, size_t length);
78+
void (*set_transactions_in_queue)(void* const context, unsigned long long int value);
79+
void (*set_transactions_certified)(void* const context, unsigned long long int value);
80+
void (*set_transactions_conflicts_detected)(void* const context, unsigned long long int value);
81+
void (*set_transactions_in_validation)(void* const context, unsigned long long int value);
82+
} GROUP_REPLICATION_GROUP_MEMBER_STATS_CALLBACKS;
5883

5984
struct st_mysql_group_replication
6085
{
@@ -83,32 +108,39 @@ struct st_mysql_group_replication
83108
/*
84109
This function is used to fetch information for group replication kernel stats.
85110
86-
@param info[out] The retrieved information
111+
@param callbacks The set of callbacks and its context used to set the
112+
information on caller.
87113
88114
@note The caller is responsible to free memory from the info structure and
89115
from all its fields.
90116
*/
91-
bool (*get_connection_status_info)(GROUP_REPLICATION_CONNECTION_STATUS_INFO *info);
117+
bool (*get_connection_status_info)
118+
(const GROUP_REPLICATION_CONNECTION_STATUS_CALLBACKS& callbacks);
92119

93120
/*
94121
This function is used to fetch information for group replication members.
95122
96-
@param info[out] The retrieved information
123+
@param callbacks The set of callbacks and its context used to set the
124+
information on caller.
97125
98126
@note The caller is responsible to free memory from the info structure and
99127
from all its fields.
100128
*/
101-
bool (*get_group_members_info)(unsigned int index, GROUP_REPLICATION_GROUP_MEMBERS_INFO *info);
129+
bool (*get_group_members_info)
130+
(unsigned int index,
131+
const GROUP_REPLICATION_GROUP_MEMBERS_CALLBACKS& callbacks);
102132

103133
/*
104134
This function is used to fetch information for group replication members statistics.
105135
106-
@param info[out] The retrieved information
136+
@param callbacks The set of callbacks and its context used to set the
137+
information on caller.
107138
108139
@note The caller is responsible to free memory from the info structure and
109140
from all its fields.
110141
*/
111-
bool (*get_group_member_stats_info)(GROUP_REPLICATION_GROUP_MEMBER_STATS_INFO* info);
142+
bool (*get_group_member_stats_info)
143+
(const GROUP_REPLICATION_GROUP_MEMBER_STATS_CALLBACKS& callbacks);
112144

113145
/*
114146
Get number of group replication members.

‎mysql-test/suite/perfschema/r/table_schema.result

+1-1
Original file line numberDiff line numberDiff line change
@@ -908,7 +908,7 @@ def performance_schema replication_group_members CHANNEL_NAME 1 NULL NO char 64
908908
def performance_schema replication_group_members MEMBER_ID 2 NULL NO char 36 108 NULL NULL NULL utf8 utf8_bin char(36) select,insert,update,references
909909
def performance_schema replication_group_members MEMBER_HOST 3 NULL NO char 60 180 NULL NULL NULL utf8 utf8_bin char(60) select,insert,update,references
910910
def performance_schema replication_group_members MEMBER_PORT 4 NULL YES int NULL NULL 10 0 NULL NULL NULL int(11) select,insert,update,references
911-
def performance_schema replication_group_members MEMBER_STATE 5 NULL NO enum 10 30 NULL NULL NULL utf8 utf8_general_ci enum('ONLINE','OFFLINE','RECOVERING') select,insert,update,references
911+
def performance_schema replication_group_members MEMBER_STATE 5 NULL NO char 64 192 NULL NULL NULL utf8 utf8_bin char(64) select,insert,update,references
912912
def performance_schema replication_group_member_stats CHANNEL_NAME 1 NULL NO char 64 192 NULL NULL NULL utf8 utf8_general_ci char(64) select,insert,update,references
913913
def performance_schema replication_group_member_stats VIEW_ID 2 NULL NO char 60 180 NULL NULL NULL utf8 utf8_bin char(60) select,insert,update,references
914914
def performance_schema replication_group_member_stats MEMBER_ID 3 NULL NO char 36 108 NULL NULL NULL utf8 utf8_bin char(36) select,insert,update,references

‎scripts/mysql_system_tables.sql

+1-1
Original file line numberDiff line numberDiff line change
@@ -2248,7 +2248,7 @@ SET @cmd="CREATE TABLE performance_schema.replication_group_members("
22482248
"MEMBER_ID CHAR(36) collate utf8_bin not null,"
22492249
"MEMBER_HOST CHAR(60) collate utf8_bin not null,"
22502250
"MEMBER_PORT INTEGER,"
2251-
"MEMBER_STATE ENUM('ONLINE','OFFLINE','RECOVERING') not null"
2251+
"MEMBER_STATE CHAR(64) collate utf8_bin not null"
22522252
") ENGINE=PERFORMANCE_SCHEMA;";
22532253

22542254
SET @str = IF(@have_pfs = 1, @cmd, 'SET @dummy = 0');

‎sql/rpl_group_replication.cc

+20-12
Original file line numberDiff line numberDiff line change
@@ -82,28 +82,32 @@ set_retrieved_certification_info(View_change_log_event* view_change_event)
8282

8383
bool
8484
Group_replication_handler::
85-
get_connection_status_info(GROUP_REPLICATION_CONNECTION_STATUS_INFO *info)
85+
get_connection_status_info(
86+
const GROUP_REPLICATION_CONNECTION_STATUS_CALLBACKS& callbacks)
8687
{
8788
if (plugin_handle)
88-
return plugin_handle->get_connection_status_info(info);
89+
return plugin_handle->get_connection_status_info(callbacks);
8990
return true;
9091
}
9192

9293
bool
9394
Group_replication_handler::
94-
get_group_members_info(unsigned int index, GROUP_REPLICATION_GROUP_MEMBERS_INFO *info)
95+
get_group_members_info(
96+
unsigned int index,
97+
const GROUP_REPLICATION_GROUP_MEMBERS_CALLBACKS& callbacks)
9598
{
9699
if (plugin_handle)
97-
return plugin_handle->get_group_members_info(index, info);
100+
return plugin_handle->get_group_members_info(index, callbacks);
98101
return true;
99102
}
100103

101104
bool
102105
Group_replication_handler::
103-
get_group_member_stats_info(GROUP_REPLICATION_GROUP_MEMBER_STATS_INFO *info)
106+
get_group_member_stats_info(
107+
const GROUP_REPLICATION_GROUP_MEMBER_STATS_CALLBACKS& callbacks)
104108
{
105109
if (plugin_handle)
106-
return plugin_handle->get_group_member_stats_info(info);
110+
return plugin_handle->get_group_member_stats_info(callbacks);
107111
return true;
108112
}
109113

@@ -217,24 +221,28 @@ int set_group_replication_retrieved_certification_info(View_change_log_event *vi
217221
return 1;
218222
}
219223

220-
bool get_group_replication_connection_status_info(GROUP_REPLICATION_CONNECTION_STATUS_INFO *info)
224+
bool get_group_replication_connection_status_info(
225+
const GROUP_REPLICATION_CONNECTION_STATUS_CALLBACKS& callbacks)
221226
{
222227
if (group_replication_handler)
223-
return group_replication_handler->get_connection_status_info(info);
228+
return group_replication_handler->get_connection_status_info(callbacks);
224229
return true;
225230
}
226231

227-
bool get_group_replication_group_members_info(unsigned int index, GROUP_REPLICATION_GROUP_MEMBERS_INFO *info)
232+
bool get_group_replication_group_members_info(
233+
unsigned int index,
234+
const GROUP_REPLICATION_GROUP_MEMBERS_CALLBACKS& callbacks)
228235
{
229236
if (group_replication_handler)
230-
return group_replication_handler->get_group_members_info(index, info);
237+
return group_replication_handler->get_group_members_info(index, callbacks);
231238
return true;
232239
}
233240

234-
bool get_group_replication_group_member_stats_info(GROUP_REPLICATION_GROUP_MEMBER_STATS_INFO* info)
241+
bool get_group_replication_group_member_stats_info(
242+
const GROUP_REPLICATION_GROUP_MEMBER_STATS_CALLBACKS& callbacks)
235243
{
236244
if (group_replication_handler)
237-
return group_replication_handler->get_group_member_stats_info(info);
245+
return group_replication_handler->get_group_member_stats_info(callbacks);
238246
return true;
239247
}
240248

‎sql/rpl_group_replication.h

+14-6
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,13 @@ class Group_replication_handler
3737
int stop();
3838
bool is_running();
3939
int set_retrieved_certification_info(View_change_log_event *view_change_event);
40-
bool get_connection_status_info(GROUP_REPLICATION_CONNECTION_STATUS_INFO *info);
41-
bool get_group_members_info(unsigned int index, GROUP_REPLICATION_GROUP_MEMBERS_INFO *info);
42-
bool get_group_member_stats_info(GROUP_REPLICATION_GROUP_MEMBER_STATS_INFO* info);
40+
bool get_connection_status_info(
41+
const GROUP_REPLICATION_CONNECTION_STATUS_CALLBACKS& callbacks);
42+
bool get_group_members_info(
43+
unsigned int index,
44+
const GROUP_REPLICATION_GROUP_MEMBERS_CALLBACKS& callbacks);
45+
bool get_group_member_stats_info(
46+
const GROUP_REPLICATION_GROUP_MEMBER_STATS_CALLBACKS& callbacks);
4347
unsigned int get_members_number_info();
4448

4549
private:
@@ -61,9 +65,13 @@ int group_replication_stop();
6165
bool is_group_replication_running();
6266
int set_group_replication_retrieved_certification_info(View_change_log_event *view_change_event);
6367

64-
bool get_group_replication_connection_status_info(GROUP_REPLICATION_CONNECTION_STATUS_INFO *info);
65-
bool get_group_replication_group_members_info(unsigned int index, GROUP_REPLICATION_GROUP_MEMBERS_INFO *info);
66-
bool get_group_replication_group_member_stats_info(GROUP_REPLICATION_GROUP_MEMBER_STATS_INFO* info);
68+
bool get_group_replication_connection_status_info(
69+
const GROUP_REPLICATION_CONNECTION_STATUS_CALLBACKS& callbacks);
70+
bool get_group_replication_group_members_info(
71+
unsigned int index,
72+
const GROUP_REPLICATION_GROUP_MEMBERS_CALLBACKS& callbacks);
73+
bool get_group_replication_group_member_stats_info(
74+
const GROUP_REPLICATION_GROUP_MEMBER_STATS_CALLBACKS& callbacks);
6775
unsigned int get_group_replication_members_number_info();
6876

6977

‎storage/perfschema/table_replication_connection_status.cc

+56-37
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,48 @@
3434
#include "log.h"
3535
#include "rpl_group_replication.h"
3636

37+
/*
38+
Callbacks implementation for GROUP_REPLICATION_CONNECTION_STATUS_CALLBACKS.
39+
*/
40+
static void set_channel_name(void* const context, const char& value,
41+
size_t length)
42+
{
43+
}
44+
45+
static void set_group_name(void* const context, const char& value,
46+
size_t length)
47+
{
48+
struct st_row_connect_status* row=
49+
static_cast<struct st_row_connect_status*>(context);
50+
const size_t max= UUID_LENGTH;
51+
length= std::min(length, max);
52+
53+
row->group_name_is_null= false;
54+
memcpy(row->group_name, &value, length);
55+
}
56+
57+
static void set_source_uuid(void* const context, const char& value,
58+
size_t length)
59+
{
60+
struct st_row_connect_status* row=
61+
static_cast<struct st_row_connect_status*>(context);
62+
const size_t max= UUID_LENGTH;
63+
length= std::min(length, max);
64+
65+
row->source_uuid_is_null= false;
66+
memcpy(row->source_uuid, &value, length);
67+
}
68+
69+
static void set_service_state(void* const context, bool value)
70+
{
71+
struct st_row_connect_status* row=
72+
static_cast<struct st_row_connect_status*>(context);
73+
74+
row->service_state= value ? PS_RPL_CONNECT_SERVICE_STATE_YES
75+
: PS_RPL_CONNECT_SERVICE_STATE_NO;
76+
}
77+
78+
3779
THR_LOCK table_replication_connection_status::m_table_lock;
3880

3981

@@ -219,47 +261,24 @@ void table_replication_connection_status::make_row(Master_info *mi)
219261
if (is_group_replication_plugin_loaded() &&
220262
msr_map.is_group_replication_channel_name(mi->get_channel(), true))
221263
{
222-
/* Group Replication applier channel. */
223-
GROUP_REPLICATION_CONNECTION_STATUS_INFO *group_replication_info= NULL;
224-
if (!(group_replication_info= (GROUP_REPLICATION_CONNECTION_STATUS_INFO*)
225-
my_malloc(PSI_NOT_INSTRUMENTED,
226-
sizeof(GROUP_REPLICATION_CONNECTION_STATUS_INFO),
227-
MYF(MY_WME))))
264+
/*
265+
Group Replication applier channel.
266+
Set callbacks on GROUP_REPLICATION_GROUP_MEMBER_STATS_CALLBACKS.
267+
*/
268+
const GROUP_REPLICATION_CONNECTION_STATUS_CALLBACKS callbacks=
228269
{
229-
sql_print_error("Unable to allocate memory on "
230-
"table_replication_connection_status::make_row");
231-
error= true;
232-
goto end;
233-
}
234-
235-
bool stats_not_available=
236-
get_group_replication_connection_status_info(group_replication_info);
237-
if (stats_not_available)
270+
&m_row,
271+
&set_channel_name,
272+
&set_group_name,
273+
&set_source_uuid,
274+
&set_service_state,
275+
};
276+
277+
// Query plugin and let callbacks do their job.
278+
if (get_group_replication_connection_status_info(callbacks))
238279
{
239280
DBUG_PRINT("info", ("Group Replication stats not available!"));
240281
}
241-
else
242-
{
243-
my_free((void*)group_replication_info->channel_name);
244-
245-
if (group_replication_info->group_name != NULL)
246-
{
247-
memcpy(m_row.group_name, group_replication_info->group_name, UUID_LENGTH);
248-
m_row.group_name_is_null= false;
249-
250-
memcpy(m_row.source_uuid, group_replication_info->group_name, UUID_LENGTH);
251-
m_row.source_uuid_is_null= false;
252-
253-
my_free((void*)group_replication_info->group_name);
254-
}
255-
256-
if (group_replication_info->service_state)
257-
m_row.service_state= PS_RPL_CONNECT_SERVICE_STATE_YES;
258-
else
259-
m_row.service_state= PS_RPL_CONNECT_SERVICE_STATE_NO;
260-
}
261-
262-
my_free(group_replication_info);
263282
}
264283
else
265284
{

0 commit comments

Comments
 (0)