tests: Synchronize boost logger for multithreaded tests in sstable_directory_test

The logger is not thread safe, so a multithreaded test can concurrently
write into the log, yielding unreadable XMLs.

Example:
boost/sstable_directory_test: failed to parse XML output '/scylladir/testlog/x86_64/release/xml/boost.sstable_directory_test.sstable_directory_shared_sstables_reshard_correctly.3.xunit.xml': not well-formed (invalid token): line 1, column 1351

The critical (today's unprotected) section is in boost/test/utils/xml_printer.hpp:
```
inline std::ostream&
operator<<( custom_printer<cdata> const& p, const_string value )
{
    *p << BOOST_TEST_L( "<![CDATA[" );
    print_escaped_cdata( *p, value );
    return  *p << BOOST_TEST_L( "]]>" );
}
```

The problem is not restricted to xml, but the unreadable xml file caused
the test to fail when trying to parse it, to present a summary.

New thread-safe variants of BOOST_REQUIRE and BOOST_REQUIRE_EQUAL are
introduced to help multithreaded tests. We'll start patching tests of
sstable_directory_test that will call BOOST_REQUIRE* from multiple
threads. Later, we can expand its usage to other tests.

Fixes #15654.

Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>

Closes scylladb/scylladb#15655
This commit is contained in:
Raphael S. Carvalho
2023-10-06 15:04:59 -03:00
committed by Avi Kivity
parent 765e193122
commit 4e6fe34501
3 changed files with 21 additions and 5 deletions

View File

@@ -312,8 +312,8 @@ SEASTAR_THREAD_TEST_CASE(sstable_directory_test_generation_sanity) {
sstdir.invoke_on_all([&] (sstables::sstable_directory& sstdir) {
return seastar::async([&] {
sstdir.do_for_each_sstable([&] (const shared_sstable& sst) {
BOOST_REQUIRE(sst->generation() == sst1->generation());
BOOST_REQUIRE(!gen1_seen[this_shard_id()]);
THREADSAFE_BOOST_REQUIRE(sst->generation() == sst1->generation());
THREADSAFE_BOOST_REQUIRE(!gen1_seen[this_shard_id()]);
gen1_seen[this_shard_id()] = true;
return make_ready_future<>();
}).get();
@@ -330,12 +330,12 @@ future<> verify_that_all_sstables_are_local(sharded<sstable_directory>& sstdir,
return d.do_for_each_sstable([count] (sstables::shared_sstable sst) {
count->fetch_add(1, std::memory_order_relaxed);
auto shards = sst->get_shards_for_this_sstable();
BOOST_REQUIRE_EQUAL(shards.size(), 1);
BOOST_REQUIRE_EQUAL(shards[0], this_shard_id());
THREADSAFE_BOOST_REQUIRE_EQUAL(shards.size(), 1);
THREADSAFE_BOOST_REQUIRE_EQUAL(shards[0], this_shard_id());
return make_ready_future<>();
});
}).then([count = count.get(), expected_sstables] {
BOOST_REQUIRE_EQUAL(count->load(std::memory_order_relaxed), expected_sstables);
THREADSAFE_BOOST_REQUIRE_EQUAL(count->load(std::memory_order_relaxed), expected_sstables);
return make_ready_future<>();
});
});

View File

@@ -98,4 +98,6 @@ future<> touch_file(std::string name) {
co_await f.close();
}
std::mutex boost_logger_mutex;
}

View File

@@ -61,4 +61,18 @@ extern boost::test_tools::assertion_result has_scylla_test_env(boost::unit_test:
future<bool> compare_files(std::string fa, std::string fb);
future<> touch_file(std::string name);
extern std::mutex boost_logger_mutex;
#define THREADSAFE_BOOST_CHECK( BOOST_CHECK_EXPR ) { \
std::lock_guard<std::mutex> guard(tests::boost_logger_mutex); \
BOOST_CHECK_EXPR; \
}
// Thread-safe variants of BOOST macros for unit tests
// This is required to address lack of synchronization in boost test logger, which is susceptible to races
// in multi-threaded tests. Boost's XML printer (see boost/test/utils/xml_printer.hpp) can generate
// unreadable XML files, and therefore cause failure when parsing its content.
#define THREADSAFE_BOOST_REQUIRE( P ) THREADSAFE_BOOST_CHECK(BOOST_REQUIRE( P ))
#define THREADSAFE_BOOST_REQUIRE_EQUAL( L, R ) THREADSAFE_BOOST_CHECK(BOOST_REQUIRE_EQUAL( L, R ))
}