[rhmessaging-commits] rhmessaging commits: r4318 - in store/trunk/cpp: tests/jrnl and 1 other directory.

Author: kpvdr Date: 2010-09-21 11:53:24 -0400 (Tue, 21 Sep 2010) New Revision: 4318 Modified: store/trunk/cpp/lib/jrnl/jerrno.cpp store/trunk/cpp/lib/jrnl/jerrno.hpp store/trunk/cpp/lib/jrnl/jinf.cpp store/trunk/cpp/lib/jrnl/jinf.hpp store/trunk/cpp/lib/jrnl/lpmgr.hpp store/trunk/cpp/lib/jrnl/rcvdat.hpp store/trunk/cpp/tests/jrnl/_st_helper_fns.hpp store/trunk/cpp/tests/jrnl/_ut_jinf.cpp store/trunk/cpp/tests/jrnl/_ut_lpmgr.cpp Log: Made store recovery checks more stringent to eliminate obvioiusly corrupted files. This change adds a file size consistency check, and where possible, and OWI flag consistency check. The test for this class had to be upgraded also. Modified: store/trunk/cpp/lib/jrnl/jerrno.cpp =================================================================== --- store/trunk/cpp/lib/jrnl/jerrno.cpp 2010-09-21 15:51:59 UTC (rev 4317) +++ store/trunk/cpp/lib/jrnl/jerrno.cpp 2010-09-21 15:53:24 UTC (rev 4318) @@ -127,6 +127,11 @@ const u_int32_t jerrno::JERR_JINF_BADVALUESTR = 0x0c02; const u_int32_t jerrno::JERR_JINF_JDATEMPTY = 0x0c03; const u_int32_t jerrno::JERR_JINF_TOOMANYFILES = 0x0c04; +const u_int32_t jerrno::JERR_JINF_INVALIDFHDR = 0x0c05; +const u_int32_t jerrno::JERR_JINF_STAT = 0x0c06; +const u_int32_t jerrno::JERR_JINF_NOTREGFILE = 0x0c07; +const u_int32_t jerrno::JERR_JINF_BADFILESIZE = 0x0c08; +const u_int32_t jerrno::JERR_JINF_OWIBAD = 0x0c09; // Negative returns for some functions const int32_t jerrno::AIO_TIMEOUT = -1; @@ -222,6 +227,11 @@ _err_map[JERR_JINF_BADVALUESTR] = "JERR_JINF_BADVALUESTR: Bad format for value attribute in jinf file"; _err_map[JERR_JINF_JDATEMPTY] = "JERR_JINF_JDATEMPTY: Journal data files empty."; _err_map[JERR_JINF_TOOMANYFILES] = "JERR_JINF_TOOMANYFILES: Too many journal data files."; + _err_map[JERR_JINF_INVALIDFHDR] = "JERR_JINF_INVALIDFHDR: Invalid journal data file header"; + _err_map[JERR_JINF_STAT] = "JERR_JINF_STAT: Error while trying to stat a journal data file"; + _err_map[JERR_JINF_NOTREGFILE] = "JERR_JINF_NOTREGFILE: Target journal data file is not a regular file"; + _err_map[JERR_JINF_BADFILESIZE] = "JERR_JINF_BADFILESIZE: Journal data file is of incorrect or unexpected size"; + _err_map[JERR_JINF_OWIBAD] = "JERR_JINF_OWIBAD: Journal data files have inconsistent OWI flags; >1 transition found in non-auto-expand or min-size journal"; //_err_map[] = ""; Modified: store/trunk/cpp/lib/jrnl/jerrno.hpp =================================================================== --- store/trunk/cpp/lib/jrnl/jerrno.hpp 2010-09-21 15:51:59 UTC (rev 4317) +++ store/trunk/cpp/lib/jrnl/jerrno.hpp 2010-09-21 15:53:24 UTC (rev 4318) @@ -145,6 +145,11 @@ static const u_int32_t JERR_JINF_BADVALUESTR; ///< Bad format for value attr in jinf file static const u_int32_t JERR_JINF_JDATEMPTY; ///< Journal data files empty static const u_int32_t JERR_JINF_TOOMANYFILES; ///< Too many journal data files + static const u_int32_t JERR_JINF_INVALIDFHDR; ///< Invalid file header + static const u_int32_t JERR_JINF_STAT; ///< Error while trying to stat a file + static const u_int32_t JERR_JINF_NOTREGFILE; ///< Target file is not a regular file + static const u_int32_t JERR_JINF_BADFILESIZE; ///< File is of incorrect or unexpected size + static const u_int32_t JERR_JINF_OWIBAD; ///< OWI inconsistent (>1 transition in non-ae journal) // Negative returns for some functions static const int32_t AIO_TIMEOUT; ///< Timeout waiting for AIO return Modified: store/trunk/cpp/lib/jrnl/jinf.cpp =================================================================== --- store/trunk/cpp/lib/jrnl/jinf.cpp 2010-09-21 15:51:59 UTC (rev 4317) +++ store/trunk/cpp/lib/jrnl/jinf.cpp 2010-09-21 15:53:24 UTC (rev 4318) @@ -42,6 +42,7 @@ #include "jrnl/jerrno.hpp" #include "jrnl/lp_map.hpp" #include <sstream> +#include <sys/stat.h> namespace mrg { @@ -129,7 +130,7 @@ } if (_ae) { - if (_ae_max_jfiles > _num_jfiles) + if (_ae_max_jfiles < _num_jfiles) { oss << "File \"" << _filename << "\": "; oss << "Number of journal files exceeds auto-expansion limit: found=" << _num_jfiles; @@ -173,19 +174,32 @@ void jinf::analyze() { - bool done = false; lp_map early_map; // map for all owi flags same as pfid 0 lp_map late_map; // map for all owi flags opposite to pfid 0 + bool late_latch = false; // latch for owi switchover - if (!_valid_flag) - validate(); - done = false; + if (!_valid_flag) + validate(); + bool done = false; for (u_int16_t pfid=0; pfid<_num_jfiles && !done; pfid++) { std::ostringstream oss; - oss << _jdir << "/" << _base_filename << "."; + if (_jdir.at(_jdir.size() - 1) == '/') + oss << _jdir << _base_filename << "."; + else + oss << _jdir << "/" << _base_filename << "."; oss << std::setw(4) << std::setfill('0') << std::hex << pfid; oss << "." << JRNL_DATA_EXTENSION; + + // Check size of each file is consistent and expected + u_int32_t fsize = get_filesize(oss.str()); + if (fsize != (_jfsize_sblks + 1) * _sblk_size_dblks * _dblk_size) + { + std::ostringstream oss1; + oss1 << "File \"" << oss.str() << "\": size=" << fsize << "; expected=" << ((_jfsize_sblks + 1) * _sblk_size_dblks * _dblk_size); + throw jexception(jerrno::JERR_JINF_BADFILESIZE, oss1.str(), "jinf", "analyze"); + } + std::ifstream jifs(oss.str().c_str()); if (!jifs.good()) throw jexception(jerrno::JERR__FILEIO, oss.str(), "jinf", "analyze"); @@ -193,8 +207,10 @@ jifs.read((char*)&fhdr, sizeof(fhdr)); if (fhdr._magic != RHM_JDAT_FILE_MAGIC) // No file header { + if (fhdr._magic != 0) + throw jexception(jerrno::JERR_JINF_INVALIDFHDR, oss.str(), "jinf", "analyze"); if (!pfid) // pfid 0 == lid 0 cannot be empty - throw jexception(jerrno::JERR_JINF_JDATEMPTY, "jinf", "analyze"); + throw jexception(jerrno::JERR_JINF_JDATEMPTY, oss.str(), "jinf", "analyze"); _frot = true; done = true; } @@ -209,9 +225,16 @@ else { if (_initial_owi == fhdr.get_owi()) + { early_map.insert(fhdr._lfid, pfid); + if (late_latch && (!_ae || _num_jfiles == JRNL_MIN_NUM_FILES)) + throw jexception(jerrno::JERR_JINF_OWIBAD, oss.str(), "jinf", "analyze"); + } else + { late_map.insert(fhdr._lfid, pfid); + late_latch = true; + } } } jifs.close(); @@ -223,6 +246,13 @@ _pfid_list.clear(); late_map.get_pfid_list(_pfid_list); early_map.get_pfid_list(_pfid_list); + + // Check OWI consistency +// for (u_int16_t lfid=0; lfid<_num_jfiles && !done; lfid++) +// { +// throw jexception(jerrno::JERR_JINF_OWIBAD, oss.str(), "jinf", "analyze"); +// } + _analyzed_flag = true; } @@ -404,10 +434,10 @@ string_value(_base_filename, buff); else if(std::strstr(buff, "number_jrnl_files")) _num_jfiles = u_int16_value(buff); + else if(std::strstr(buff, "auto_expand_max_jrnl_files")) + _ae_max_jfiles = u_int16_value(buff); else if(std::strstr(buff, "auto_expand")) _ae = bool_value(buff); - else if(std::strstr(buff, "auto_expand_max_jrnl_files")) - _ae_max_jfiles = u_int16_value(buff); else if(std::strstr(buff, "jrnl_file_size_sblks")) _jfsize_sblks = u_int32_value(buff); else if(std::strstr(buff, "JRNL_SBLK_SIZE")) @@ -483,5 +513,24 @@ return t1; } +u_int32_t +jinf::get_filesize(const std::string& file_name) const +{ + struct stat s; + if (::stat(file_name.c_str(), &s)) + { + std::ostringstream oss; + oss << "stat: file=\"" << file_name << "\"" << FORMAT_SYSERR(errno); + throw jexception(jerrno::JERR_JINF_STAT, oss.str(), "jinf", "get_filesize"); + } + if (!S_ISREG(s.st_mode)) // not a regular file, + { + std::ostringstream oss; + oss << "File \"" << file_name << "\" is not a regular file: mode=0x" << std::hex << s.st_mode; + throw jexception(jerrno::JERR_JINF_NOTREGFILE, oss.str(), "jinf", "get_filesize"); + } + return u_int32_t(s.st_size); +} + } // namespace journal } // namespace mrg Modified: store/trunk/cpp/lib/jrnl/jinf.hpp =================================================================== --- store/trunk/cpp/lib/jrnl/jinf.hpp 2010-09-21 15:51:59 UTC (rev 4317) +++ store/trunk/cpp/lib/jrnl/jinf.hpp 2010-09-21 15:53:24 UTC (rev 4318) @@ -124,6 +124,7 @@ u_int32_t u_int32_value(char* line) const; std::string& string_value(std::string& str, char* line) const; char* find_value(char* line) const; + u_int32_t get_filesize(const std::string& file_name) const; }; } // namespace journal Modified: store/trunk/cpp/lib/jrnl/lpmgr.hpp =================================================================== --- store/trunk/cpp/lib/jrnl/lpmgr.hpp 2010-09-21 15:51:59 UTC (rev 4317) +++ store/trunk/cpp/lib/jrnl/lpmgr.hpp 2010-09-21 15:53:24 UTC (rev 4318) @@ -88,7 +88,7 @@ * </pre> * * The insert() function updates the internal map immediately, but the physical files (which have both the pfid and - * lfid written into the file header) are only updated as they are overwirtten in the normal course of enqueueing + * lfid written into the file header) are only updated as they are overwritten in the normal course of enqueueing * and dequeueing messages. If the %journal should fail after insertion but before the files following those inserted * are overwritten, then duplicate lfids will be present (though no duplicate pfids are possible). The overwrite * indicator (owi) flag and the pfid numbers may be used to resolve the ambiguity and determine the logically earlier Modified: store/trunk/cpp/lib/jrnl/rcvdat.hpp =================================================================== --- store/trunk/cpp/lib/jrnl/rcvdat.hpp 2010-09-21 15:51:59 UTC (rev 4317) +++ store/trunk/cpp/lib/jrnl/rcvdat.hpp 2010-09-21 15:53:24 UTC (rev 4318) @@ -86,14 +86,14 @@ _njf = num_jfiles; _ae = auto_expand; _aemjf = ae_max_jfiles; - _owi=false; + _owi = false; _frot = false; - _jempty=true; - _ffid=0; - _fro=0; - _lfid=0; - _eo=0; - _h_rid=0; + _jempty = true; + _ffid = 0; + _fro = 0; + _lfid = 0; + _eo = 0; + _h_rid = 0; _lffull = false; _jfull = false; _fid_list.clear(); Modified: store/trunk/cpp/tests/jrnl/_st_helper_fns.hpp =================================================================== --- store/trunk/cpp/tests/jrnl/_st_helper_fns.hpp 2010-09-21 15:51:59 UTC (rev 4317) +++ store/trunk/cpp/tests/jrnl/_st_helper_fns.hpp 2010-09-21 15:53:24 UTC (rev 4318) @@ -383,16 +383,16 @@ * header (512 bytes each) and a single jinf file which contains the journal metadata required for recovery * analysis. */ - void write_journal(const bool ae, const u_int16_t ae_max_jfiles) + void write_journal(const bool ae, const u_int16_t ae_max_jfiles, const u_int32_t fsize_sblks = JFSIZE_SBLKS) { create_jinf(ae, ae_max_jfiles); u_int16_t pfid = 0; for (lpmap_citr itr = _map.begin(); itr != _map.end(); itr++, pfid++) { - if (itr->second._pfid == 0 && itr->second._magic == 0) // empty header, use pfid counter instaed - create_journal_file(pfid, itr->second, _base_filename); + if (itr->second._pfid == 0 && itr->second._magic == 0) // empty header, use pfid counter instead + create_journal_file(pfid, itr->second, _base_filename, fsize_sblks); else - create_journal_file(itr->second._pfid, itr->second, _base_filename); + create_journal_file(itr->second._pfid, itr->second, _base_filename, fsize_sblks); } } @@ -481,35 +481,51 @@ ji.write(); } - static void create_journal_file(const u_int16_t pfid, const file_hdr& fh, const string base_filename) + static void create_journal_file(const u_int16_t pfid, + const file_hdr& fh, + const string base_filename, + const u_int32_t fsize_sblks = JFSIZE_SBLKS, + const char fill_char = 0) { - write_file_header(fh, create_journal_filename(pfid, base_filename)); - } - - static void write_file_header(const file_hdr& fh, - const string filename) - { + const std::string filename = create_journal_filename(pfid, base_filename); ofstream of(filename.c_str(), ofstream::out | ofstream::trunc); if (!of.good()) BOOST_FAIL("Unable to open test journal file \"" << filename << "\" for writing."); + write_file_header(filename, of, fh, fill_char); + write_file_body(of, fsize_sblks, fill_char); + + of.close(); + if (of.fail() || of.bad()) + BOOST_FAIL("Error closing test journal file \"" << filename << "\"."); + } + + static void write_file_header(const std::string& filename, ofstream& of, const file_hdr& fh, const char fill_char) + { // write file header u_int32_t cnt = sizeof(file_hdr); of.write((const char*)&fh, cnt); if (of.fail() || of.bad()) BOOST_FAIL("Error writing file header to test journal file \"" << filename << "\"."); - // fill remaining sblk with 0s + // fill remaining sblk with fill char while (cnt++ < JRNL_DBLK_SIZE * JRNL_SBLK_SIZE) { - of.put(0); + of.put(fill_char); if (of.fail() || of.bad()) BOOST_FAIL("Error writing filler to test journal file \"" << filename << "\"."); } + } - of.close(); - if (of.fail() || of.bad()) - BOOST_FAIL("Error closing test journal file \"" << filename << "\"."); + static void write_file_body(ofstream& of, const u_int32_t fsize_sblks, const char fill_char) + { + if (fsize_sblks > 1) + { + std::vector<char> sblk_buffer(JRNL_DBLK_SIZE * JRNL_SBLK_SIZE, fill_char); + u_int32_t fwritten_sblks = 0; // hdr + while (fwritten_sblks++ < fsize_sblks) + of.write(sblk_buffer.data(), JRNL_DBLK_SIZE * JRNL_SBLK_SIZE); + } } static string create_journal_filename(const u_int16_t pfid, const string base_filename) Modified: store/trunk/cpp/tests/jrnl/_ut_jinf.cpp =================================================================== --- store/trunk/cpp/tests/jrnl/_ut_jinf.cpp 2010-09-21 15:51:59 UTC (rev 4317) +++ store/trunk/cpp/tests/jrnl/_ut_jinf.cpp 2010-09-21 15:53:24 UTC (rev 4318) @@ -215,12 +215,12 @@ const string base_filename = test_name + "_bfn"; lfid_pfid_map m(jid, base_filename); for (u_int16_t oldest_lid = 0; oldest_lid < NUM_JFILES; oldest_lid++) - for (u_int16_t after_lid = 0; after_lid < NUM_JFILES; after_lid++) + for (u_int16_t after_lid = 0; after_lid < NUM_JFILES; after_lid++) for (u_int16_t num_files = 1; num_files <= 5; num_files++) { m.journal_create(NUM_JFILES, NUM_JFILES, oldest_lid); m.journal_insert(after_lid, num_files); - m.write_journal(false, 0); + m.write_journal(true, 16); stringstream fn; fn << test_dir << "/" << base_filename << "." << JRNL_INFO_EXTENSION; @@ -251,7 +251,7 @@ const u_int16_t after_lid = u_int16_t(m.size() * ::drand48()); m.journal_insert(after_lid, num_files); } - m.write_journal(false, 0); + m.write_journal(true, 24); stringstream fn; fn << test_dir << "/" << base_filename << "." << JRNL_INFO_EXTENSION; @@ -291,7 +291,7 @@ const u_int16_t num_files = u_int16_t(1 + (NUM_JFILES * ::drand48())); const u_int16_t after_lid = oldest_lid == 0 ? m.size() - 1 : oldest_lid - 1; m.journal_insert(after_lid, num_files, false); - m.write_journal(false, 0); + m.write_journal(true, 32); stringstream fn; fn << test_dir << "/" << base_filename << "." << JRNL_INFO_EXTENSION; Modified: store/trunk/cpp/tests/jrnl/_ut_lpmgr.cpp =================================================================== --- store/trunk/cpp/tests/jrnl/_ut_lpmgr.cpp 2010-09-21 15:51:59 UTC (rev 4317) +++ store/trunk/cpp/tests/jrnl/_ut_lpmgr.cpp 2010-09-21 15:53:24 UTC (rev 4318) @@ -152,7 +152,7 @@ lfm.get_pfid_list(pfidl); lfm.get_lfid_list(lfidl); lm.finalize(); // clear all file handles before erasing old journal files - lfm.write_journal(ae, ae_max_jfiles); + lfm.write_journal(ae, ae_max_jfiles, JFSIZE_SBLKS); lpmgr_test_helper::rcvdat_init(rd, pfidl, ae, ae_max_jfiles); lm.recover(rd, &jc, &jc.new_fcntl); @@ -822,7 +822,7 @@ // Recover from previous iteration lfm.get_pfid_list(pfidl); lfm.get_lfid_list(lfidl); - lfm.write_journal(true, curr_ae_max_jfiles); + lfm.write_journal(true, curr_ae_max_jfiles, JFSIZE_SBLKS); lpmgr_test_helper::rcvdat_init(rd, pfidl, true, curr_ae_max_jfiles); lm.recover(rd, &jc, &jc.new_fcntl); lpmgr_test_helper::check_pfids_lfids(lm, pfidl, lfidl);