#242 fix loader creation bug that unlinks the wrong fractal tree files

src/loader.cc

@@ -251,6 +251,7 @@ toku_loader_create_loader(DB_ENV *env,
                           bool check_empty) {
     int rval;
     HANDLE_READ_ONLY_TXN(txn);
+    DB_TXN *loader_txn = nullptr;
 
     *blp = NULL;           // set later when created
 
@@ -305,6 +306,13 @@ toku_loader_create_loader(DB_ENV *env,
     }
 
     {
+        if (env->i->open_flags & DB_INIT_TXN) {
+            rval = env->txn_begin(env, txn, &loader_txn, 0);
+            if (rval) {
+                goto create_exit;
+            }
+        }
+
         ft_compare_func compare_functions[N];
         for (int i=0; i<N; i++) {
             compare_functions[i] = env->i->bt_compare;
@@ -320,13 +328,13 @@ toku_loader_create_loader(DB_ENV *env,
             fts[i] = dbs[i]->i->ft_handle;
         }
         LSN load_lsn;
-        rval = locked_load_inames(env, txn, N, dbs, new_inames_in_env, &load_lsn, puts_allowed);
+        rval = locked_load_inames(env, loader_txn, N, dbs, new_inames_in_env, &load_lsn, puts_allowed);
         if ( rval!=0 ) {
             free_inames(new_inames_in_env, N);
             toku_free(fts);
             goto create_exit;
         }
-        TOKUTXN ttxn = txn ? db_txn_struct_i(txn)->tokutxn : NULL;
+        TOKUTXN ttxn = loader_txn ? db_txn_struct_i(loader_txn)->tokutxn : NULL;
         rval = toku_ft_loader_open(&loader->i->ft_loader,
                                  env->i->cachetable,
                                  env->i->generate_row_for_put,
@@ -359,10 +367,19 @@ toku_loader_create_loader(DB_ENV *env,
             rval = 0;
         }
 
+        rval = loader_txn->commit(loader_txn, 0);
+        assert_zero(rval);
+        loader_txn = nullptr;
+
         rval = 0;
     }
     *blp = loader;
  create_exit:
+    if (loader_txn) {
+        int r  = loader_txn->abort(loader_txn);
+        assert_zero(r);
+        loader_txn = nullptr;
+    }
     if (rval == 0) {
         (void) toku_sync_fetch_and_add(&STATUS_VALUE(LOADER_CREATE), 1);
         (void) toku_sync_fetch_and_add(&STATUS_VALUE(LOADER_CURRENT), 1);

src/tests/loader-create-commit-nproc-limit.cc

+/* -*- mode: C++; c-basic-offset: 4; indent-tabs-mode: nil -*- */
+// vim: ft=cpp:expandtab:ts=8:sw=4:softtabstop=4:
+/*
+COPYING CONDITIONS NOTICE:
+
+  This program is free software; you can redistribute it and/or modify
+  it under the terms of version 2 of the GNU General Public License as
+  published by the Free Software Foundation, and provided that the
+  following conditions are met:
+
+      * Redistributions of source code must retain this COPYING
+        CONDITIONS NOTICE, the COPYRIGHT NOTICE (below), the
+        DISCLAIMER (below), the UNIVERSITY PATENT NOTICE (below), the
+        PATENT MARKING NOTICE (below), and the PATENT RIGHTS
+        GRANT (below).
+
+      * Redistributions in binary form must reproduce this COPYING
+        CONDITIONS NOTICE, the COPYRIGHT NOTICE (below), the
+        DISCLAIMER (below), the UNIVERSITY PATENT NOTICE (below), the
+        PATENT MARKING NOTICE (below), and the PATENT RIGHTS
+        GRANT (below) in the documentation and/or other materials
+        provided with the distribution.
+
+  You should have received a copy of the GNU General Public License
+  along with this program; if not, write to the Free Software
+  Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+  02110-1301, USA.
+
+COPYRIGHT NOTICE:
+
+  TokuDB, Tokutek Fractal Tree Indexing Library.
+  Copyright (C) 2007-2013 Tokutek, Inc.
+
+DISCLAIMER:
+
+  This program is distributed in the hope that it will be useful, but
+  WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+  General Public License for more details.
+
+UNIVERSITY PATENT NOTICE:
+
+  The technology is licensed by the Massachusetts Institute of
+  Technology, Rutgers State University of New Jersey, and the Research
+  Foundation of State University of New York at Stony Brook under
+  United States of America Serial No. 11/760379 and to the patents
+  and/or patent applications resulting from it.
+
+PATENT MARKING NOTICE:
+
+  This software is covered by US Patent No. 8,185,551.
+  This software is covered by US Patent No. 8,489,638.
+
+PATENT RIGHTS GRANT:
+
+  "THIS IMPLEMENTATION" means the copyrightable works distributed by
+  Tokutek as part of the Fractal Tree project.
+
+  "PATENT CLAIMS" means the claims of patents that are owned or
+  licensable by Tokutek, both currently or in the future; and that in
+  the absence of this license would be infringed by THIS
+  IMPLEMENTATION or by using or running THIS IMPLEMENTATION.
+
+  "PATENT CHALLENGE" shall mean a challenge to the validity,
+  patentability, enforceability and/or non-infringement of any of the
+  PATENT CLAIMS or otherwise opposing any of the PATENT CLAIMS.
+
+  Tokutek hereby grants to you, for the term and geographical scope of
+  the PATENT CLAIMS, a non-exclusive, no-charge, royalty-free,
+  irrevocable (except as stated in this section) patent license to
+  make, have made, use, offer to sell, sell, import, transfer, and
+  otherwise run, modify, and propagate the contents of THIS
+  IMPLEMENTATION, where such license applies only to the PATENT
+  CLAIMS.  This grant does not include claims that would be infringed
+  only as a consequence of further modifications of THIS
+  IMPLEMENTATION.  If you or your agent or licensee institute or order
+  or agree to the institution of patent litigation against any entity
+  (including a cross-claim or counterclaim in a lawsuit) alleging that
+  THIS IMPLEMENTATION constitutes direct or contributory patent
+  infringement, or inducement of patent infringement, then any rights
+  granted to you under this License shall terminate as of the date
+  such litigation is filed.  If you or your agent or exclusive
+  licensee institute or order or agree to the institution of a PATENT
+  CHALLENGE, then Tokutek may terminate any rights granted to you
+  under this License.
+*/
+
+#ident "Copyright (c) 2010-2013 Tokutek Inc.  All rights reserved."
+#ident "$Id$"
+
+// This test crashes if a failed loader creation causes the db to be corrupted by unlinking 
+// the underlying fractal tree files.  This unlinking occurs because the txn that logs the
+// load log entries is committed rather than aborted.
+
+#include "test.h"
+#include <db.h>
+#include <sys/resource.h>
+
+static int loader_flags = 0;
+static const char *envdir = TOKU_TEST_FILENAME;
+
+static void loader_create_commit(int ndb) {
+    int r;
+
+    char rmcmd[32 + strlen(envdir)];
+    snprintf(rmcmd, sizeof rmcmd, "rm -rf %s", envdir);
+    r = system(rmcmd);                                                                             CKERR(r);
+    r = toku_os_mkdir(envdir, S_IRWXU+S_IRWXG+S_IRWXO);                                                       CKERR(r);
+
+    DB_ENV *env;
+    r = db_env_create(&env, 0);                                                                               CKERR(r);
+    int envflags = DB_INIT_LOCK | DB_INIT_LOG | DB_INIT_MPOOL | DB_INIT_TXN | DB_CREATE | DB_PRIVATE;
+    r = env->open(env, envdir, envflags, S_IRWXU+S_IRWXG+S_IRWXO);                                            CKERR(r);
+    env->set_errfile(env, stderr);
+
+    DB *dbs[ndb];
+    uint32_t db_flags[ndb];
+    uint32_t dbt_flags[ndb];
+    for (int i = 0; i < ndb; i++) {
+        db_flags[i] = DB_NOOVERWRITE;
+        dbt_flags[i] = 0;
+        r = db_create(&dbs[i], env, 0); CKERR(r);
+        char name[32];
+        sprintf(name, "db%d", i);
+        r = dbs[i]->open(dbs[i], NULL, name, NULL, DB_BTREE, DB_CREATE, 0666); CKERR(r);
+    }
+
+    DB_TXN *txn;
+    r = env->txn_begin(env, NULL, &txn, 0); CKERR(r);
+
+    struct rlimit current_nproc_limit;
+    r = getrlimit(RLIMIT_NPROC, &current_nproc_limit);
+    assert(r == 0);
+    
+    struct rlimit new_nproc_limit = current_nproc_limit;
+    new_nproc_limit.rlim_cur = 0;
+    r = setrlimit(RLIMIT_NPROC, &new_nproc_limit);
+    assert(r == 0);
+
+    DB_LOADER *loader;
+    int loader_r = env->create_loader(env, txn, &loader, ndb > 0 ? dbs[0] : NULL, ndb, dbs, db_flags, dbt_flags, loader_flags);
+
+    r = setrlimit(RLIMIT_NPROC, &current_nproc_limit);
+    assert(r == 0);
+
+    if (loader_flags & LOADER_DISALLOW_PUTS)  {
+        CKERR(loader_r);
+        loader_r = loader->close(loader);
+        CKERR(loader_r);
+    } else {
+        CKERR2(loader_r, EAGAIN);
+    }
+
+    r = txn->commit(txn, 0); CKERR(r);
+
+    for (int i = 0; i < ndb; i++) {
+        r = dbs[i]->close(dbs[i], 0); CKERR(r);
+    }
+
+    for (int i = 0; i < ndb; i++) {
+        r = db_create(&dbs[i], env, 0); CKERR(r);
+        char name[32];
+        sprintf(name, "db%d", i);
+        r = dbs[i]->open(dbs[i], NULL, name, NULL, DB_BTREE, 0, 0666); CKERR(r);
+    }
+
+    for (int i = 0; i < ndb; i++) {
+        r = dbs[i]->close(dbs[i], 0); CKERR(r);
+    }
+
+    r = env->close(env, 0); CKERR(r);
+}
+
+static void do_args(int argc, char * const argv[]) {
+    int resultcode;
+    char *cmd = argv[0];
+    argc--; argv++;
+    while (argc>0) {
+        if (strcmp(argv[0], "-h")==0) {
+	    resultcode=0;
+	do_usage:
+	    fprintf(stderr, "Usage: %s -h -v -q -p\n", cmd);
+	    exit(resultcode);
+	} else if (strcmp(argv[0], "-v")==0) {
+	    verbose++;
+	} else if (strcmp(argv[0],"-q")==0) {
+	    verbose--;
+	    if (verbose<0) verbose=0;
+        } else if (strcmp(argv[0], "-p") == 0) {
+            loader_flags |= LOADER_DISALLOW_PUTS;
+        } else if (strcmp(argv[0], "-z") == 0) {
+            loader_flags |= LOADER_COMPRESS_INTERMEDIATES;
+        } else if (strcmp(argv[0], "-e") == 0) {
+            argc--; argv++;
+            if (argc > 0)
+                envdir = argv[0];
+	} else {
+	    fprintf(stderr, "Unknown arg: %s\n", argv[0]);
+	    resultcode=1;
+	    goto do_usage;
+	}
+	argc--;
+	argv++;
+    }
+}
+
+int test_main(int argc, char * const *argv) {
+    do_args(argc, argv);
+    loader_create_commit(1);
+    return 0;
+}

src/ydb_db.cc

@@ -1221,36 +1221,14 @@ load_inames(DB_ENV * env, DB_TXN * txn, int N, DB * dbs[/*N*/], const char * new
 
 int
 locked_load_inames(DB_ENV * env, DB_TXN * txn, int N, DB * dbs[/*N*/], char * new_inames_in_env[/*N*/], LSN *load_lsn, bool mark_as_loader) {
-    int ret, r;
+    int r;
     HANDLE_READ_ONLY_TXN(txn);
 
-    DB_TXN *child_txn = NULL;
-    int using_txns = env->i->open_flags & DB_INIT_TXN;
-    if (using_txns) {
-        ret = toku_txn_begin(env, txn, &child_txn, 0);
-        invariant_zero(ret);
-    }
-
     // cannot begin a checkpoint
     toku_multi_operation_client_lock();
-    r = load_inames(env, child_txn, N, dbs, (const char **) new_inames_in_env, load_lsn, mark_as_loader);
+    r = load_inames(env, txn, N, dbs, (const char **) new_inames_in_env, load_lsn, mark_as_loader);
     toku_multi_operation_client_unlock();
 
-    if (using_txns) {
-        if (r == 0) {
-            ret = locked_txn_commit(child_txn, DB_TXN_NOSYNC);
-            invariant_zero(ret);
-        } else {
-            ret = locked_txn_abort(child_txn);
-            invariant_zero(ret);
-            for (int i = 0; i < N; i++) {
-                if (new_inames_in_env[i]) {
-                    toku_free(new_inames_in_env[i]);
-                    new_inames_in_env[i] = NULL;
-                }
-            }
-        }
-    }
     return r;
 
 }