From d2576d5f71221292a1c27eb1f3c85df1e9ce0afd Mon Sep 17 00:00:00 2001
From: Yoni Fogel <yoni@tokutek.com>
Date: Tue, 16 Apr 2013 23:58:03 -0400
Subject: [PATCH] Closes #2057 closes[t:2057] Add warnings about code that
 needs to remain in certain order to prevent recovery errors, Fixed bug where
 lsn in header could be wrong (or crash during recovery).

git-svn-id: file:///svn/toku/tokudb@14856 c7de825b-a66e-492c-adef-691d508d4ae1
---
 newbrt/brt.c     | 12 +++++++-----
 newbrt/recover.c | 10 ++++++----
 2 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/newbrt/brt.c b/newbrt/brt.c
index 2b63b9f47b6..7abf5387bb7 100644
--- a/newbrt/brt.c
+++ b/newbrt/brt.c
@@ -2967,11 +2967,6 @@ brtheader_note_brt_open(BRT live, char** fnamep) {
     char *fname = *fnamep;
     assert(fname);
     *fnamep     = NULL;
-    //Use fname, or throw it away.
-    if (h->fname == NULL)
-        h->fname = fname;
-    else
-        toku_free(fname);
     while (!list_empty(&h->zombie_brts)) {
         //Remove dead brt from list
         BRT zombie = list_struct(list_pop(&h->zombie_brts), struct brt, zombie_brt_link);
@@ -2982,6 +2977,12 @@ brtheader_note_brt_open(BRT live, char** fnamep) {
     }
     if (retval==0)
         list_push(&h->live_brts, &live->live_brt_link);
+    //Use fname, or throw it away.
+    //Must not use fname on failure (or when brt is closed, a (possibly corrupt) header will be written.
+    if (retval == 0 && h->fname == NULL)
+        h->fname = fname;
+    else
+        toku_free(fname);
     toku_brtheader_unlock(h);
     return retval;
 }
@@ -3092,6 +3093,7 @@ int toku_brt_open(BRT t, const char *fname, const char *fname_in_env, int is_cre
     r = toku_maybe_upgrade_brt(t);	// possibly do some work to complete the version upgrade of brt
     if (r!=0) goto died_after_read_and_pin;
 
+    // brtheader_note_brt_open must be after all functions that can fail.
     r = brtheader_note_brt_open(t, &brt_fname);
     if (r!=0) goto died_after_read_and_pin;
     if (t->db) t->db->descriptor = &t->h->descriptor.dbt;
diff --git a/newbrt/recover.c b/newbrt/recover.c
index 2db33f419a4..a834a171194 100644
--- a/newbrt/recover.c
+++ b/newbrt/recover.c
@@ -303,6 +303,8 @@ static int internal_toku_recover_fopen_or_fcreate (RECOVER_ENV renv, int flags,
 
     r = toku_brt_open(brt, fixedfname, fixedfname, (flags & O_CREAT) != 0, FALSE, renv->ct, NULL, NULL);
     if (r != 0) {
+        //Note:  If brt_open fails, then close_brt will NOT write a header to disk.
+        //No need to provide lsn
         int rr = toku_close_brt(brt, NULL, NULL); assert(rr == 0);
         toku_free(fixedfname);
         return r;
@@ -323,10 +325,10 @@ static int toku_recover_backward_fopen (struct logtype_fopen *l, RECOVER_ENV ren
         struct file_map_tuple *tuple = NULL;
         int r = file_map_find(&renv->fmap, l->filenum, &tuple);
         if (r == 0) {
-            //Must not write header to disk.  If not dirty, it will not be written.
-            //Since we're not writing to disk, we do not need to provide an LSN.
-            assert(!tuple->brt->h->dirty);
-            r = toku_close_brt(tuple->brt, 0, 0);
+            //Must keep existing lsn.
+            //The only way this should be dirty, is if its doing a file-format upgrade.
+            //If not dirty, header will not be written.
+            r = toku_close_brt_lsn(tuple->brt, 0, 0, TRUE, tuple->brt->h->checkpoint_lsn);
             assert(r == 0);
             file_map_remove(&renv->fmap, l->filenum);
         }