From d6b5f0b432ecc485c21e2bb088645175cd936441 Mon Sep 17 00:00:00 2001 From: John Esmet Date: Wed, 17 Apr 2013 00:00:51 -0400 Subject: [PATCH] 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 | 37 ++++++++++++++++++++++++++----------- ft/ft-ops.h | 9 +++++++-- ft/ft.c | 9 +++++++-- ft/logger.c | 2 +- ft/recover.c | 11 ++++------- src/ydb_db.c | 24 +++++++++++------------- 6 files changed, 56 insertions(+), 36 deletions(-) diff --git a/ft/ft-ops.c b/ft/ft-ops.c index 315fbd2edbd..67acd1b4b40 100644 --- a/ft/ft-ops.c +++ b/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); } -int toku_close_ft_handle_nolsn (FT_HANDLE brt, char** UU(error_string)) { - return toku_ft_handle_close(brt, FALSE, ZERO_LSN); +// 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); +} + +// 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) { diff --git a/ft/ft-ops.h b/ft/ft-ops.h index 5184927093f..4606e183a42 100644 --- a/ft/ft-ops.h +++ b/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)); diff --git a/ft/ft.c b/ft/ft.c index 1044d8c679d..9b84b211939 100644 --- a/ft/ft.c +++ b/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; diff --git a/ft/logger.c b/ft/logger.c index 443855ba62b..f7cc1912b27 100644 --- a/ft/logger.c +++ b/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; } diff --git a/ft/recover.c b/ft/recover.c index fd30d160d06..fb5ed0bfcfb 100644 --- a/ft/recover.c +++ b/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); diff --git a/src/ydb_db.c b/src/ydb_db.c index 0fdbbddd028..6041d389fd1 100644 --- a/src/ydb_db.c +++ b/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; }