Merge pull request #8199 from sfc-gh-tclinkenbeard/improve-code-coverage

Add `probe::assert::RocksDB` code probe annotation

fdbserver/CMakeLists.txt

@@ -7,8 +7,6 @@ else()
 endif()
 
 if (WITH_ROCKSDB_EXPERIMENTAL)
-  add_definitions(-DSSD_ROCKSDB_EXPERIMENTAL)
-
   include(CompileRocksDB)
   # CompileRocksDB sets `lz4_LIBRARIES` to be the shared lib, we want to link
   # statically, so find the static library here.
@@ -44,6 +42,7 @@ else()
 endif()
 
 target_link_libraries(fdbserver PRIVATE toml11_target jemalloc rapidjson)
+target_compile_definitions(fdbserver PRIVATE SSD_ROCKSDB_EXPERIMENTAL)
 # target_compile_definitions(fdbserver PRIVATE -DENABLE_SAMPLING)
 
 if(GPERFTOOLS_FOUND)

fdbserver/SimulatedCluster.actor.cpp

@@ -59,6 +59,27 @@ extern const char* getSourceVersion();
 
 using namespace std::literals;
 
+namespace probe {
+
+namespace assert {
+
+struct HasRocksDB {
+	constexpr static AnnotationType type = AnnotationType::Assertion;
+	bool operator()(ICodeProbe const* self) const {
+#ifdef SSD_ROCKSDB_EXPERIMENTAL
+		return true;
+#else
+		return false;
+#endif
+	}
+};
+
+constexpr HasRocksDB hasRocksDB;
+
+} // namespace assert
+
+} // namespace probe
+
 // TODO: Defining these here is just asking for ODR violations.
 template <>
 std::string describe(bool const& val) {
@@ -1461,7 +1482,7 @@ void SimulationConfig::setStorageEngine(const TestConfig& testConfig) {
 		break;
 	}
 	case 4: {
-		CODE_PROBE(true, "Simulated cluster using RocksDB storage engine");
+		CODE_PROBE(true, "Simulated cluster using RocksDB storage engine", probe::assert::hasRocksDB);
 		set_config("ssd-rocksdb-v1");
 		// Tests using the RocksDB engine are necessarily non-deterministic because of RocksDB
 		// background threads.
@@ -1471,7 +1492,7 @@ void SimulationConfig::setStorageEngine(const TestConfig& testConfig) {
 		break;
 	}
 	case 5: {
-		CODE_PROBE(true, "Simulated cluster using Sharded RocksDB storage engine");
+		CODE_PROBE(true, "Simulated cluster using Sharded RocksDB storage engine", probe::assert::hasRocksDB);
 		set_config("ssd-sharded-rocksdb");
 		// Tests using the RocksDB engine are necessarily non-deterministic because of RocksDB
 		// background threads.

flow/CodeProbe.cpp

@@ -228,12 +228,12 @@ void CodeProbes::traceMissedProbes(Optional<ExecutionContext> context) const {
 		std::tie(iter, std::ignore) = locations.emplace(probe.first, false);
 		iter->second = iter->second || probe.second->wasHit();
 	}
-	for (auto probe : codeProbes) {
-		auto iter = locations.find(probe.first);
+	for (const auto& [loc, probe] : codeProbes) {
+		auto iter = locations.find(loc);
 		ASSERT(iter != locations.end());
-		if (!iter->second) {
+		if (!iter->second && probe->shouldTrace()) {
 			iter->second = true;
-			probe.second->trace(false);
+			probe->trace(false);
 		}
 	}
 }
@@ -280,11 +280,11 @@ void ICodeProbe::printProbesJSON(std::vector<std::string> const& ctxs) {
 // annotations
 namespace assert {
 
-bool NoSim::operator()(ICodeProbe* self) const {
+bool NoSim::operator()(ICodeProbe const* self) const {
 	return !g_network->isSimulated();
 }
 
-bool SimOnly::operator()(ICodeProbe* self) const {
+bool SimOnly::operator()(ICodeProbe const* self) const {
 	return g_network->isSimulated();
 }
 

flow/include/flow/CodeProbe.h

@@ -64,11 +64,11 @@ operator|(Left const& lhs, Right const& rhs) {
 namespace assert {
 struct NoSim {
 	constexpr static AnnotationType type = AnnotationType::Assertion;
-	bool operator()(ICodeProbe* self) const;
+	bool operator()(ICodeProbe const* self) const;
 };
 struct SimOnly {
 	constexpr static AnnotationType type = AnnotationType::Assertion;
-	bool operator()(ICodeProbe* self) const;
+	bool operator()(ICodeProbe const* self) const;
 };
 
 template <class Left, class Right>
@@ -135,6 +135,7 @@ struct CodeProbeAnnotations<> {
 	constexpr bool expectContext(ExecutionContext context, bool prevHadSomeContext = false) const {
 		return !prevHadSomeContext;
 	}
+	constexpr bool shouldTrace(ICodeProbe const*) const { return true; }
 	constexpr bool deduplicate() const { return false; }
 };
 
@@ -156,6 +157,16 @@ struct CodeProbeAnnotations<Head, Tail...> {
 		}
 		tail.hit(self);
 	}
+
+	bool shouldTrace(ICodeProbe const* self) const {
+		if constexpr (Head::type == AnnotationType::Assertion) {
+			if (!head(self)) {
+				return false;
+			}
+		}
+		return tail.shouldTrace(self);
+	}
+
 	void trace(const ICodeProbe* self, BaseTraceEvent& evt, bool condition) const {
 		if constexpr (Head::type == AnnotationType::Decoration) {
 			head.trace(self, evt, condition);
@@ -199,6 +210,7 @@ struct ICodeProbe {
 	virtual const char* condition() const = 0;
 	virtual const char* compilationUnit() const = 0;
 	virtual void trace(bool) const = 0;
+	virtual bool shouldTrace() const = 0;
 	virtual bool wasHit() const = 0;
 	virtual unsigned hitCount() const = 0;
 	virtual bool expectInContext(ExecutionContext context) const = 0;
@@ -232,6 +244,9 @@ struct CodeProbeImpl : ICodeProbe {
 		    .detail("Comment", Comment::value());
 		annotations.trace(this, evt, condition);
 	}
+
+	bool shouldTrace() const override { return annotations.shouldTrace(this); }
+
 	bool wasHit() const override { return _hitCount > 0; }
 	unsigned hitCount() const override { return _hitCount; }