Data race on writing_rollback variable.

cmake_modules/TokuSetupCTest.cmake

@@ -94,7 +94,11 @@ 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 bash.suppressions bash_suppressions)
+  file(READ third_party/xz.suppressions xz_suppressions)
+  file(APPEND "${CMAKE_CURRENT_BINARY_DIR}/valgrind.suppressions" "${xz_suppressions}")
+  file(READ third_party/cmake.suppressions cmake_suppressions)
+  file(APPEND "${CMAKE_CURRENT_BINARY_DIR}/valgrind.suppressions" "${cmake_suppressions}")
+  file(READ third_party/bash.suppressions bash_suppressions)
   file(APPEND "${CMAKE_CURRENT_BINARY_DIR}/valgrind.suppressions" "${bash_suppressions}")
 
   include(CMakeDependentOption)
@@ -126,7 +130,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(
@@ -137,7 +141,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(

ft/ft-internal.h

@@ -80,7 +80,7 @@ extern "C" {
 extern uint force_recovery;
 }
 
-extern int writing_rollback;
+extern std::atomic_int writing_rollback;
 
 // The ft_header is not managed by the cachetable.  Instead, it hangs off the cachefile as userdata.
 struct ft_header {

ft/logger/logger.cc

@@ -49,7 +49,7 @@ Copyright (c) 2006, 2015, Percona and/or its affiliates. All rights reserved.
 
 #include "util/status.h"
 
-int writing_rollback = 0;
+std::atomic_int writing_rollback = {0};
 extern "C" {
   uint force_recovery = 0;
 }

ft/node.h

@@ -155,7 +155,7 @@ class ftnode_pivot_keys {
     size_t _total_size;
 };
 
-extern int writing_rollback;
+extern std::atomic_int writing_rollback;
 
 extern "C" {
 extern uint force_recovery;

ft/tests/CMakeLists.txt

@@ -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)

ft/txn/rollback.cc

@@ -43,7 +43,7 @@ Copyright (c) 2006, 2015, Percona and/or its affiliates. All rights reserved.
 #include "ft/logger/log-internal.h"
 #include "ft/txn/rollback-ct-callbacks.h"
 
-extern int writing_rollback;
+extern std::atomic_int writing_rollback;
 
 static void rollback_unpin_remove_callback(CACHEKEY* cachekey, bool for_checkpoint, void* extra) {
     FT CAST_FROM_VOIDP(ft, extra);

locktree/lock_request.cc

@@ -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);
@@ -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();
 

locktree/lock_request.h

@@ -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

locktree/tests/lock_request_create_set.cc

@@ -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();
 
@@ -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;
 }
 

locktree/tests/lock_request_get_set_keys.cc

@@ -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();
 
@@ -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;
 }
 

locktree/tests/lock_request_killed.cc

@@ -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;
 
@@ -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;
 }
 

locktree/tests/lock_request_not_killed.cc

@@ -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;
 
@@ -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;
 }
 

locktree/tests/lock_request_retry_out_of_locks.cc

@@ -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;
+}

locktree/tests/lock_request_start_deadlock.cc

@@ -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;
 
@@ -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;
 }
 

locktree/tests/lock_request_start_pending.cc

@@ -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;
@@ -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;
 }
 

locktree/tests/lock_request_start_retry_race.cc

@@ -83,7 +83,6 @@ namespace toku {
             }
 
             request.destroy();
-            request.clearmem(0xab);
 
             toku_pthread_yield();
             if ((i % 10) == 0)

locktree/tests/lock_request_start_retry_race_3.cc

@@ -96,7 +96,6 @@ namespace toku {
             }
 
             request.destroy();
-            request.clearmem(0xab);
 
             toku_pthread_yield();
             if ((i % 10) == 0)

locktree/tests/lock_request_start_retry_wait_race_2.cc

@@ -98,7 +98,6 @@ namespace toku {
             }
 
             request.destroy();
-            request.clearmem(0xab);
 
             toku_pthread_yield();
             if ((i % 10) == 0)