From 53fcc77e7d077f1a9ca17d63787634baa1d2893c Mon Sep 17 00:00:00 2001 From: John Mainzer Date: Wed, 26 Nov 2008 23:48:40 -0500 Subject: [svn-r16136] Several changes: Modified H5C2_journal_post_flush() write the super block and flush the file before truncating the journal. Failure to do this opened a window in which the application could crash leaving the HDF5 file in a state that was un-recoverable. The hope is that this will fix the file recovery bug observed on RSQ -- but I have not been able to test there. However, I was able to generate a similar bug on Linew, and this fix seems to deal with the Linew bug. Added a third test to the h5recovery tests. This is really a test for the library, but it was easier to use existing test code there to construct the new test. The new test runs the same application repeatedly, but setting a timer to crash the application at progressively later times. The object is to search for windows in which the application leaves the HDF5 file in an un-recoverable state. Also, updated H5recover.c to use HDstrtoll() instead of HDstrtod() to read some addresses and such from the journal file. Tested serial (debug) on Phoenix and Linew, and parallel (debug) on Jam. --- src/H5C2.c | 7 +- src/H5C2journal.c | 35 +++++- src/H5C2private.h | 4 +- src/H5FDsec2.c | 3 + tools/h5recover/h5recover.c | 41 +++++-- tools/h5recover/testh5recover.sh.in | 224 ++++++++++++++++++++++++++++++++++++ 6 files changed, 299 insertions(+), 15 deletions(-) diff --git a/src/H5C2.c b/src/H5C2.c index 1be0867..64fa9fd 100644 --- a/src/H5C2.c +++ b/src/H5C2.c @@ -1723,7 +1723,7 @@ H5C2_flush_cache(const H5F_t *f, if ( cache_ptr->mdj_enabled ) { - status = H5C2_journal_post_flush(cache_ptr, + status = H5C2_journal_post_flush(f, dxpl_id, cache_ptr, (hbool_t)(flush_marked_entries == FALSE)); if ( status != SUCCEED ) { @@ -9179,8 +9179,11 @@ H5C2_load_entry(H5F_t * f, { #if 0 /* JRM */ HDfprintf(stdout, "can't read image.\n."); + HDfprintf(stdout, "%s: addr = 0X%llx, len = %d.\n", + FUNC, (long long)addr, (int)len); + fflush(stdout); #endif /* JRM */ - HGOTO_ERROR(H5E_CACHE, H5E_CANTLOAD, NULL, "Can't read image") + HGOTO_ERROR(H5E_CACHE, H5E_CANTLOAD, NULL, "Can't read image*") } thing = type->deserialize(addr, len, image_ptr, udata_ptr, &dirty); diff --git a/src/H5C2journal.c b/src/H5C2journal.c index b8ec171..ab79a7b 100644 --- a/src/H5C2journal.c +++ b/src/H5C2journal.c @@ -184,6 +184,7 @@ H5C2_begin_journaling(H5F_t * f, FUNC); #endif /* JRM */ + /* Note that this call flushes the HDF5 file in passing */ result = H5C2_mark_journaling_in_progress(f, dxpl_id, config_ptr->journal_file_path); @@ -305,7 +306,8 @@ done: * journal file. * * 2) Mark the superblock to indicate that we are no longer - * journaling. + * journaling. Note that this will flush the HDF5 file + * again in passing. * * 3) Tell the journal file write code to shutdown. This will * also cause the journal file to be deleted. @@ -631,9 +633,11 @@ done: * * 3) If the cache_is_clean parameter is true: * - * a) Truncate the journal file + * a) Flush the HDF5 file * - * b) Reset cache_ptr->trans_num and + * b) Truncate the journal file + * + * c) Reset cache_ptr->trans_num and * cache_ptr->last_trans_on_disk to zero. * * Return: Success: SUCCEED @@ -646,7 +650,9 @@ done: */ herr_t -H5C2_journal_post_flush(H5C2_t * cache_ptr, +H5C2_journal_post_flush(const H5F_t * f, + hid_t dxpl_id, + H5C2_t * cache_ptr, hbool_t cache_is_clean) { herr_t result; @@ -654,6 +660,7 @@ H5C2_journal_post_flush(H5C2_t * cache_ptr, FUNC_ENTER_NOAPI(H5C2_journal_post_flush, FAIL) + HDassert( f != NULL ); HDassert( cache_ptr != NULL ); HDassert( cache_ptr->magic == H5C2__H5C2_T_MAGIC ); HDassert( cache_ptr->mdj_enabled ); @@ -672,6 +679,24 @@ H5C2_journal_post_flush(H5C2_t * cache_ptr, if ( cache_is_clean ) { + /* Write the superblock to disk */ + + result = H5F_super_write(f, dxpl_id); + + if ( result != SUCCEED ) { + + HGOTO_ERROR(H5E_CACHE, H5E_WRITEERROR, FAIL, \ + "unable to write superblock to file") + } + + result = H5FD_flush(f->shared->lf, dxpl_id, (unsigned)0); + + if ( result > 0 ) { + + HGOTO_ERROR(H5E_IO, H5E_WRITEERROR, FAIL, \ + "low level flush failed") + } + result = H5C2_jb__trunc(&(cache_ptr->mdj_jbrb)); if ( result != SUCCEED ) { @@ -705,7 +730,7 @@ done: * * 3) Get the ID of the last transaction on disk. * - * 4) If the value obtained in 2) above has changed, + * 4) If the value obtained in 3) above has changed, * remove all entries whose last transaction has * made it to disk from the journal write in progress * list. diff --git a/src/H5C2private.h b/src/H5C2private.h index ce4388b..833bfa1 100644 --- a/src/H5C2private.h +++ b/src/H5C2private.h @@ -1570,7 +1570,9 @@ H5_DLL herr_t H5C2_end_transaction(H5F_t * f, H5_DLL herr_t H5C2_get_journal_config(H5C2_t * cache_ptr, H5C2_mdj_config_t * config_ptr); -H5_DLL herr_t H5C2_journal_post_flush(H5C2_t * cache_ptr, +H5_DLL herr_t H5C2_journal_post_flush(const H5F_t * f, + hid_t dxpl_id, + H5C2_t * cache_ptr, hbool_t cache_is_clean); H5_DLL herr_t H5C2_journal_pre_flush(H5C2_t * cache_ptr); diff --git a/src/H5FDsec2.c b/src/H5FDsec2.c index b01ba3f..0a3879b 100644 --- a/src/H5FDsec2.c +++ b/src/H5FDsec2.c @@ -903,6 +903,9 @@ H5FD_sec2_flush(H5FD_t *_file, hid_t UNUSED dxpl_id, unsigned UNUSED closing) /* Reset last file I/O information */ file->pos = HADDR_UNDEF; file->op = OP_UNKNOWN; +#if 1 /* JRM */ + fsync(file->fd); +#endif /* JRM */ } done: diff --git a/tools/h5recover/h5recover.c b/tools/h5recover/h5recover.c index 2c3a3b3..9be4f30 100644 --- a/tools/h5recover/h5recover.c +++ b/tools/h5recover/h5recover.c @@ -563,7 +563,7 @@ main (int argc, const char *argv[]) } /* end for */ - size = HDstrtod(tok[3], NULL); + size = (size_t)HDstrtoll(tok[3], NULL, 10); if (max_size < size) { @@ -587,15 +587,30 @@ main (int argc, const char *argv[]) fgets(temp, 100, journal_fp); p = &temp[11]; - - eoa = HDstrtod(p, NULL); + + /* according to the man page, strtoll() should accept a + * "0x" prefix on any base 16 value -- seems this is + * not the case. Deal with this by incrementing p + * past the prefix. + */ + + while ( HDisspace(*(p)) ) { p++; } + + if ( ( *(p) == '0' ) && + ( *(p + 1) == 'x' ) ) { + + p += 2; + } + + eoa = (haddr_t)HDstrtoll(p, NULL, 16); if (eoa == 0) { - error_msg(progname, "Could not convert eoa to integer\n"); + error_msg(progname, + "Could not convert eoa to integer\n"); leave( EXIT_FAILURE); } /* end if */ - + if (update_eoa < eoa) { update_eoa = eoa; @@ -851,7 +866,19 @@ main (int argc, const char *argv[]) if ( verbose ) printf("Converting data from character strings.\n"); /* convert address from character character string */ - address = HDstrtod(tok[6], NULL); + + /* according to the man page, strtoll() should accept a + * "0x" prefix on any base 16 value -- seems this is + * not the case. Deal with this by incrementing tok[6] + * past the prefix. + */ + while ( HDisspace(*(tok[6])) ) { (tok[6])++; } + if ( ( *(tok[6]) == '0' ) && + ( *(tok[6] + 1) == 'x' ) ) { + + (tok[6]) += 2; + } + address = (off_t)HDstrtoll(tok[6], NULL, 16); if (address == 0) { error_msg(progname, "Could not convert address to integer\n"); @@ -862,7 +889,7 @@ main (int argc, const char *argv[]) if ( verbose ) printf(" address : %llx\n", address); /* convert size from character string*/ - size = HDstrtod(tok[4], NULL); + size = (size_t)HDstrtoll(tok[4], NULL, 10); if (size == 0) { error_msg(progname, "Could not convert size to double\n"); diff --git a/tools/h5recover/testh5recover.sh.in b/tools/h5recover/testh5recover.sh.in index 3efe186..469bf86 100644 --- a/tools/h5recover/testh5recover.sh.in +++ b/tools/h5recover/testh5recover.sh.in @@ -155,6 +155,224 @@ MYH5DIFF(){ ############################################################################## +## ## +## The following functions are used to perform a sort of "temporal scan" ## +## of the possible HDF5 file and journal file value created by trecover. ## +## ## +## The basic idea is to do run trecover -a n over a wide selection of ## +## values for n in the hope that if there is a window in which the journal ## +## is not maintained properly, we will find it when we force a crash, and ## +## then attempt to recover. ## +## ## +## The following pseudo code describes the test: ## +## ## +## let n = 0.01; // these values are arbitrary ## +## let incr = 0.001; // for now ## +## let fail = false; ## +## ## +## do { ## +## trecover -a n ## +## ## +## if ( trecover.h5.jnl exists ) ## +## { ## +## if ( h5dump trecover.h5 is able to open trecover.h5 ) ## +## { ## +## ## +## fail = true; ## +## ## +## } else if ( h5recover -j trecover.h5.jnl trecover.h5 fails ) { ## +## ## +## fail = true; ## +## ## +## } else if ( h5dump trecover.h5 can't open trecover.h5 ) { ## +## ## +## fail = true; ## +## } ## +## } else if ( h5dump trecover.h5 can't open trecover.h5 ) { ## +## ## +## fail = true; ## +## } ## +## ## +## n += incr; ## +## ## +## } while ( ( trecover.h5.jnl exists ) && ( not fail ) ) ## +## ## +############################################################################## + +test_point() { + + pass=1 + seconds=$1 + msecs=$2 + + if [ $msecs -lt 10 ] + then + + delay=${seconds}.00${msecs} + + else + + if [ $msecs -lt 100 ] + then + + delay=${seconds}.0${msecs} + + else + + delay=${seconds}.${msecs} + + fi + fi + + echo "trecover -a $delay" + + ./trecover -a $delay > /dev/null 2>&1 + + if [ -f trecover.h5 ] + then + + # echo "trecover.h5 exists" + + ../h5dump/h5dump trecover.h5 > /dev/null 2>&1 + + if [ $? -ne 0 ] + then + + # echo "h5dump trecover.h5 failed (1)" + + if [ -f trecover.h5.jnl ] + then + + # echo "trecover.h5.jnl exists -- try to recover" + + # echo "./h5recover -j trecover.h5.jnl trecover.h5" + + ./h5recover -j trecover.h5.jnl trecover.h5 > /dev/null 2>&1 + + if [ $? -ne 0 ] + then + + # echo "h5recover failed!!! FAILED" + pass=0 + + else + + # echo "h5recover succeeded -- try h5dump again" + + ../h5dump/h5dump trecover.h5 > /dev/null 2>&1 + + if [ $? -ne 0 ] + then + + # echo "h5dump trecover.h5 failed again?!? FAILED" + pass=0 + + else + + # echo "h5dump trecover.h5 succeeded (2) -- PASS" + pass=1 + + fi + + fi + + else + + # echo "trecover.h5.jnl doesn't exist!!! FAILED" + pass=0 + + fi + + else + + # echo "h5dump trecover.h5 succeeded (1) -- PASS" + pass=1 + + fi + fi + + return $pass +} + +run_jnl_invarient_test() { + + echo "Starting journaling invarients test:" + echo "" + echo "This test runs the same journaled test application repeatedly, " + echo "but with a gradually increasing delay before the test procees is" + echo "killed. The opjective is to search for windows in which the " + echo "the journaling invarients are not maintained. If the test finds " + echo "such a case, the file should be unreadable after the journal is " + echo "recovered." + echo "" + echo "This test generates a lot of garbage output, as we are killing the " + echo "test application repeatedly. Ignore this output, and only look at" + echo "the last line." + echo "" + echo "Starting the test now:" + echo "" + echo "Testing journaling invarients...." + echo "" + + finished=0; + sec=0; + msec=0; + + while [ $finished -eq 0 ] + do + msec="`expr $msec + 1`" + + if [ $msec -ge 1000 ] + then + + msec=0; + sec="`expr $sec + 1`" + + fi + + test_point $sec $msec + + pass=$? + + echo "pass = $pass" + + if [ $pass -eq 0 ] + then + + finished=1 + + fi + + if [ ! -f trecover.h5.jnl ] + then + + finished=1 + + fi + + done + + echo -n "Testing journaling invarients" + echo -n " " + + if [ $pass -eq 0 ] + then + + echo "FAILED" + nerrors="`expr $nerrors + 1`" + + else + + echo "PASSED" + + fi + + return + +} + + +############################################################################## ### T H E T E S T S ### ############################################################################## @@ -167,6 +385,12 @@ TOOLTEST default.txt # against the expected results by hand. TOOLTEST async_crash.txt -a 0.05 +# Invarient test +# This is a very different test from the previous TOOLTEST calls -- +# basic idea is to scan the execution time of a journaled process to +# look for points in time at which the journaling invarients are not +# maintained. see comments for details. +run_jnl_invarient_test if test $nerrors -eq 0 ; then echo "All $TOOL tests passed." -- cgit v0.12