Addresses #378

Refactored error handling to work with C++ API.
Modify Makefile to avoid repeating all tests.

git-svn-id: file:///svn/tokudb@2309 c7de825b-a66e-492c-adef-691d508d4ae1

src/errors.c

@@ -62,11 +62,11 @@ static void __toku_ydb_error_file(const DB_ENV *env, BOOL use_stderr,
                   if the prefix is empty)
      \param ap    Optional prefix
 */
-static void __toku_ydb_error_all_cases(const DB_ENV * env, 
-                                       int error, 
-                                       BOOL include_stderrstring, 
-                                       BOOL use_stderr_if_nothing_else, 
-                                       const char *fmt, va_list ap) {
+void toku_ydb_error_all_cases(const DB_ENV * env, 
+                              int error, 
+                              BOOL include_stderrstring, 
+                              BOOL use_stderr_if_nothing_else, 
+                              const char *fmt, va_list ap) {
     /* Construct the error message */
     char buf [4000];
     int count=0;
@@ -96,7 +96,7 @@ int toku_ydb_do_error (const DB_ENV *dbenv, int error, const char *fmt, ...) {
     if (toku_logger_panicked(dbenv->i->logger)) dbenv->i->is_panicked=1;
     va_list ap;
     va_start(ap, fmt);
-    __toku_ydb_error_all_cases(dbenv, error, TRUE, FALSE, fmt, ap);
+    toku_ydb_error_all_cases(dbenv, error, TRUE, FALSE, fmt, ap);
     va_end(ap);
     return error;
 }
@@ -114,7 +114,7 @@ void toku_locked_env_err(const DB_ENV * env, int error, const char *fmt, ...) {
     toku_ydb_lock();
     va_list ap;
     va_start(ap, fmt);
-    __toku_ydb_error_all_cases(env, error, TRUE, TRUE, fmt, ap);
+    toku_ydb_error_all_cases(env, error, TRUE, TRUE, fmt, ap);
     va_end(ap);
     toku_ydb_unlock();
 }

src/lock_tree/locktree.c

@@ -13,19 +13,6 @@
 #include <ydb-internal.h>
 #include <brt-internal.h>
 
-/* TODO: Yoni should check that all asserts make sense instead of panic,
-         and all early returns make sense instead of panic,
-         and vice versa. */
-/* TODO: During integration, create a db panic function to take care of this.
-         The panic function will go in ydb.c.
-         We may have to return the panic return code something.
-         We know the DB will always return EINVAL afterwards, but
-         what is the INITIAL panic return?
-         ALSO maybe make ticket, maybe it should be doing DB_RUNRECOVERY after
-         instead of EINVAL.
-*/
-/* TODO: During integration, make sure we first verify the NULL CONSISTENCY,
-         (return EINVAL if necessary) before making lock tree calls. */
 inline static int __toku_lt_panic(toku_lock_tree *tree, int r) {
     return tree->panic(tree->db, r);
 }

src/tests/Makefile

@@ -17,7 +17,7 @@ ifneq ($(OSX),)
  LIBEXT=dylib
  VGRIND=
  BDB_SUPPRESSIONS=
- TDB_LOADLIBES = -L../ -ltokudb
+ TDB_LOADLIBES = -L../ -ltokudb 
  SETTOKUENV=export DYLD_LIBRARY_PATH=.. ;
  UNSETTOKUENV=unset DYLD_LIBRARY_PATH ;
 else
@@ -77,7 +77,7 @@ check.tdb: make_libs $(RUN_TDB_TESTS)
 	@ echo ok $@
 
 # Need these rule so that Make knows about all the file names
-.PHONY: %.bdbrun %.tdbrun %.run
+.PHONY: %.run
 $(RUN_ALL_TESTS):
 $(ALL_TESTS):
 
@@ -106,10 +106,11 @@ endif
 #%.bdbrun: %.bdb
 #	$(MAYBEATSIGN) $(UNSETTOKUENV) $(VGRIND) $(BDB_SUPPRESSIONS) ./$< $(VERBVERBOSE)
 # Now thiss:
-%.bdbrun: %.bdb
+%.bdbrun: %.bdb ../libtokudb.$(LIBEXT)
 	$(MAYBEATSIGN) $(UNSETTOKUENV) ./$< $(VERBVERBOSE)
-%.tdbrun: %.tdb
+%.tdbrun: %.tdb ../libtokudb.$(LIBEXT)
 	$(MAYBEATSIGN) $(SETTOKUENV) $(VGRIND) ./$< $(VERBVERBOSE)
+	@ touch $@
 
 # For BDB, VGRIND isn't happy
 NO_VGRIND = \
@@ -186,10 +187,15 @@ all.recover: test_log2.recover test_log3.recover test_log4.recover test_log5.rec
 make_libs:
 	cd ..;make
 
+.PHONY: clean cleanall
+
 clean:
-	rm -f $(ALL_TESTS) *.o *.gcno *.gcda *.gcov
+	rm -f $(ALL_TESTS) *.o *.gcno *.gcda *.gcov 
 	rm -rf dir.*.tdb dir.*.bdb
 
+cleanall: clean
+	rm -f *.tdbrun
+
 test_db_curs4.tdb: trace.h
 test_db_curs4.bdb: trace.h
 test_db_assoc3.tdb test_db_assoc3.bdb: test.h
@@ -204,10 +210,12 @@ test_db_assoc3.tdbrun_wasbad: test_db_assoc3.tdb
 	$(MAYBEATSIGN) $(SETTOKUENV) ./test_db_assoc3.tdb --seed=1 --count=200 --more
 	$(MAYBEATSIGN) $(SETTOKUENV) ./test_db_assoc3.tdb --seed=1 --count=200 --more
 	$(MAYBEATSIGN) $(SETTOKUENV) ./test_db_assoc3.tdb --seed=1 --count=200 --more
+	@ touch $@
 
 test_db_assoc3.tdbrun: test_db_assoc3.tdb
 	$(MAYBEATSIGN) $(SETTOKUENV) $(VGRIND) ./test_db_assoc3.tdb --seed=2 --count=100000 $(VERBVERBOSE)
 	$(MAYBEATSIGN) $(SETTOKUENV) $(VGRIND) ./test_db_assoc3.tdb --seed=2 --count=100000 --more $(VERBVERBOSE)
+	@ touch $@
 
 # Give up on VGRIND for bdbrun
 test_db_assoc3.bdbrun: test_db_assoc3.bdb

src/ydb-internal.h

@@ -109,6 +109,11 @@ void toku_ydb_unlock();
 #define HANDLE_PANICKED_DB(db) HANDLE_PANICKED_ENV(db->dbenv)
 
 /* */
+void toku_ydb_error_all_cases(const DB_ENV * env, 
+                              int error, 
+                              BOOL include_stderrstring, 
+                              BOOL use_stderr_if_nothing_else, 
+                              const char *fmt, va_list ap);
 int toku_ydb_do_error (const DB_ENV *, int, const char *, ...);
 
 /* Location specific debug print-outs */

src/ydb.c

@@ -910,24 +910,40 @@ static int toku_db_close(DB * db, u_int32_t flags) {
     return r;
 }
 
+/* Verify that an element from the secondary database is still consistent
+   with the primary.
+   \param secondary Secondary database
+   \param pkey Primary key
+   \param data Primary data
+   \param skey Secondary key to test
+   
+   \return 
+*/
 static int verify_secondary_key(DB *secondary, DBT *pkey, DBT *data, DBT *skey) {
     int r = 0;
     DBT idx;
 
     assert(secondary->i->primary != 0);
     memset(&idx, 0, sizeof(idx));
-    secondary->i->associate_callback(secondary, pkey, data, &idx);
-    if (r==DB_DONOTINDEX) return DB_SECONDARY_BAD;
+    r = secondary->i->associate_callback(secondary, pkey, data, &idx);
+    if (r==DB_DONOTINDEX) { r = DB_SECONDARY_BAD; goto clean_up; }
+    if (r!=0) goto clean_up;
 #ifdef DB_DBT_MULTIPLE
     if (idx.flags & DB_DBT_MULTIPLE) {
-        return EINVAL; // We aren't ready for this
+        r = EINVAL; // We aren't ready for this
+        goto clean_up;
     }
 #endif
-	if (skey->size != idx.size || memcmp(skey->data, idx.data, idx.size) != 0) r = DB_SECONDARY_BAD;
-    if (idx.flags & DB_DBT_APPMALLOC) {
-    	toku_free(idx.data);
+    if (secondary->i->brt->compare_fun(secondary, skey, &idx) != 0) {
+        r = DB_SECONDARY_BAD;
+        goto clean_up;
     }
-    return r;
+    clean_up:
+    if (idx.flags & DB_DBT_APPMALLOC) {
+        /* This should be free because idx.data is allocated by the user */
+    	free(idx.data);
+    }
+    return r; 
 }
 
 //Get the main portion of a cursor flag (excluding the bitwise or'd components).
@@ -1299,7 +1315,8 @@ delete_silently_and_retry:
     if (r == DB_NOTFOUND)   goto delete_silently_and_retry;
     if (r != 0) goto died3;
     r = verify_secondary_key(db, pkey, data, key);
-    if (r != 0)             goto delete_silently_and_retry;
+    if (r == DB_SECONDARY_BAD) goto delete_silently_and_retry;
+    if (r != 0) goto died3;
 
     //Copy everything and return.
     assert(r==0);
@@ -1435,12 +1452,14 @@ static int do_associated_deletes(DB_TXN *txn, DBT *key, DBT *data, DB *secondary
     u_int32_t brtflags;
     DBT idx;
     memset(&idx, 0, sizeof(idx));
-    int r = secondary->i->associate_callback(secondary, key, data, &idx);
     int r2 = 0;
-    if (r==DB_DONOTINDEX) return 0;
+    int r = secondary->i->associate_callback(secondary, key, data, &idx);
+    if (r==DB_DONOTINDEX) { r = 0; goto clean_up; }
+    if (r!=0) goto clean_up;
 #ifdef DB_DBT_MULTIPLE
     if (idx.flags & DB_DBT_MULTIPLE) {
-        return EINVAL; // We aren't ready for this
+        r = EINVAL; // We aren't ready for this
+        goto clean_up;
     }
 #endif
     toku_brt_get_flags(secondary->i->brt, &brtflags);
@@ -1448,16 +1467,18 @@ static int do_associated_deletes(DB_TXN *txn, DBT *key, DBT *data, DB *secondary
         //If the secondary has duplicates we need to use cursor deletes.
         DBC *dbc;
         r = toku_db_cursor(secondary, txn, &dbc, 0);
-        if (r!=0) goto cleanup;
+        if (r!=0) goto cursor_cleanup;
         r = toku_c_get_noassociate(dbc, &idx, key, DB_GET_BOTH);
-        if (r!=0) goto cleanup;
+        if (r!=0) goto cursor_cleanup;
         r = toku_c_del_noassociate(dbc, 0);
-    cleanup:
+    cursor_cleanup:
         r2 = toku_c_close(dbc);
     } else 
         r = toku_db_del_noassociate(secondary, txn, &idx, DB_DELETE_ANY);
+    clean_up:
     if (idx.flags & DB_DBT_APPMALLOC) {
-    	toku_free(idx.data);
+        /* This should be free because idx.data is allocated by the user */
+    	free(idx.data);
     }
     if (r!=0) return r;
     return r2;
@@ -2037,15 +2058,18 @@ static int do_associated_inserts (DB_TXN *txn, DBT *key, DBT *data, DB *secondar
     DBT idx;
     memset(&idx, 0, sizeof(idx));
     int r = secondary->i->associate_callback(secondary, key, data, &idx);
-    if (r==DB_DONOTINDEX) return 0;
+    if (r==DB_DONOTINDEX) { r = 0; goto clean_up; }
+    if (r != 0) goto clean_up;
 #ifdef DB_DBT_MULTIPLE
     if (idx.flags & DB_DBT_MULTIPLE) {
 	return EINVAL; // We aren't ready for this
     }
 #endif
     r = toku_db_put_noassociate(secondary, txn, &idx, key, DB_YESOVERWRITE);
+    clean_up:
     if (idx.flags & DB_DBT_APPMALLOC) {
-        toku_free(idx.data);
+        /* This should be free because idx.data is allocated by the user */
+        free(idx.data);
     }
     return r;
 }