Author: kpvdr Date: 2008-07-24 16:07:07 -0400 (Thu, 24 Jul 2008) New Revision: 2215 Modified: store/trunk/cpp/lib/BdbMessageStore.cpp store/trunk/cpp/lib/jrnl/jdir.cpp store/trunk/cpp/tests/jrnl/_ut_jdir.cpp Log: Possible fix for BZ456272: "Broker fails to create journal directory - File already exists". Removed check for existence of dir at time of creation in class jdir, and handled EEXIST error instead. Also corrected some jdir tests, added additional tests containing double '/' chars in path. Modified: store/trunk/cpp/lib/BdbMessageStore.cpp =================================================================== --- store/trunk/cpp/lib/BdbMessageStore.cpp 2008-07-24 08:51:22 UTC (rev 2214) +++ store/trunk/cpp/lib/BdbMessageStore.cpp 2008-07-24 20:07:07 UTC (rev 2215) @@ -358,6 +358,7 @@ localFileSizeSblks = (u_int32_t) value->get<int>() * JRNL_RMGR_PAGE_SIZE; { + // TODO: Is this mutex necessary? qpid::sys::Mutex::ScopedLock sl(jrnlCreateLock); jQueue = new JournalImpl(queue.getName(), getJrnlDir(queue), string("JournalData"), defJournalGetEventsTimeout, @@ -1490,14 +1491,12 @@ string BdbMessageStore::getJrnlDir(const char* queueName) //for exmaple /var/rhm/ + queueDir/ { std::stringstream dir; - dir << getJrnlBaseDir(); - dir << std::setw(4); - dir << std::setfill('0'); + dir << getJrnlBaseDir() << std::hex << std::setfill('0') << std::setw(4); u_int32_t count = 0; for (u_int32_t i = 0; i < strlen(queueName); i++) { count += queueName[i]; } - dir << (count % 20); + dir << (count % 29); // Use a prime number for better distribution across dirs dir << "/" << queueName << "/"; return dir.str(); } Modified: store/trunk/cpp/lib/jrnl/jdir.cpp =================================================================== --- store/trunk/cpp/lib/jrnl/jdir.cpp 2008-07-24 08:51:22 UTC (rev 2214) +++ store/trunk/cpp/lib/jrnl/jdir.cpp 2008-07-24 20:07:07 UTC (rev 2215) @@ -81,9 +81,9 @@ if (!exists(parent_dir)) create_dir(parent_dir); } - if (!exists(dirname)) + if (::mkdir(dirname.c_str(), S_IRWXU | S_IRWXG | S_IROTH | S_IXOTH)) { - if (::mkdir(dirname.c_str(), S_IRWXU | S_IRWXG | S_IROTH | S_IXOTH)) + if (errno != EEXIST) // Dir exists, ignore { std::ostringstream oss; oss << "dir=\"" << dirname << "\"" << FORMAT_SYSERR(errno); Modified: store/trunk/cpp/tests/jrnl/_ut_jdir.cpp =================================================================== --- store/trunk/cpp/tests/jrnl/_ut_jdir.cpp 2008-07-24 08:51:22 UTC (rev 2214) +++ store/trunk/cpp/tests/jrnl/_ut_jdir.cpp 2008-07-24 20:07:07 UTC (rev 2215) @@ -274,7 +274,7 @@ BOOST_CHECK(jdir::is_dir(test_dir + "/E/F/G/H")); dir1.delete_dir(); BOOST_CHECK(!jdir::exists(test_dir + "/E/F/G/H")); // only H deleted, E/F/G remain - BOOST_CHECK(jdir::exists(test_dir + "/E")); + BOOST_CHECK(jdir::exists(test_dir + "/E/F/G")); jdir::delete_dir(test_dir + "/E"); // delete remaining dirs BOOST_CHECK(!jdir::exists(test_dir + "/E")); @@ -284,51 +284,70 @@ BOOST_CHECK(jdir::is_dir(test_dir + "/F/G/H/I/")); dir2.delete_dir(); BOOST_CHECK(!jdir::exists(test_dir + "/F/G/H/I/")); - BOOST_CHECK(jdir::exists(test_dir + "/F")); + BOOST_CHECK(jdir::exists(test_dir + "/F/G/H/")); jdir::delete_dir(test_dir + "/F"); BOOST_CHECK(!jdir::exists(test_dir + "/F")); + + check_dir_not_existing(test_dir + "/G"); + jdir dir3(test_dir + "/G/H//I//J", "test_base"); // extra '/' in path + dir3.create_dir(); + BOOST_CHECK(jdir::is_dir(test_dir + "/G/H//I//J")); + dir3.delete_dir(); + BOOST_CHECK(!jdir::exists(test_dir + "/G/H//I//J")); + BOOST_CHECK(jdir::exists(test_dir + "/G/H//I")); + jdir::delete_dir(test_dir + "/F"); + BOOST_CHECK(!jdir::exists(test_dir + "/F")); // Use static fn - check_dir_not_existing(test_dir + "/G"); - jdir::create_dir(test_dir + "/G/H/I/J"); - BOOST_CHECK(jdir::is_dir(test_dir + "/G/H/I/J")); - jdir::delete_dir(test_dir + "/G/H/I/J"); - BOOST_CHECK(!jdir::exists(test_dir + "/G/H/I/J")); - BOOST_CHECK(jdir::exists(test_dir + "/G")); - jdir::delete_dir(test_dir + "/G"); - BOOST_CHECK(!jdir::exists(test_dir + "/G")); - check_dir_not_existing(test_dir + "/H"); - jdir::create_dir(test_dir + "/H/I/J/K/"); - BOOST_CHECK(jdir::is_dir(test_dir + "/H/I/J/K/")); - jdir::delete_dir(test_dir + "/H/I/J/K/"); - BOOST_CHECK(!jdir::exists(test_dir + "/H/I/J/K/")); - BOOST_CHECK(jdir::exists(test_dir + "/H")); + jdir::create_dir(test_dir + "/H/I/J/K"); + BOOST_CHECK(jdir::is_dir(test_dir + "/H/I/J/K")); + jdir::delete_dir(test_dir + "/H/I/J/K"); + BOOST_CHECK(!jdir::exists(test_dir + "/H/I/J/K")); // only J deleted, H/I/J remain + BOOST_CHECK(jdir::exists(test_dir + "/H/I/J")); jdir::delete_dir(test_dir + "/H"); BOOST_CHECK(!jdir::exists(test_dir + "/H")); - - // Non-empty dirs + check_dir_not_existing(test_dir + "/I"); - jdir::create_dir(test_dir + "/I/J/K1/L1"); - jdir::create_dir(test_dir + "/I/J/K1/L2"); - jdir::create_dir(test_dir + "/I/J/K1/L3"); - jdir::create_dir(test_dir + "/I/J/K1/L4"); - create_file(test_dir + "/I/J/K1/L4/test_file_1.txt"); // mode 644 (default) - create_file(test_dir + "/I/J/K1/L4/test_file_2.txt", S_IRWXU | S_IRWXG | S_IRWXO); // mode 777 - create_file(test_dir + "/I/J/K1/L4/test_file_3.txt", S_IRUSR | S_IRGRP | S_IROTH); // mode 444 - create_file(test_dir + "/I/J/K1/L4/test_file_4.txt", 0); // mode 000 (no permissions) - jdir::create_dir(test_dir + "/I/J/K2"); - jdir::create_dir(test_dir + "/I/J/K3/L5"); - jdir::create_dir(test_dir + "/I/J/K3/L6"); - BOOST_CHECK(jdir::is_dir(test_dir + "/I/J/K1/L1")); - BOOST_CHECK(jdir::is_dir(test_dir + "/I/J/K1/L2")); - BOOST_CHECK(jdir::is_dir(test_dir + "/I/J/K1/L3")); - BOOST_CHECK(jdir::is_dir(test_dir + "/I/J/K1/L4")); - BOOST_CHECK(jdir::is_dir(test_dir + "/I/J/K2")); - BOOST_CHECK(jdir::is_dir(test_dir + "/I/J/K3/L5")); - BOOST_CHECK(jdir::is_dir(test_dir + "/I/J/K3/L6")); + jdir::create_dir(test_dir + "/I/J/K/L/"); // trailing '/' + BOOST_CHECK(jdir::is_dir(test_dir + "/I/J/K/L/")); + jdir::delete_dir(test_dir + "/I/J/K/L/"); + BOOST_CHECK(!jdir::exists(test_dir + "/I/J/K/L/")); + BOOST_CHECK(jdir::exists(test_dir + "/I/J/K/")); jdir::delete_dir(test_dir + "/I"); BOOST_CHECK(!jdir::exists(test_dir + "/I")); + + check_dir_not_existing(test_dir + "//J"); + jdir::create_dir(test_dir + "//J//K//L//M"); // extra '/' in path + BOOST_CHECK(jdir::is_dir(test_dir + "//J//K//L//M")); + jdir::delete_dir(test_dir + "//J//K//L//M"); + BOOST_CHECK(!jdir::exists(test_dir + "//J//K//L//M")); + BOOST_CHECK(jdir::exists(test_dir + "//J//K//L")); + jdir::delete_dir(test_dir + "//J"); + BOOST_CHECK(!jdir::exists(test_dir + "//J")); + + // Non-empty dirs + check_dir_not_existing(test_dir + "/K"); + jdir::create_dir(test_dir + "/K/L/M1/N1"); + jdir::create_dir(test_dir + "/K/L/M1/N2"); + jdir::create_dir(test_dir + "/K/L/M1/N3"); + jdir::create_dir(test_dir + "/K/L/M1/N4"); + create_file(test_dir + "/K/L/M1/N4/test_file_1.txt"); // mode 644 (default) + create_file(test_dir + "/K/L/M1/N4/test_file_2.txt", S_IRWXU | S_IRWXG | S_IRWXO); // mode 777 + create_file(test_dir + "/K/L/M1/N4/test_file_3.txt", S_IRUSR | S_IRGRP | S_IROTH); // mode 444 + create_file(test_dir + "/K/L/M1/N4/test_file_4.txt", 0); // mode 000 (no permissions) + jdir::create_dir(test_dir + "/K/L/M2"); + jdir::create_dir(test_dir + "/K/L/M3/N5"); + jdir::create_dir(test_dir + "/K/L/M3/N6"); + BOOST_CHECK(jdir::is_dir(test_dir + "/K/L/M1/N1")); + BOOST_CHECK(jdir::is_dir(test_dir + "/K/L/M1/N2")); + BOOST_CHECK(jdir::is_dir(test_dir + "/K/L/M1/N3")); + BOOST_CHECK(jdir::is_dir(test_dir + "/K/L/M1/N4")); + BOOST_CHECK(jdir::is_dir(test_dir + "/K/L/M2")); + BOOST_CHECK(jdir::is_dir(test_dir + "/K/L/M3/N5")); + BOOST_CHECK(jdir::is_dir(test_dir + "/K/L/M3/N6")); + jdir::delete_dir(test_dir + "/K"); + BOOST_CHECK(!jdir::exists(test_dir + "/K")); cout << "ok" << endl; }