close[t:5124] use a cleaner way of access an FT_HANDLE's descriptor pointers...

close[t:5124] use a cleaner way of access an FT_HANDLE's descriptor pointers at the ydb layer. comment the ft API, too.


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

ft/ft.c

@@ -944,6 +944,16 @@ toku_ft_update_cmp_descriptor(FT ft) {
         );
 }
 
+DESCRIPTOR
+toku_ft_get_descriptor(FT_HANDLE ft_handle) {
+    return &ft_handle->ft->descriptor;
+}
+
+DESCRIPTOR
+toku_ft_get_cmp_descriptor(FT_HANDLE ft_handle) {
+    return &ft_handle->ft->cmp_descriptor;
+}
+
 void
 toku_ft_update_stats(STAT64INFO headerstats, STAT64INFO_S delta) {
     (void) __sync_fetch_and_add(&(headerstats->numrows),  delta.numrows);

ft/ft.h

@@ -15,10 +15,9 @@
 #include "ft-ops.h"
 #include "compress.h"
 
-// unlink a ft from the filesystem, without a txn.
+// unlink a ft from the filesystem with or without a txn.
+// if with a txn, then the unlink happens on commit.
 void toku_ft_unlink(FT_HANDLE handle);
-
-// unlink a ft from the filesystem when the given txn commits.
 int toku_ft_unlink_on_commit(FT_HANDLE handle, TOKUTXN txn) __attribute__((__warn_unused_result__));
 
 //Effect: suppresses rollback logs
@@ -69,7 +68,6 @@ void toku_reset_root_xid_that_created(FT h, TXNID new_root_xid_that_created);
 // Reset the root_xid_that_created field to the given value.  
 // This redefines which xid created the dictionary.
 
-
 void toku_ft_add_txn_ref(FT h);
 void toku_ft_remove_txn_ref(FT h);
 
@@ -79,9 +77,20 @@ LSN toku_ft_checkpoint_lsn(FT h)  __attribute__ ((warn_unused_result));
 int toku_ft_set_panic(FT h, int panic, char *panic_string) __attribute__ ((warn_unused_result));
 void toku_ft_stat64 (FT h, struct ftstat64_s *s);
 
-void toku_ft_update_descriptor(FT h, DESCRIPTOR d);
+// unconditionally set the descriptor for an open FT. can't do this when 
+// any operation has already occurred on the ft. 
+// see toku_ft_change_descriptor(), which is the transactional version
+// used by the ydb layer. it better describes the client contract.
+void toku_ft_update_descriptor(FT ft, DESCRIPTOR d);
 void toku_ft_update_cmp_descriptor(FT ft);
 
+// get the descriptor for a ft. safe to read as long as clients honor the
+// strict contract put forth by toku_ft_update_descriptor/toku_ft_change_descriptor
+// essentially, there should never be a reader while there is a writer, enforced
+// by the client, not the FT.
+DESCRIPTOR toku_ft_get_descriptor(FT_HANDLE ft_handle);
+DESCRIPTOR toku_ft_get_cmp_descriptor(FT_HANDLE ft_handle);
+
 void toku_ft_update_stats(STAT64INFO headerstats, STAT64INFO_S delta);
 void toku_ft_decrease_stats(STAT64INFO headerstats, STAT64INFO_S delta);
 

src/ydb_db.c

@@ -7,6 +7,7 @@
 #include <ctype.h>
 #include <db.h>
 #include "ydb-internal.h"
+#include <ft/ft.h>
 #include <ft/ft-flusher.h>
 #include <ft/checkpoint.h>
 #include "indexer.h"
@@ -310,16 +311,23 @@ toku_db_open(DB * db, DB_TXN * txn, const char *fname, const char *dbname, DBTYP
     return r;
 }
 
+// set the descriptor and cmp_descriptor to the
+// descriptors from the given ft
+static void
+db_set_descriptors(DB *db, FT_HANDLE ft_handle) {
+    db->descriptor = toku_ft_get_descriptor(ft_handle);
+    db->cmp_descriptor = toku_ft_get_cmp_descriptor(ft_handle);
+}
+
 // callback that sets the descriptors when 
-// a dictionary is redirected at the brt layer
+// a dictionary is redirected at the ft layer
 // I wonder if client applications can safely access
 // the descriptor via db->descriptor, because
 // a redirect may be happening underneath the covers.
 // Need to investigate further.
-static void db_on_redirect_callback(FT_HANDLE brt, void* extra) {
-    DB* db = extra;
-    db->descriptor = &brt->ft->descriptor;
-    db->cmp_descriptor = &brt->ft->cmp_descriptor;
+static void db_on_redirect_callback(FT_HANDLE ft_handle, void* extra) {
+    DB *db = extra;
+    db_set_descriptors(db, ft_handle);
 }
 
 int 
@@ -351,38 +359,39 @@ db_open_iname(DB * db, DB_TXN * txn, const char *iname_in_env, u_int32_t flags,
                                             flags&=~DB_READ_COMMITTED;
                                             flags&=~DB_SERIALIZABLE;
                                             flags&=~DB_IS_HOT_INDEX;
-    if (flags & ~DB_THREAD) return EINVAL; // unknown flags
-
-    if (is_db_excl && !is_db_create) return EINVAL;
+    // unknown or conflicting flags are bad
+    if ((flags & ~DB_THREAD) || (is_db_excl && !is_db_create)) {
+        return EINVAL;
+    }
 
     /* tokudb supports no duplicates and sorted duplicates only */
     unsigned int tflags;
     r = toku_ft_get_flags(db->i->ft_handle, &tflags);
-    if (r != 0) 
+    if (r != 0)  {
         return r;
+    }
 
-    if (db_opened(db))
+    if (db_opened(db)) {
         return EINVAL;              /* It was already open. */
+    }
     
     db->i->open_flags = flags;
     db->i->open_mode = mode;
 
-    FT_HANDLE brt = db->i->ft_handle;
-    r = toku_ft_handle_open(brt, iname_in_env,
+    FT_HANDLE ft_handle = db->i->ft_handle;
+    r = toku_ft_handle_open(ft_handle, iname_in_env,
                       is_db_create, is_db_excl,
                       db->dbenv->i->cachetable,
                       txn ? db_txn_struct_i(txn)->tokutxn : NULL_TXN);
-    if (r != 0)
+    if (r != 0) {
         goto error_cleanup;
+    }
 
     db->i->opened = 1;
 
-    // now that the brt has successfully opened, a valid descriptor
-    // is in the brt header. we need a copy of the pointer in the DB.
-    // TODO: there may be a cleaner way to do this. 
-    // toku_ft_get_descriptor(db, &cmp_desc, &desc); ??
-    db->descriptor = &brt->ft->descriptor;
-    db->cmp_descriptor = &brt->ft->cmp_descriptor;
+    // now that the handle has successfully opened, a valid descriptor
+    // is in the ft. we need to set the db's descriptor pointers
+    db_set_descriptors(db, ft_handle);
 
     if (need_locktree) {
         db->i->dict_id = toku_ft_get_dictionary_id(db->i->ft_handle);