Skip to content

Commit e3413d6

Browse files
author
Jimmy Yang
committed
Fix issues discovered when VALGRIND is enabled.
1 parent f29cc40 commit e3413d6

File tree

8 files changed

+148
-26
lines changed

8 files changed

+148
-26
lines changed

‎plugin/innodb_memcached/daemon_memcached/daemon/memcached_mysql.cc

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,8 @@ static MYSQL_SYSVAR_UINT(r_batch_size, mci_r_batch_size,
6060

6161
static MYSQL_SYSVAR_UINT(w_batch_size, mci_w_batch_size,
6262
PLUGIN_VAR_READONLY,
63-
"write batch commit size", 0, 0, 1,
64-
32, 1048576, 0);
63+
"write batch commit size", 0, 0, 32,
64+
1, 1048576, 0);
6565

6666
static MYSQL_SYSVAR_BOOL(enable_binlog, mci_enable_binlog,
6767
PLUGIN_VAR_READONLY,
@@ -106,7 +106,7 @@ static int daemon_memcached_plugin_init(void *p)
106106
pthread_attr_t attr;
107107
struct st_plugin_int* plugin = (struct st_plugin_int *)p;
108108

109-
con = (mysql_memcached_context*) malloc(sizeof(*con));
109+
con = (mysql_memcached_context*) my_malloc(sizeof(*con), MYF(0));
110110

111111
if (mci_engine_library) {
112112
char* lib_path = (mci_eng_lib_path)
@@ -115,7 +115,8 @@ static int daemon_memcached_plugin_init(void *p)
115115
+ strlen(mci_engine_library)
116116
+ strlen(FN_DIRSEP) + 1;
117117

118-
con->memcached_conf.m_engine_library = (char*) malloc(lib_len);
118+
con->memcached_conf.m_engine_library = (char*) my_malloc(
119+
lib_len, MYF(0));
119120

120121
strxmov(con->memcached_conf.m_engine_library, lib_path,
121122
FN_DIRSEP, mci_engine_library, NullS);

‎plugin/innodb_memcached/innodb_memcache/include/innodb_api.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,7 @@ typedef struct mci_column {
147147
bool m_is_str;
148148
bool m_enabled;
149149
bool m_is_null;
150+
bool m_allocated;
150151
} mci_column_t;
151152

152153
/** We would need to fetch 5 values from each key value rows if they

‎plugin/innodb_memcached/innodb_memcache/include/innodb_cb_api.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -320,6 +320,13 @@ void
320320
/*===========================*/
321321
ib_crsr_t ib_crsr);
322322

323+
typedef
324+
ib_err_t
325+
(*CB_CURSOR_COMMIT_TRX)(
326+
/*====================*/
327+
ib_crsr_t ib_crsr,
328+
ib_trx_t ib_trx);
329+
323330
CB_OPEN_TABLE ib_cb_open_table;
324331
CB_READ_ROW ib_cb_read_row;
325332
CB_INSERT_ROW ib_cb_insert_row;
@@ -362,5 +369,6 @@ CB_CURSOR_OPEN_INDEX_USING_NAME ib_cb_cursor_open_index_using_name;
362369
CB_CLOSE_THD ib_cb_close_thd;
363370
CB_BINLOG_ENABLED ib_cb_binlog_enabled;
364371
CB_CURSOR_SET_CLUSTER_ACCESS ib_cb_cursor_set_cluster_access;
372+
CB_CURSOR_COMMIT_TRX ib_cb_cursor_commit_trx;
365373

366374
#endif

‎plugin/innodb_memcached/innodb_memcache/src/innodb_api.c

Lines changed: 67 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,8 @@ static ib_cb_t* innodb_memcached_api[] = {
8484
(ib_cb_t*) &ib_cb_cursor_open_index_using_name,
8585
(ib_cb_t*) &ib_cb_close_thd,
8686
(ib_cb_t*) &ib_cb_binlog_enabled,
87-
(ib_cb_t*) &ib_cb_cursor_set_cluster_access
87+
(ib_cb_t*) &ib_cb_cursor_set_cluster_access,
88+
(ib_cb_t*) &ib_cb_cursor_commit_trx
8889
};
8990

9091
/*************************************************************//**
@@ -313,6 +314,42 @@ innodb_api_fill_mci(
313314
mci_item->m_len = data_len;
314315
}
315316

317+
mci_item->m_is_str = TRUE;
318+
mci_item->m_enabled = TRUE;
319+
mci_item->m_allocated = FALSE;
320+
321+
return(TRUE);
322+
}
323+
324+
/*************************************************************//**
325+
copy data from a read tuple and instantiate a "mci_item"
326+
@return TRUE if successful */
327+
static
328+
bool
329+
innodb_api_copy_mci(
330+
/*================*/
331+
ib_tpl_t read_tpl, /*!< in: Read tuple */
332+
int col_id, /*!< in: Column ID for the column to
333+
read */
334+
mci_column_t* mci_item) /*!< out: item to fill */
335+
{
336+
ib_ulint_t data_len;
337+
ib_col_meta_t col_meta;
338+
339+
data_len = ib_cb_col_get_meta(read_tpl, col_id, &col_meta);
340+
341+
if (data_len == IB_SQL_NULL) {
342+
mci_item->m_str = NULL;
343+
mci_item->m_len = 0;
344+
mci_item->m_allocated = FALSE;
345+
} else {
346+
mci_item->m_str = malloc(data_len);
347+
mci_item->m_allocated = TRUE;
348+
memcpy(mci_item->m_str, ib_cb_col_get_value(read_tpl, col_id),
349+
data_len);
350+
mci_item->m_len = data_len;
351+
}
352+
316353
mci_item->m_is_str = TRUE;
317354
mci_item->m_enabled = TRUE;
318355

@@ -329,7 +366,8 @@ innodb_api_fill_value(
329366
meta_info_t* meta_info, /*!< in: Metadata */
330367
mci_item_t* item, /*!< out: result */
331368
ib_tpl_t read_tpl, /*!< in: read tuple */
332-
int col_id) /*!< in: column Id */
369+
int col_id, /*!< in: column Id */
370+
bool alloc_mem) /*!< in: allocate memory */
333371
{
334372
ib_err_t err = DB_NOT_FOUND;
335373

@@ -340,16 +378,31 @@ innodb_api_fill_value(
340378

341379
if (col_id == col_info[META_VALUE].m_field_id) {
342380

343-
innodb_api_fill_mci(read_tpl, col_id,
344-
&item->mci_item[MCI_COL_VALUE]);
381+
if (alloc_mem) {
382+
innodb_api_copy_mci(
383+
read_tpl, col_id,
384+
&item->mci_item[MCI_COL_VALUE]);
385+
} else {
386+
innodb_api_fill_mci(
387+
read_tpl, col_id,
388+
&item->mci_item[MCI_COL_VALUE]);
389+
}
345390
err = DB_SUCCESS;
346391
}
347392
} else {
348393
int i;
349394
for (i = 0; i < meta_info->m_num_add; i++) {
350395
if (col_id == meta_info->m_add_item[i].m_field_id) {
351-
innodb_api_fill_mci(read_tpl, col_id,
352-
&item->mci_add_value[i]);
396+
if (alloc_mem) {
397+
innodb_api_copy_mci(
398+
read_tpl, col_id,
399+
&item->mci_add_value[i]);
400+
} else {
401+
innodb_api_fill_mci(
402+
read_tpl, col_id,
403+
&item->mci_add_value[i]);
404+
}
405+
353406
err = DB_SUCCESS;
354407
break;
355408
}
@@ -515,7 +568,8 @@ innodb_api_search(
515568
item->mci_item[MCI_COL_EXP].m_enabled = TRUE;
516569
} else {
517570
innodb_api_fill_value(meta_info, item,
518-
read_tpl, i);
571+
read_tpl, i,
572+
sel_only);
519573
}
520574
}
521575

@@ -1365,9 +1419,11 @@ innodb_api_cursor_reset(
13651419
}
13661420

13671421
if (conn_data->c_trx) {
1368-
err = ib_cb_trx_commit(conn_data->c_trx);
1422+
err = ib_cb_cursor_commit_trx(
1423+
conn_data->c_crsr, conn_data->c_trx);
13691424
conn_data->c_trx = NULL;
13701425
}
1426+
13711427
conn_data->c_w_count_commit = 0;
13721428
}
13731429

@@ -1380,7 +1436,9 @@ innodb_api_cursor_reset(
13801436
}
13811437

13821438
if (conn_data->c_r_trx) {
1383-
err = ib_cb_trx_commit(conn_data->c_r_trx);
1439+
err = ib_cb_cursor_commit_trx(
1440+
conn_data->c_r_crsr,
1441+
conn_data->c_r_trx);
13841442
conn_data->c_r_trx = NULL;
13851443
}
13861444
conn_data->c_r_count_commit = 0;

‎plugin/innodb_memcached/innodb_memcache/src/innodb_engine.c

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ create_instance(
114114

115115
/* Now call create_instace() for the default engine */
116116
e = create_my_default_instance(interface, get_server_api,
117-
& (innodb_eng->m_default_engine));
117+
&(innodb_eng->m_default_engine));
118118
if(e != ENGINE_SUCCESS) return e;
119119

120120
innodb_eng->initialized = true;
@@ -151,7 +151,7 @@ innodb_initialize(
151151
ENGINE_HANDLE* handle,
152152
const char* config_str)
153153
{
154-
ENGINE_ERROR_CODE return_status;
154+
ENGINE_ERROR_CODE return_status = ENGINE_SUCCESS;
155155
struct innodb_engine* innodb_eng = innodb_handle(handle);
156156
struct default_engine* def_eng = default_handle(innodb_eng);
157157
eng_config_info_t* my_eng_config;
@@ -192,14 +192,18 @@ innodb_initialize(
192192
return(ENGINE_FAILED);
193193
}
194194

195-
return_status = def_eng->engine.initialize(
196-
innodb_eng->m_default_engine, my_eng_config->option_string);
195+
if (innodb_eng->m_default_engine) {
196+
return_status = def_eng->engine.initialize(
197+
innodb_eng->m_default_engine,
198+
my_eng_config->option_string);
199+
}
197200

198201
return(return_status);
199202
}
200203

201204
extern void handler_close_thd(void*);
202205

206+
/*** Cleanup a connection***/
203207
static
204208
int
205209
innodb_conn_clean(
@@ -307,7 +311,13 @@ innodb_destroy(
307311

308312
pthread_mutex_destroy(&innodb_eng->conn_mutex);
309313

310-
def_eng->engine.destroy(innodb_eng->m_default_engine, force);
314+
if (innodb_eng->m_default_engine) {
315+
def_eng->engine.destroy(innodb_eng->m_default_engine, force);
316+
}
317+
318+
innodb_config_free(&innodb_eng->meta_info);
319+
320+
free(innodb_eng);
311321
}
312322

313323

@@ -649,6 +659,10 @@ innodb_get(
649659
} else {
650660
memcpy(hash_item_get_data(it),
651661
result.mci_item[MCI_COL_VALUE].m_str, it->nbytes);
662+
663+
if (result.mci_item[MCI_COL_VALUE].m_allocated) {
664+
free(result.mci_item[MCI_COL_VALUE].m_str);
665+
}
652666
}
653667

654668
func_exit:

‎storage/innobase/api/api0api.cc

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,8 @@ typedef struct ib_cursor_struct {
117117
ib_match_mode_t match_mode; /*!< ib_cursor_moveto match mode */
118118

119119
row_prebuilt_t* prebuilt; /*!< For reading rows */
120+
121+
bool valid_trx; /*!< Valid transaction attached */
120122
} ib_cursor_t;
121123

122124
/** InnoDB table columns used during table and index schema creation. */
@@ -128,6 +130,7 @@ typedef struct ib_col_struct {
128130
ulint len; /*!< Length of the column */
129131

130132
ib_col_attr_t ib_col_attr; /*!< Column attributes */
133+
131134
} ib_col_t;
132135

133136
/** InnoDB index columns used during index and index schema creation. */
@@ -967,7 +970,7 @@ ib_create_cursor(
967970
/*=============*/
968971
ib_crsr_t* ib_crsr, /*!< out: InnoDB cursor */
969972
dict_table_t* table, /*!< in: table instance */
970-
ib_idd_t index_id, /*!< in: index id or 0 */
973+
ib_idd_t index_id, /*!< in: index id or 0 */
971974
trx_t* trx) /*!< in: transaction */
972975
{
973976
mem_heap_t* heap;
@@ -998,6 +1001,9 @@ ib_create_cursor(
9981001
prebuilt = cursor->prebuilt;
9991002

10001003
prebuilt->trx = trx;
1004+
1005+
cursor->valid_trx = TRUE;
1006+
10011007
prebuilt->table = table;
10021008
prebuilt->select_lock_type = LOCK_NONE;
10031009
prebuilt->innodb_api = TRUE;
@@ -1038,7 +1044,7 @@ UNIV_INTERN
10381044
ib_err_t
10391045
ib_cursor_open_table_using_id(
10401046
/*==========================*/
1041-
ib_idd_t table_id, /*!< in: table id of table to open */
1047+
ib_idd_t table_id, /*!< in: table id of table to open */
10421048
ib_trx_t ib_trx, /*!< in: Current transaction handle
10431049
can be NULL */
10441050
ib_crsr_t* ib_crsr) /*!< out,own: InnoDB cursor */
@@ -1069,7 +1075,7 @@ UNIV_INTERN
10691075
ib_err_t
10701076
ib_cursor_open_index_using_id(
10711077
/*==========================*/
1072-
ib_idd_t index_id, /*!< in: index id of index to open */
1078+
ib_idd_t index_id, /*!< in: index id of index to open */
10731079
ib_trx_t ib_trx, /*!< in: Current transaction handle
10741080
can be NULL */
10751081
ib_crsr_t* ib_crsr) /*!< out: InnoDB cursor */
@@ -1285,6 +1291,8 @@ ib_cursor_new_trx(
12851291

12861292
row_update_prebuilt_trx(prebuilt, trx);
12871293

1294+
cursor->valid_trx = TRUE;
1295+
12881296
trx_assign_read_view(prebuilt->trx);
12891297

12901298
ib_qry_proc_free(&cursor->q_proc);
@@ -1294,6 +1302,26 @@ ib_cursor_new_trx(
12941302
return(err);
12951303
}
12961304

1305+
/*****************************************************************//**
1306+
Commit the transaction in a cursor
1307+
@return DB_SUCCESS or err code */
1308+
ib_err_t
1309+
ib_cursor_commit_trx(
1310+
/*=================*/
1311+
ib_crsr_t ib_crsr, /*!< in/out: InnoDB cursor */
1312+
ib_trx_t ib_trx) /*!< in: transaction */
1313+
{
1314+
ib_err_t err = DB_SUCCESS;
1315+
ib_cursor_t* cursor = (ib_cursor_t*) ib_crsr;
1316+
trx_t* trx = (trx_t*) ib_trx;
1317+
row_prebuilt_t* prebuilt = cursor->prebuilt;
1318+
1319+
ut_ad(prebuilt->trx == trx);
1320+
err = ib_trx_commit(ib_trx);
1321+
cursor->valid_trx = FALSE;
1322+
return(err);
1323+
}
1324+
12971325
/*****************************************************************//**
12981326
Close an InnoDB table and free the cursor.
12991327
@return DB_SUCCESS or err code */
@@ -1310,7 +1338,8 @@ ib_cursor_close(
13101338
ib_qry_proc_free(&cursor->q_proc);
13111339

13121340
/* The transaction could have been detached from the cursor. */
1313-
if (trx != NULL && trx->n_mysql_tables_in_use > 0) {
1341+
if (cursor->valid_trx && trx != NULL
1342+
&& trx->n_mysql_tables_in_use > 0) {
13141343
--trx->n_mysql_tables_in_use;
13151344
}
13161345

@@ -3209,7 +3238,7 @@ ib_cursor_unlock(
32093238
/*=============*/
32103239
ib_crsr_t ib_crsr) /*!< in/out: InnoDB cursor */
32113240
{
3212-
ib_err_t err;
3241+
ib_err_t err = DB_SUCCESS;
32133242
ib_cursor_t* cursor = (ib_cursor_t*) ib_crsr;
32143243
row_prebuilt_t* prebuilt = cursor->prebuilt;
32153244

@@ -3219,7 +3248,7 @@ ib_cursor_unlock(
32193248
err = DB_ERROR;
32203249
}
32213250

3222-
return(DB_SUCCESS);
3251+
return(err);
32233252
}
32243253

32253254
/*****************************************************************//**

‎storage/innobase/handler/ha_innodb.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -443,7 +443,8 @@ ib_cb_t innodb_api_cb[] = {
443443
(ib_cb_t) ib_cursor_open_index_using_name,
444444
(ib_cb_t) ib_close_thd,
445445
(ib_cb_t) ib_is_binlog_enabled,
446-
(ib_cb_t) ib_cursor_set_cluster_access
446+
(ib_cb_t) ib_cursor_set_cluster_access,
447+
(ib_cb_t) ib_cursor_commit_trx
447448
};
448449

449450
/*************************************************************//**

0 commit comments

Comments
 (0)