Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix memory leak of pfs keys #444

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions cmake_modules/TokuSetupCTest.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ if (BUILD_TESTING OR BUILD_FT_TESTS OR BUILD_SRC_TESTS)
## set up full valgrind suppressions file (concatenate the suppressions files)
file(READ ft/valgrind.suppressions valgrind_suppressions)
file(WRITE "${CMAKE_CURRENT_BINARY_DIR}/valgrind.suppressions" "${valgrind_suppressions}")
file(READ third_party/xz.suppressions xz_suppressions)
file(APPEND "${CMAKE_CURRENT_BINARY_DIR}/valgrind.suppressions" "${xz_suppressions}")
file(READ bash.suppressions bash_suppressions)
file(APPEND "${CMAKE_CURRENT_BINARY_DIR}/valgrind.suppressions" "${bash_suppressions}")

Expand Down Expand Up @@ -126,7 +128,7 @@ if (BUILD_TESTING OR BUILD_FT_TESTS OR BUILD_SRC_TESTS)
endmacro(add_toku_test)

## setup a function to write tests that will run with helgrind
set(CMAKE_HELGRIND_COMMAND_STRING "valgrind --quiet --tool=helgrind --error-exitcode=1 --soname-synonyms=somalloc=*tokuportability* --suppressions=${TokuDB_SOURCE_DIR}/src/tests/helgrind.suppressions --trace-children=yes --trace-children-skip=sh,*/sh,basename,*/basename,dirname,*/dirname,rm,*/rm,cp,*/cp,mv,*/mv,cat,*/cat,diff,*/diff,grep,*/grep,date,*/date,test,*/tokudb_dump* --trace-children-skip-by-arg=--only_create,--test,--no-shutdown,novalgrind")
set(CMAKE_HELGRIND_COMMAND_STRING "valgrind --quiet --tool=helgrind --error-exitcode=1 --soname-synonyms=somalloc=*tokuportability* --suppressions=${TokuDB_SOURCE_DIR}/src/tests/helgrind.suppressions --trace-children=yes --trace-children-skip=sh,*/sh,basename,*/basename,dirname,*/dirname,rm,*/rm,cp,*/cp,mv,*/mv,cat,*/cat,diff,*/diff,grep,*/grep,date,*/date,test,*/tokudb_dump* --trace-children-skip-by-arg=--only_create,--test,--no-shutdown,novalgrind --fair-sched=try")
function(add_helgrind_test pfx name)
separate_arguments(CMAKE_HELGRIND_COMMAND_STRING)
add_test(
Expand All @@ -137,7 +139,7 @@ if (BUILD_TESTING OR BUILD_FT_TESTS OR BUILD_SRC_TESTS)
endfunction(add_helgrind_test)

## setup a function to write tests that will run with drd
set(CMAKE_DRD_COMMAND_STRING "valgrind --quiet --tool=drd --error-exitcode=1 --soname-synonyms=somalloc=*tokuportability* --suppressions=${TokuDB_SOURCE_DIR}/src/tests/drd.suppressions --trace-children=yes --trace-children-skip=sh,*/sh,basename,*/basename,dirname,*/dirname,rm,*/rm,cp,*/cp,mv,*/mv,cat,*/cat,diff,*/diff,grep,*/grep,date,*/date,test,*/tokudb_dump* --trace-children-skip-by-arg=--only_create,--test,--no-shutdown,novalgrind")
set(CMAKE_DRD_COMMAND_STRING "valgrind --quiet --tool=drd --error-exitcode=1 --soname-synonyms=somalloc=*tokuportability* --suppressions=${TokuDB_SOURCE_DIR}/src/tests/drd.suppressions --trace-children=yes --trace-children-skip=sh,*/sh,basename,*/basename,dirname,*/dirname,rm,*/rm,cp,*/cp,mv,*/mv,cat,*/cat,diff,*/diff,grep,*/grep,date,*/date,test,*/tokudb_dump* --trace-children-skip-by-arg=--only_create,--test,--no-shutdown,novalgrind --fair-sched=try")
function(add_drd_test pfx name)
separate_arguments(CMAKE_DRD_COMMAND_STRING)
add_test(
Expand Down
18 changes: 0 additions & 18 deletions ft/tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -123,22 +123,4 @@ if(BUILD_TESTING OR BUILD_FT_TESTS)
get_filename_component(test_basename "${test}" NAME)
add_ft_test_aux(test-${test_basename} test-upgrade-recovery-logs ${test})
endforeach(test)

## give some tests, that time out normally, 1 hour to complete
set(long_tests
ft/ftloader-test-extractor-3a
ft/log-test7
ft/recovery-bad-last-entry
ft/subblock-test-compression
ft/upgrade_test_simple
)
set_tests_properties(${long_tests} PROPERTIES TIMEOUT 3600)
## some take even longer, with valgrind
set(extra_long_tests
ft/benchmark-test
ft/benchmark-test_256
ft/is_empty
ft/subblock-test-checksum
)
set_tests_properties(${extra_long_tests} PROPERTIES TIMEOUT 7200)
endif(BUILD_TESTING OR BUILD_FT_TESTS)
16 changes: 8 additions & 8 deletions locktree/lock_request.cc
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,6 @@ void lock_request::destroy(void) {
toku_cond_destroy(&m_wait_cond);
}

void lock_request::clearmem(char c) {
memset(this, c, sizeof(* this));
}

// set the lock request parameters. this API allows a lock request to be reused.
void lock_request::set(locktree *lt, TXNID txnid, const DBT *left_key, const DBT *right_key, lock_request::type lock_type, bool big_txn, void *extra) {
invariant(m_state != state::PENDING);
Expand Down Expand Up @@ -321,16 +317,20 @@ int lock_request::retry(void) {
m_txnid, m_left_key, m_right_key, &conflicts, m_big_txn);
}

// if the acquisition succeeded then remove ourselves from the
// set of lock requests, complete, and signal the waiting thread.
if (r == 0) {
// if the acquisition succeeded or if out of locks
// then remove ourselves from the set of lock requests, complete
// the lock request, and signal the waiting threads.
if (r == 0 || r == TOKUDB_OUT_OF_LOCKS) {
remove_from_lock_requests();
complete(r);
if (m_retry_test_callback)
m_retry_test_callback(); // test callback
toku_cond_broadcast(&m_wait_cond);
} else {
} else if (r == DB_LOCK_NOTGRANTED) {
// get the conflicting txnid and remain pending
m_conflicting_txnid = conflicts.get(0);
} else {
invariant(0);
}
conflicts.destroy();

Expand Down
1 change: 0 additions & 1 deletion locktree/lock_request.h
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ class lock_request {

// effect: Destroys a lock request.
void destroy(void);
void clearmem(char c);

// effect: Resets the lock request parameters, allowing it to be reused.
// requires: Lock request was already created at some point
Expand Down
4 changes: 2 additions & 2 deletions locktree/tests/lock_request_create_set.cc
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ Copyright (c) 2006, 2015, Percona and/or its affiliates. All rights reserved.
namespace toku {

// create and set the object's internals, destroy should not crash.
void lock_request_unit_test::test_create_destroy(void) {
void lock_request_unit_test::run(void) {
lock_request request;
request.create();

Expand All @@ -66,7 +66,7 @@ void lock_request_unit_test::test_create_destroy(void) {

int main(void) {
toku::lock_request_unit_test test;
test.test_create_destroy();
test.run();
return 0;
}

4 changes: 2 additions & 2 deletions locktree/tests/lock_request_get_set_keys.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ namespace toku {
// make setting keys and getting them back works properly.
// at a high level, we want to make sure keys are copied
// when appropriate and plays nice with +/- infinity.
void lock_request_unit_test::test_get_set_keys(void) {
void lock_request_unit_test::run(void) {
lock_request request;
request.create();

Expand Down Expand Up @@ -82,7 +82,7 @@ void lock_request_unit_test::test_get_set_keys(void) {

int main(void) {
toku::lock_request_unit_test test;
test.test_get_set_keys();
test.run();
return 0;
}

4 changes: 2 additions & 2 deletions locktree/tests/lock_request_killed.cc
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ static int my_killed_callback(void) {
}

// make sure deadlocks are detected when a lock request starts
void lock_request_unit_test::test_wait_time_callback(void) {
void lock_request_unit_test::run(void) {
int r;
locktree lt;

Expand Down Expand Up @@ -118,7 +118,7 @@ void lock_request_unit_test::test_wait_time_callback(void) {

int main(void) {
toku::lock_request_unit_test test;
test.test_wait_time_callback();
test.run();
return 0;
}

4 changes: 2 additions & 2 deletions locktree/tests/lock_request_not_killed.cc
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ static int my_killed_callback(void) {
}

// make sure deadlocks are detected when a lock request starts
void lock_request_unit_test::test_wait_time_callback(void) {
void lock_request_unit_test::run(void) {
int r;
locktree lt;

Expand Down Expand Up @@ -112,7 +112,7 @@ void lock_request_unit_test::test_wait_time_callback(void) {

int main(void) {
toku::lock_request_unit_test test;
test.test_wait_time_callback();
test.run();
return 0;
}

117 changes: 117 additions & 0 deletions locktree/tests/lock_request_retry_out_of_locks.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
/* -*- mode: C++; c-basic-offset: 4; indent-tabs-mode: nil -*- */
// vim: ft=cpp:expandtab:ts=8:sw=4:softtabstop=4:

// Verify that lock request retry returns TOKUDB_OUT_OF_LOCKS when
// all of the locktree memory is used.

#include "lock_request_unit_test.h"

namespace toku {

static void locktree_release_lock(locktree *lt,
TXNID txn_id,
const DBT *left,
const DBT *right) {
range_buffer buffer;
buffer.create();
buffer.append(left, right);
lt->release_locks(txn_id, &buffer);
buffer.destroy();
}

void lock_request_unit_test::run(void) {
int r;

locktree_manager mgr;
mgr.create(nullptr, nullptr, nullptr, nullptr);

DICTIONARY_ID dict_id = {1};
locktree *lt = mgr.get_lt(dict_id, dbt_comparator, nullptr);

// set max lock memory small so that we can test the limit
// with just 2 locks
mgr.set_max_lock_memory(300);

// create a small key
DBT small_dbt;
int64_t small_key = 1;
toku_fill_dbt(&small_dbt, &small_key, sizeof small_key);
small_dbt.flags = DB_DBT_USERMEM;
const DBT *small_ptr = &small_dbt;

// create a large key
DBT large_dbt;
union { int64_t n; char c[64]; } large_key;
memset(&large_key, 0, sizeof large_key);
large_key.n = 2;
toku_fill_dbt(&large_dbt, &large_key, sizeof large_key);
large_dbt.flags = DB_DBT_USERMEM;
const DBT *large_dbt_ptr = &large_dbt;

TXNID txn_a = { 1 };
TXNID txn_b = { 2 };

// a locks small key
lock_request a;
a.create();
a.set(lt, txn_a, small_ptr, small_ptr, lock_request::type::WRITE, false);
r = a.start();
assert(r == 0);
assert(a.m_state == lock_request::state::COMPLETE);

// b tries to lock small key, fails since it is already locked
lock_request b;
b.create();
b.set(lt, txn_b, small_ptr, small_ptr, lock_request::type::WRITE, false);
r = b.start();
assert(r == DB_LOCK_NOTGRANTED);
assert(b.m_state == lock_request::state::PENDING);

// a locks large key. lock memory is over the limit
lock_request c;
c.create();
c.set(lt, txn_a, large_dbt_ptr, large_dbt_ptr, lock_request::type::WRITE, false);
r = c.start();
assert(r == 0);
assert(c.m_state == lock_request::state::COMPLETE);

// a releases small key, lock memory is still over the limit
locktree_release_lock(lt, txn_a, small_ptr, small_ptr);

// retry all lock requests, should complete lock request
// b with a TOKUDB_OUT_OF_LOCKS result
lock_request::retry_all_lock_requests(lt);

assert(b.m_state == lock_request::state::COMPLETE);
assert(b.m_complete_r == TOKUDB_OUT_OF_LOCKS);

// b waits for small key, gets out of locks
r = b.wait(0);
assert(r == TOKUDB_OUT_OF_LOCKS);
assert(b.m_state == lock_request::state::COMPLETE);
assert(b.m_complete_r == TOKUDB_OUT_OF_LOCKS);

// a releases large key
locktree_release_lock(lt, txn_a, large_dbt_ptr, large_dbt_ptr);

// b locks small key, gets its
r = b.start();
assert(r == 0);

// b releases lock so we can exit cleanly
locktree_release_lock(lt, txn_b, small_ptr, small_ptr);

a.destroy();
b.destroy();

mgr.release_lt(lt);
mgr.destroy();
}

} /* namespace toku */

int main(void) {
toku::lock_request_unit_test test;
test.run();
return 0;
}
4 changes: 2 additions & 2 deletions locktree/tests/lock_request_start_deadlock.cc
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ Copyright (c) 2006, 2015, Percona and/or its affiliates. All rights reserved.
namespace toku {

// make sure deadlocks are detected when a lock request starts
void lock_request_unit_test::test_start_deadlock(void) {
void lock_request_unit_test::run(void) {
int r;
locktree lt;

Expand Down Expand Up @@ -114,7 +114,7 @@ void lock_request_unit_test::test_start_deadlock(void) {

int main(void) {
toku::lock_request_unit_test test;
test.test_start_deadlock();
test.run();
return 0;
}

4 changes: 2 additions & 2 deletions locktree/tests/lock_request_start_pending.cc
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ namespace toku {

// starting a lock request without immediate success should get
// stored in the lock request set as pending.
void lock_request_unit_test::test_start_pending(void) {
void lock_request_unit_test::run(void) {
int r;
locktree lt;
lock_request request;
Expand Down Expand Up @@ -100,7 +100,7 @@ void lock_request_unit_test::test_start_pending(void) {

int main(void) {
toku::lock_request_unit_test test;
test.test_start_pending();
test.run();
return 0;
}

1 change: 0 additions & 1 deletion locktree/tests/lock_request_start_retry_race.cc
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ namespace toku {
}

request.destroy();
request.clearmem(0xab);

toku_pthread_yield();
if ((i % 10) == 0)
Expand Down
1 change: 0 additions & 1 deletion locktree/tests/lock_request_start_retry_race_3.cc
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,6 @@ namespace toku {
}

request.destroy();
request.clearmem(0xab);

toku_pthread_yield();
if ((i % 10) == 0)
Expand Down
1 change: 0 additions & 1 deletion locktree/tests/lock_request_start_retry_wait_race_2.cc
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,6 @@ namespace toku {
}

request.destroy();
request.clearmem(0xab);

toku_pthread_yield();
if ((i % 10) == 0)
Expand Down
18 changes: 1 addition & 17 deletions locktree/tests/lock_request_unit_test.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,23 +47,7 @@ namespace toku {

class lock_request_unit_test {
public:
// create and set the object's internals, destroy should not crash.
void test_create_destroy(void);

// make setting keys and getting them back works properly.
// at a high level, we want to make sure keys are copied
// when appropriate and plays nice with +/- infinity.
void test_get_set_keys(void);

// starting a lock request without immediate success should get
// stored in the lock request set as pending.
void test_start_pending(void);

// make sure deadlocks are detected when a lock request starts
void test_start_deadlock(void);

// test that the get_wait_time callback works
void test_wait_time_callback(void);
void run(void);

private:
// releases a single range lock and retries all lock requests.
Expand Down
Loading