[t:5112] removed the fd param from update descriptor, improved the name,...

[t:5112] removed the fd param from update descriptor, improved the name, removed some unnecessary return codes from functions that cannot fail

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

ft/ft-internal.h

@@ -463,7 +463,7 @@ struct ft {
 };
 
 // Copy the descriptor into a temporary variable, and tell DRD that subsequent code happens after reading that pointer.
-// In combination with the annotation in toku_update_descriptor, this seems to be enough to convince test_4015 that all is well.
+// In combination with the annotation in toku_ft_update_descriptor, this seems to be enough to convince test_4015 that all is well.
 // Otherwise, drd complains that the newly malloc'd descriptor string is touched later by some comparison operation.
 static const struct __toku_db zero_db; // it's static, so it's all zeros.  icc needs this to be a global
 static inline void setup_fake_db (DB *fake_db, DESCRIPTOR orig_desc) {
@@ -595,7 +595,7 @@ int toku_serialize_ft_to_wbuf (
     DISKOFF translation_size_on_disk
     );
 int toku_deserialize_ft_from (int fd, LSN max_acceptable_lsn, FT *ft);
-int toku_serialize_descriptor_contents_to_fd(int fd, const DESCRIPTOR desc, DISKOFF offset);
+void toku_serialize_descriptor_contents_to_fd(int fd, const DESCRIPTOR desc, DISKOFF offset);
 void toku_serialize_descriptor_contents_to_wbuf(struct wbuf *wb, const DESCRIPTOR desc);
 BASEMENTNODE toku_create_empty_bn(void);
 BASEMENTNODE toku_create_empty_bn_no_buffer(void); // create a basement node with a null buffer.

ft/ft-ops.c

@@ -3074,7 +3074,6 @@ toku_ft_change_descriptor(
     )
 {
     int r = 0;
-    int fd;
     DESCRIPTOR_S new_d;
     BYTESTRING old_desc_bs = { old_descriptor->size, old_descriptor->data };
     BYTESTRING new_desc_bs = { new_descriptor->size, new_descriptor->data };
@@ -3108,8 +3107,7 @@ toku_ft_change_descriptor(
 
     // write new_descriptor to header
     new_d.dbt = *new_descriptor;
-    fd = toku_cachefile_get_fd (ft_h->ft->cf);
-    r = toku_update_descriptor(ft_h->ft, &new_d, fd);
+    toku_ft_update_descriptor(ft_h->ft, &new_d);
     // very infrequent operation, worth precise threadsafe count
     if (r == 0) {
         STATUS_VALUE(FT_DESCRIPTOR_SET)++;

ft/ft-ops.h

@@ -215,7 +215,6 @@ struct ftstat64_s {
     u_int64_t dsize; /* estimate the sum of the sizes of the pairs (exact when flattened and committed) */
     u_int64_t fsize;  /* the size of the underlying file                                                */
     u_int64_t ffree; /* Number of free bytes in the underlying file                                    */
-    // 4018
     u_int64_t create_time_sec; /* creation time in seconds. */
     u_int64_t modify_time_sec; /* time of last serialization, in seconds. */ 
     u_int64_t verify_time_sec; /* time of last verification, in seconds */

ft/ft-serialize.c

@@ -46,9 +46,8 @@ toku_serialize_descriptor_contents_to_wbuf(struct wbuf *wb, const DESCRIPTOR des
 //Descriptor is written to disk during toku_ft_handle_open iff we have a new (or changed)
 //descriptor.
 //Descriptors are NOT written during the header checkpoint process.
-int
+void
 toku_serialize_descriptor_contents_to_fd(int fd, const DESCRIPTOR desc, DISKOFF offset) {
-    int r = 0;
     // make the checksum
     int64_t size = toku_serialize_descriptor_size(desc)+4; //4 for checksum
     struct wbuf w;
@@ -65,7 +64,6 @@ toku_serialize_descriptor_contents_to_fd(int fd, const DESCRIPTOR desc, DISKOFF
         toku_os_full_pwrite(fd, w.buf, size, offset);
     }
     toku_free(w.buf);
-    return r;
 }
 
 static void
@@ -334,10 +332,7 @@ deserialize_ft_versioned(int fd, struct rbuf *rb, FT *ftp, uint32_t version)
     // version if it gets written out, we need to write the descriptor in
     // the new format (without those bytes) before that happens.
     if (version <= FT_LAYOUT_VERSION_13) {
-        r = toku_update_descriptor(ft, &ft->cmp_descriptor, fd);
-        if (r != 0) {
-            goto exit;
-        }
+        toku_ft_update_descriptor(ft, &ft->cmp_descriptor);
     }
     r = 0;
 exit:

ft/ft.c

@@ -209,7 +209,7 @@ ft_checkpoint (CACHEFILE cf, int fd, void *header_v) {
             r = toku_logger_fsync_if_lsn_not_fsynced(logger, ch->checkpoint_lsn);
             if (r!=0) goto handle_error;
         }
-        uint64_t now = (uint64_t) time(NULL); // 4018;
+        uint64_t now = (uint64_t) time(NULL);
         ft->h->time_of_last_modification = now;
         ch->time_of_last_modification = now;
         ch->checkpoint_count++;
@@ -906,49 +906,41 @@ toku_ft_stat64 (FT ft, struct ftstat64_s *s) {
         n = 0;
     }
     s->dsize = n; 
-
-    // 4018
     s->create_time_sec = ft->h->time_of_creation;
     s->modify_time_sec = ft->h->time_of_last_modification;
     s->verify_time_sec = ft->h->time_of_last_verification;    
 }
 
-// TODO: (Zardosht), once the fdlock has been removed from cachetable, remove
-// fd as parameter and access it in this function
-int 
-toku_update_descriptor(FT h, DESCRIPTOR d, int fd) 
-// Effect: Change the descriptor in a tree (log the change, make sure it makes it to disk eventually).
-//  Updates to the descriptor must be performed while holding some sort of lock.  (In the ydb layer
-//  there is a row lock on the directory that provides exclusion.)
+void 
+toku_ft_update_descriptor(FT ft, DESCRIPTOR d) 
+// Effect: Changes the descriptor in a tree (log the change, make sure it makes it to disk eventually).
+// requires: updates do not happen in parallel for an FT (ydb layer uses a row lock to enforce this)
 {
-    int r = 0;
-    DISKOFF offset;
-    // 4 for checksum
-    toku_realloc_descriptor_on_disk(h->blocktable, toku_serialize_descriptor_size(d)+4, &offset, h, fd);
-    r = toku_serialize_descriptor_contents_to_fd(fd, d, offset);
-    if (r) {
-        goto cleanup;
-    }
-    if (h->descriptor.dbt.data) {
-        toku_free(h->descriptor.dbt.data);
-    }
-    h->descriptor.dbt.size = d->dbt.size;
-    h->descriptor.dbt.data = toku_memdup(d->dbt.data, d->dbt.size);
+    // the checksum is four bytes, so that's where the magic number comes from
+    // make space for the new descriptor and write it out to disk
+    DISKOFF offset, size;
+    size = toku_serialize_descriptor_size(d) + 4;
+    int fd = toku_cachefile_get_fd(ft->cf);
+    toku_realloc_descriptor_on_disk(ft->blocktable, size, &offset, ft, fd);
+    toku_serialize_descriptor_contents_to_fd(fd, d, offset);
 
-    r = 0;
-cleanup:
-    return r;
+    // cleanup the old descriptor and set the in-memory descriptor to the new one
+    if (ft->descriptor.dbt.data) {
+        toku_free(ft->descriptor.dbt.data);
+    }
+    ft->descriptor.dbt.size = d->dbt.size;
+    ft->descriptor.dbt.data = toku_memdup(d->dbt.data, d->dbt.size);
 }
 
 void 
-toku_ft_update_cmp_descriptor(FT h) {
-    if (h->cmp_descriptor.dbt.data != NULL) {
-        toku_free(h->cmp_descriptor.dbt.data);
-    }
-    h->cmp_descriptor.dbt.size = h->descriptor.dbt.size;
-    h->cmp_descriptor.dbt.data = toku_xmemdup(
-        h->descriptor.dbt.data, 
-        h->descriptor.dbt.size
+toku_ft_update_cmp_descriptor(FT ft) {
+    if (ft->cmp_descriptor.dbt.data != NULL) {
+        toku_free(ft->cmp_descriptor.dbt.data);
+    }
+    ft->cmp_descriptor.dbt.size = ft->descriptor.dbt.size;
+    ft->cmp_descriptor.dbt.data = toku_xmemdup(
+        ft->descriptor.dbt.data, 
+        ft->descriptor.dbt.size
         );
 }
 

ft/ft.h

@@ -78,9 +78,9 @@ void toku_ft_set_new_root_blocknum(FT h, CACHEKEY new_root_key);
 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);
-int toku_update_descriptor(FT h, DESCRIPTOR d, int fd);
-// Note: See the locking discussion in ft-ops.c for toku_ft_change_descriptor and toku_update_descriptor.
-void toku_ft_update_cmp_descriptor(FT h);
+
+void toku_ft_update_descriptor(FT h, DESCRIPTOR d);
+void toku_ft_update_cmp_descriptor(FT ft);
 
 void toku_ft_update_stats(STAT64INFO headerstats, STAT64INFO_S delta);
 void toku_ft_decrease_stats(STAT64INFO headerstats, STAT64INFO_S delta);

ft/roll.c

@@ -531,27 +531,24 @@ toku_rollback_change_fdescriptor(FILENUM    filenum,
 {
     CACHEFILE cf;
     int r;
-    int fd;
     r = toku_cachefile_of_filenum(txn->logger->ct, filenum, &cf);
-    if (r==ENOENT) { //Missing file on recovered transaction is not an error
+    if (r == ENOENT) { //Missing file on recovered transaction is not an error
         assert(txn->recovered_from_checkpoint);
         r = 0;
         goto done;
     }
     // file must be open, because the txn that created it opened it and
     // noted it, 
-    assert(r==0);
+    assert(r == 0);
 
-    fd = toku_cachefile_get_fd(cf);
-    OMTVALUE hv=NULL;
-    r = toku_omt_find_zero(txn->open_fts, find_ft_from_filenum, &filenum, &hv, NULL);
-    assert(r==0);
-    FT h = hv;
-    DESCRIPTOR_S d;
+    OMTVALUE ftv = NULL;
+    r = toku_omt_find_zero(txn->open_fts, find_ft_from_filenum, &filenum, &ftv, NULL);
+    assert(r == 0);
+    FT ft = ftv;
 
+    DESCRIPTOR_S d;
     toku_fill_dbt(&d.dbt,  old_descriptor.data,  old_descriptor.len);
-    r = toku_update_descriptor(h, &d, fd);
-    assert(r == 0);
+    toku_ft_update_descriptor(ft, &d);
 done:
     return r;
 }

src/ydb_db.c

@@ -552,7 +552,6 @@ toku_db_stat64(DB * db, DB_TXN *txn, DB_BTREE_STAT64 *s) {
         s->bt_ndata = ftstat.ndata;
         s->bt_dsize = ftstat.dsize;
         s->bt_fsize = ftstat.fsize;
-        // 4018
         s->bt_create_time_sec = ftstat.create_time_sec;
         s->bt_modify_time_sec = ftstat.modify_time_sec;
         s->bt_verify_time_sec = ftstat.verify_time_sec;