Add INDEX FLIP and CP abrupt #407
Conversation
| std::string edge = edge_info.m_bnodeid == empty_bnodeid | ||
| ? "" | ||
| : "edge:" + std::to_string(edge_info.m_bnodeid) + "." + std::to_string(edge_info.m_link_version); | ||
| return fmt::format("{}.{} level:{} nEntries={} {} {} {} node_gen={} ", node_id, link_version, level, nentries, |
There was a problem hiding this comment.
missing version, checksum etc.
src/include/homestore/homestore.hpp
Outdated
| @@ -95,6 +95,31 @@ struct HS_SERVICE { | |||
| } | |||
| }; | |||
|
|
|||
| struct HOMESTORE_FLIPS { | |||
There was a problem hiding this comment.
please move it from homestore.cpp to btree code, these flips are btree specific, you can add a function which sets all this flips .
src/include/homestore/homestore.hpp
Outdated
| @@ -169,7 +194,19 @@ class HomeStore { | |||
| ResourceMgr& resource_mgr() { return *m_resource_mgr.get(); } | |||
| CPManager& cp_mgr() { return *m_cp_mgr.get(); } | |||
| shared< sisl::Evictor > evictor() { return m_evictor; } | |||
|
|
|||
| #ifdef _PRERELEASE | |||
There was a problem hiding this comment.
why one more indirection ? why not directly flip_clinet->inject("...",...)
| @@ -74,6 +74,10 @@ class IndexWBCacheBase { | |||
| /// @param cur_buf | |||
| /// @return | |||
| virtual IndexBufferPtr copy_buffer(const IndexBufferPtr& cur_buf, const CPContext* context) const = 0; | |||
|
|
|||
| #ifdef _PRERELEASE | |||
| virtual void add_to_crashing_buffers(IndexBufferPtr, std::string reason) = 0; | |||
src/lib/index/wb_cache.cpp
Outdated
|
|
||
| #ifdef _PRERELEASE | ||
|
|
||
| if (m_abrupt_flush.load() == true) { |
There was a problem hiding this comment.
couldnt we add a function cp_ctx->isabrupt() ?
There was a problem hiding this comment.
OK. sure I ll add it to the cp_ctx instead of wbcache
| freq.set_percent(100); | ||
| m_fc.inject_noreturn_flip(flip_name, {null_cond}, freq); | ||
| auto hsi = HomeStore::instance(); | ||
| hsi->set_flip_point(flip_name); |
There was a problem hiding this comment.
as noted above, its better to directly set flip points like done in other UT's.
shosseinimotlagh
force-pushed
the
CP_abrupt_FLIP
branch
from
f41a4a4
to
b9ec401
Compare
shosseinimotlagh
force-pushed
the
CP_abrupt_FLIP
branch
from
b9ec401
to
30e8981
Compare
|
|
||
| #ifdef _PRERELEASE | ||
| BTREE_FLIPS m_flips; | ||
| #endif |
There was a problem hiding this comment.
Why should btree maintain a separate vector of flips? Why can't the caller directly set the flip?
| for (const auto& flip : flips) { | ||
| set_flip_point(flip); | ||
| } | ||
| } |
There was a problem hiding this comment.
Please avoid pass-by-value on containers, as it could be very expensive without any hint.
| @@ -147,6 +148,11 @@ class IndexTable : public IndexTableBase, public Btree< K, V > { | |||
| for (const auto& right_child_node : new_nodes) { | |||
| auto right_child = IndexBtreeNode::convert(right_child_node.get()); | |||
| write_node_impl(right_child_node, context); | |||
| #ifdef _PRERELEASE | |||
| if (iomgr_flip::instance()->test_flip("index_right_sibling")) { | |||
There was a problem hiding this comment.
Would suggest a more contextful name for the flip, say crash_on_right_sibling_flush,
crash_on_left_sibling_flush,
crash_on_non_root_parent_flush,
crash_on_root_parent_flush
| const auto& reasons = it->second; | ||
| std::string formatted_reasons = fmt::format("[{}]", fmt::join(reasons, ", ")); | ||
| LOGTRACEMOD(wbcache, "Buffer {} is in crashing_buffers with reason(s): {} - Buffer info: {}", | ||
| buf->to_string(), formatted_reasons, BtreeNode::to_string_buf(buf->raw_buffer())); |
There was a problem hiding this comment.
We don't need another fmt::format over other format right? You could merge them.
No description provided.