Subject: [PATCH] sysbench: fix memory leak in oltp prepare step From: Christian Ehrhardt The prepare step of oltp uses db_query which implicitly allocates a result set, but doesn't care about it. Due to that the result set is never freed which causes the oltp test prepare step consume almost as much process memory as the database size of the created DB will be. Eventually the symptom where crashes due to the oom-killer as soon as sysbench was requested to create databases bigger than ram+swap. The leak was found using valgrind: Before Fix: 9,992,400 bytes in 49,962 blocks are definitely lost in loss record 40 of 40 at 0x40079E6: malloc (vg_replace_malloc.c:236) by 0x41CCD34699: PQmakeEmptyPGresult (in /usr/lib64/libpq.so.5.2) by 0x41CCD3E093: ??? (in /usr/lib64/libpq.so.5.2) by 0x41CCD35273: PQgetResult (in /usr/lib64/libpq.so.5.2) by 0x41CCD35543: ??? (in /usr/lib64/libpq.so.5.2) by 0x8000EB33: pgsql_drv_query (drv_pgsql.c:557) by 0x8000661D: db_query (db_driver.c:404) by 0x8000B29D: oltp_cmd_prepare (sb_oltp.c:464) by 0x4ED756A893: (below main) (in /lib64/libc-2.12.so) LEAK SUMMARY: definitely lost: 9,993,885 bytes in 49,965 blocks indirectly lost: 0 bytes in 0 blocks possibly lost: 13,176 bytes in 281 blocks still reachable: 1,548 bytes in 15 blocks suppressed: 0 bytes in 0 blocks After Fix: LEAK SUMMARY: definitely lost: 0 bytes in 0 blocks indirectly lost: 0 bytes in 0 blocks possibly lost: 6,976 bytes in 250 blocks still reachable: 148 bytes in 8 blocks suppressed: 0 bytes in 0 blocks To fix that this patch introduces a wrapper called db_query_nors (nors = no record set). That wrapper calls db_query for the work, but frees up the retrned record set. Due to that all the checks based on the returned pointer could be simplified as well as they no more need the "== NULL" portion. Signed-off-by: Christian Ehrhardt --- [diffstat] db_driver.c | 17 +++++++++++++++++ db_driver.h | 2 ++ tests/oltp/sb_oltp.c | 27 +++++++++++++-------------- 3 files changed, 32 insertions(+), 14 deletions(-) [diff] diff -Naur sysbench-0.4.12/sysbench/db_driver.c sysbench-0.4.12-nors/sysbench/db_driver.c --- sysbench-0.4.12/sysbench/db_driver.c 2009-03-15 13:42:04.000000000 +0100 +++ sysbench-0.4.12-nors/sysbench/db_driver.c 2011-05-18 10:06:23.000000000 +0200 @@ -409,6 +409,23 @@ } +/* like db_query, but no one will use the result set - returns 0 on error */ + + +int db_query_nors(db_conn_t *con, const char *query) +{ + int rc = 0; + db_result_set_t *rs = db_query(con, query); + + if (rs == NULL) + rc = 1; + else + db_free_results(rs); + + return rc; +} + + /* Free result set */ diff -Naur sysbench-0.4.12/sysbench/db_driver.h sysbench-0.4.12-nors/sysbench/db_driver.h --- sysbench-0.4.12/sysbench/db_driver.h 2009-03-15 13:42:04.000000000 +0100 +++ sysbench-0.4.12-nors/sysbench/db_driver.h 2011-05-18 10:06:24.000000000 +0200 @@ -251,6 +251,8 @@ db_result_set_t *db_query(db_conn_t *, const char *); +int db_query_nors(db_conn_t *, const char *); + int db_free_results(db_result_set_t *); int db_store_results(db_result_set_t *); diff -Naur sysbench-0.4.12/sysbench/tests/oltp/sb_oltp.c sysbench-0.4.12-nors/sysbench/tests/oltp/sb_oltp.c --- sysbench-0.4.12/sysbench/tests/oltp/sb_oltp.c 2009-03-15 13:42:04.000000000 +0100 +++ sysbench-0.4.12-nors/sysbench/tests/oltp/sb_oltp.c 2011-05-18 10:07:04.000000000 +0200 @@ -383,7 +383,7 @@ driver_caps.unsigned_int ? "UNSIGNED" : "", (table_options_str != NULL) ? table_options_str : "" ); - if (db_query(con, query) == NULL) + if (db_query_nors(con, query)) { log_text(LOG_FATAL, "failed to create test table"); goto error; @@ -391,15 +391,14 @@ if (args.auto_inc && !driver_caps.serial && !driver_caps.auto_increment) { - if (db_query(con, "CREATE SEQUENCE sbtest_seq") == NULL || - db_query(con, + if (db_query_nors(con, "CREATE SEQUENCE sbtest_seq") || + db_query_nors(con, "CREATE TRIGGER sbtest_trig BEFORE INSERT ON sbtest " "FOR EACH ROW " "BEGIN SELECT sbtest_seq.nextval INTO :new.id FROM DUAL; " - "END;") - == NULL) + "END;")) { - log_text(LOG_FATAL, "failed to create test table"); + log_text(LOG_FATAL, "failed to create test table SEQUENCE/TRIGGER"); goto error; } } @@ -408,7 +407,7 @@ snprintf(query, query_len, "CREATE INDEX k on %s(k)", args.table_name); - if (db_query(con, query) == NULL) + if (db_query_nors(con, query)) { log_text(LOG_FATAL, "failed to create secondary index on table!"); goto error; @@ -461,9 +460,9 @@ } /* Execute query */ - if (db_query(con, query) == NULL) + if (db_query_nors(con, query)) { - log_text(LOG_FATAL, "failed to create test table!"); + log_text(LOG_FATAL, "failed test table INSERT"); goto error; } @@ -472,7 +471,7 @@ commit_cntr += nrows; if (commit_cntr >= ROWS_BEFORE_COMMIT) { - if (db_query(con, "COMMIT") == NULL) + if (db_query_nors(con, "COMMIT")) { log_text(LOG_FATAL, "failed to commit inserted rows!"); goto error; @@ -482,9 +481,9 @@ } } - if (driver_caps.needs_commit && db_query(con, "COMMIT") == NULL) + if (driver_caps.needs_commit && db_query_nors(con, "COMMIT")) { - if (db_query(con, "COMMIT") == NULL) + if (db_query_nors(con, "COMMIT")) { log_text(LOG_FATAL, "failed to commit inserted rows!"); return 1; @@ -526,7 +525,7 @@ /* Drop the test table */ log_text(LOG_NOTICE, "Dropping table '%s'...", args.table_name); snprintf(query, sizeof(query), "DROP TABLE %s", args.table_name); - if (db_query(con, query) == NULL) + if (db_query_nors(con, query)) { oltp_disconnect(con); return 1; @@ -561,7 +560,7 @@ if (con == NULL) return 1; snprintf(query, sizeof(query), "TRUNCATE TABLE %s", args.table_name); - if (db_query(con, query) == NULL) + if (db_query_nors(con, query)) return 1; oltp_disconnect(con); }