close[t:5100] close[t:4930] the api for closing a ft has been simplified for...

close[t:5100] close[t:4930] the api for closing a ft has been simplified for normal use. for recovery, there is a new function that internally asserts the underlying ft is close and passes a valid lsn to the cachefile for closing


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

ft/ft-ops.c

@@ -3417,21 +3417,36 @@ ft_remove_handle_ref_callback(FT UU(ft), void *extra) {
     toku_list_remove(&handle->live_ft_handle_link);
 }
 
-int
-toku_ft_handle_close (FT_HANDLE brt, bool oplsn_valid, LSN oplsn)
-// Effect: See ft-ops.h for the specification of this function.
-{
-    // There are error paths in the ft_handle_open that end with brt->ft==NULL.
-    FT ft = brt->ft;
+// close an ft handle during normal operation. the underlying ft may or may not close,
+// depending if there are still references. an lsn for this close will come from the logger.
+void
+toku_ft_handle_close(FT_HANDLE ft_handle) {
+    // There are error paths in the ft_handle_open that end with ft_handle->ft==NULL.
+    FT ft = ft_handle->ft;
     if (ft) {
-        toku_ft_remove_reference(brt->ft, oplsn_valid, oplsn, ft_remove_handle_ref_callback, brt);
+        const bool oplsn_valid = false;
+        toku_ft_remove_reference(ft, oplsn_valid, ZERO_LSN, ft_remove_handle_ref_callback, ft_handle);
     }
-    toku_free(brt);
-    return 0;
+    toku_free(ft_handle);
+}
+
+// close an ft handle during recovery. the underlying ft must close, and will use the given lsn.
+void 
+toku_ft_handle_close_recovery(FT_HANDLE ft_handle, LSN oplsn) {
+    FT ft = ft_handle->ft;
+    // the ft must exist if closing during recovery. error paths during 
+    // open for recovery should close handles using toku_ft_handle_close()
+    assert(ft);
+    const bool oplsn_valid = true;
+    toku_ft_remove_reference(ft, oplsn_valid, oplsn, ft_remove_handle_ref_callback, ft_handle);
+    toku_free(ft_handle);
 }
 
-int toku_close_ft_handle_nolsn (FT_HANDLE brt, char** UU(error_string)) {
-    return toku_ft_handle_close(brt, FALSE, ZERO_LSN);
+// TODO: remove this, callers should instead just use toku_ft_handle_close()
+int 
+toku_close_ft_handle_nolsn (FT_HANDLE ft_handle, char** UU(error_string)) {
+    toku_ft_handle_close(ft_handle);
+    return 0;
 }
 
 int toku_ft_handle_create(FT_HANDLE *ft_handle_ptr) {

ft/ft-ops.h

@@ -93,6 +93,13 @@ int toku_ft_handle_open(FT_HANDLE, const char *fname_in_env,
 		  int is_create, int only_create, CACHETABLE ct, TOKUTXN txn)  __attribute__ ((warn_unused_result));
 int toku_ft_handle_open_recovery(FT_HANDLE, const char *fname_in_env, int is_create, int only_create, CACHETABLE ct, TOKUTXN txn, 
 			   FILENUM use_filenum, LSN max_acceptable_lsn)  __attribute__ ((warn_unused_result));
+
+// close an ft handle during normal operation. the underlying ft may or may not close,
+// depending if there are still references. an lsn for this close will come from the logger.
+void toku_ft_handle_close(FT_HANDLE ft_handle);
+// close an ft handle during recovery. the underlying ft must close, and will use the given lsn.
+void toku_ft_handle_close_recovery(FT_HANDLE ft_handle, LSN oplsn);
+
 int
 toku_ft_handle_open_with_dict_id(
     FT_HANDLE t, 
@@ -149,8 +156,6 @@ int toku_ft_send_insert(FT_HANDLE brt, DBT *key, DBT *val, XIDS xids, enum ft_ms
 int toku_ft_send_delete(FT_HANDLE brt, DBT *key, XIDS xids) __attribute__ ((warn_unused_result));
 int toku_ft_send_commit_any(FT_HANDLE brt, DBT *key, XIDS xids) __attribute__ ((warn_unused_result));
 
-int toku_ft_handle_close (FT_HANDLE brt, bool oplsn_valid, LSN oplsn)  __attribute__ ((warn_unused_result));
-
 int toku_close_ft_handle_nolsn (FT_HANDLE, char **error_string)  __attribute__ ((warn_unused_result));
 
 int toku_ft_handle_set_panic(FT_HANDLE brt, int panic, char *panic_string)  __attribute__ ((warn_unused_result));

ft/ft.c

@@ -728,8 +728,7 @@ dictionary_redirect_internal(const char *dst_fname_in_env, FT src_h, TOKUTXN txn
     assert(toku_ft_needed_unlocked(src_h));
     toku_ft_release_reflock(src_h);
 
-    r = toku_ft_handle_close(tmp_dst_ft, FALSE, ZERO_LSN);
-    assert_zero(r);
+    toku_ft_handle_close(tmp_dst_ft);
 
     *dst_hp = dst_h;
     return r;
@@ -997,6 +996,12 @@ toku_ft_remove_reference(FT ft, bool oplsn_valid, LSN oplsn, remove_ft_ref_callb
         remove_ref(ft, extra);
         bool needed = toku_ft_needed_unlocked(ft);
         toku_ft_release_reflock(ft);
+
+        // if we're running during recovery, we must close the underlying ft.
+        // we know we're running in recovery if we were passed a valid lsn.
+        if (oplsn_valid) {
+            assert(!needed);
+        }
         if (!needed) {
             // close header
             char *error_string = NULL;

ft/logger.c

@@ -226,7 +226,7 @@ toku_logger_close_rollback(TOKULOGGER logger, BOOL recovery_failed) {
             assert(!ft->h->dirty); // it should not have been dirtied by the toku_ft_is_empty test.
         }
 
-        r = toku_ft_handle_close(ft_to_close, FALSE, ZERO_LSN);
+        toku_ft_handle_close(ft_to_close);
         //Set as dealt with already.
         logger->rollback_cachefile = NULL;
     }

ft/recover.c

@@ -140,8 +140,7 @@ static void file_map_close_dictionaries(struct file_map *fmap, BOOL recovery_suc
         }
         // Logging is on again, but we must pass the right LSN into close.
         if (tuple->ft_handle) { // it's a DB, not a rollback file
-            r = toku_ft_handle_close(tuple->ft_handle, true, oplsn);
-            lazy_assert_zero(r);
+            toku_ft_handle_close_recovery(tuple->ft_handle, oplsn);
         } else {
             assert(tuple->ft_handle==NULL);
         }
@@ -308,9 +307,8 @@ static int internal_recover_fopen_or_fcreate (RECOVER_ENV renv, BOOL must_create
     r = toku_ft_handle_open_recovery(brt, iname, must_create, must_create, renv->ct, txn, filenum, max_acceptable_lsn);
     if (r != 0) {
         //Note:  If ft_handle_open fails, then close_ft will NOT write a header to disk.
-        //No need to provide lsn
-        int r2 = toku_ft_handle_close(brt, FALSE, ZERO_LSN);
-        assert_zero(r2);
+        //No need to provide lsn, so use the regular toku_ft_handle_close function
+        toku_ft_handle_close(brt);
         toku_free(iname);
         if (r == ENOENT) //Not an error to simply be missing.
             r = 0;
@@ -848,8 +846,7 @@ static int toku_recover_fclose (struct logtype_fclose *l, RECOVER_ENV renv) {
 
         if (0!=strcmp(iname, ROLLBACK_CACHEFILE_NAME)) {
             //Rollback cachefile is closed manually at end of recovery, not here
-            r = toku_ft_handle_close(tuple->ft_handle, true, l->lsn);
-            lazy_assert_zero(r);
+            toku_ft_handle_close_recovery(tuple->ft_handle, l->lsn);
         }
         file_map_remove(&renv->fmap, l->filenum);
         toku_free(iname);

src/ydb_db.c

@@ -123,20 +123,18 @@ toku_db_close(DB * db) {
         // internal (non-user) dictionary has no dname
         env_note_db_closed(db->dbenv, db);  // tell env that this db is no longer in use by the user of this api (user-closed, may still be in use by fractal tree internals)
     }
-    r = toku_ft_handle_close(db->i->ft_handle, FALSE, ZERO_LSN);
-    if (r == 0) {
-        // go ahead and close this DB handle right away.
-        if (db->i->lt) {
-            toku_lt_remove_db_ref(db->i->lt);
-        }
-        toku_sdbt_cleanup(&db->i->skey);
-        toku_sdbt_cleanup(&db->i->sval);
-        if (db->i->dname) {
-            toku_free(db->i->dname);
-        }
-        toku_free(db->i);
-        toku_free(db);
+    // close the ft handle, and possibly close the locktree
+    toku_ft_handle_close(db->i->ft_handle);
+    if (db->i->lt) {
+        toku_lt_remove_db_ref(db->i->lt);
+    }
+    toku_sdbt_cleanup(&db->i->skey);
+    toku_sdbt_cleanup(&db->i->sval);
+    if (db->i->dname) {
+        toku_free(db->i->dname);
     }
+    toku_free(db->i);
+    toku_free(db);
     return r;
 }