Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MDEV-17243: "FSM: no such a transition ABORTING -> REPLICATING" #524

Open
wants to merge 1 commit into
base: 3.x
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion SConstruct
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ env.Append(CPPFLAGS = ' -DHAVE_COMMON_H')

# Common C/CXX flags
# These should be kept minimal as they are appended after C/CXX specific flags
env.Append(CCFLAGS = ' -fPIC -Wall -Wextra -Wno-unused-parameter')
env.Append(CCFLAGS = ' -fPIC -Wall -Wextra -Wno-unused-parameter -Wno-implicit-fallthrough ')

# C-specific flags
env.Prepend(CFLAGS = '-std=c99 -fno-strict-aliasing -pipe ')
Expand Down
3 changes: 2 additions & 1 deletion galera/src/replicator_smm.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -349,8 +349,9 @@ namespace galera
case OOOC:
return true;
case LOCAL_OOOC:
return trx_.is_local();
if (trx_.is_local()) { return true; }
// in case of remote trx fall through
// fall through
case NO_OOOC:
return (last_left + 1 == trx_.global_seqno());
}
Expand Down
86 changes: 71 additions & 15 deletions galera/src/wsdb.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include "trx_handle.hpp"
#include "write_set.hpp"

#include "gu_thread.hpp"
#include "gu_lock.hpp"
#include "gu_throw.hpp"

Expand Down Expand Up @@ -33,10 +34,11 @@ void galera::Wsdb::print(std::ostream& os) const
galera::Wsdb::Wsdb()
:
trx_pool_ (TrxHandle::LOCAL_STORAGE_SIZE(), 512, "LocalTrxHandle"),
trx_map_ (),
trx_mutex_ (),
conn_map_ (),
conn_mutex_()
trx_map_ (),
conn_trx_map_(),
trx_mutex_ (),
conn_map_ (),
conn_mutex_ ()
{}


Expand All @@ -54,6 +56,9 @@ galera::Wsdb::~Wsdb()
assert(conn_map_.size() == 0);
#else
for_each(trx_map_.begin(), trx_map_.end(), Unref2nd<TrxMap::value_type>());
for_each(conn_trx_map_.begin(),
conn_trx_map_.end(),
Unref2nd<ConnTrxMap::value_type>());
#endif // !NDEBUG
}

Expand All @@ -63,9 +68,31 @@ galera::Wsdb::find_trx(wsrep_trx_id_t const trx_id)
{
gu::Lock lock(trx_mutex_);

TrxMap::iterator const i(trx_map_.find(trx_id));
galera::TrxHandle* trx;
/* trx-id = 0 is safe-guard condition.
trx-id is generally assigned from thd->query-id and
query-id default is 0. If background thread try to assign
wsrep_next_trx_id before setting query-id we will hit
this assertion: */
assert(trx_id != 0);

return (trx_map_.end() == i ? 0 : i->second);
if (trx_id != wsrep_trx_id_t(-1))
{
/* trx_id is valid and unique. Search for this trx_id in the
trx_id -> trx map: */
TrxMap::iterator const i(trx_map_.find(trx_id));
trx = (trx_map_.end() == i ? NULL : i->second);
}
else
{
/* trx_id is default, so search for repsective connection id
in connection-transaction map: */
gu_thread_t const id = gu_thread_self();
ConnTrxMap::iterator const i(conn_trx_map_.find(id));
trx = (conn_trx_map_.end() == i ? NULL : i->second);
}

return (trx);
}


Expand All @@ -78,12 +105,28 @@ galera::Wsdb::create_trx(const TrxHandle::Params& params,

gu::Lock lock(trx_mutex_);

std::pair<TrxMap::iterator, bool> i
(trx_map_.insert(std::make_pair(trx_id, trx)));

if (gu_unlikely(i.second == false)) gu_throw_fatal;
galera::TrxHandle* trx_ref;
if (trx_id != wsrep_trx_id_t(-1))
{
/* trx_id is valid, add it to trx-map as valid trx_id, which
is unique accross connections: */
std::pair<TrxMap::iterator, bool> i
(trx_map_.insert(std::make_pair(trx_id, trx)));
if (gu_unlikely(i.second == false)) gu_throw_fatal;
trx_ref = i.first->second;
}
else
{
/* trx_id is default so add trx object to connection map
that is maintained based on gu_thread_id (actually it is
alias for connection_id): */
std::pair<ConnTrxMap::iterator, bool> i
(conn_trx_map_.insert(std::make_pair(gu_thread_self(), trx)));
if (gu_unlikely(i.second == false)) gu_throw_fatal;
trx_ref = i.first->second;
}

return i.first->second;
return (trx_ref);
}


Expand Down Expand Up @@ -153,11 +196,24 @@ galera::Wsdb::get_conn_query(const TrxHandle::Params& params,
void galera::Wsdb::discard_trx(wsrep_trx_id_t trx_id)
{
gu::Lock lock(trx_mutex_);
TrxMap::iterator i;
if ((i = trx_map_.find(trx_id)) != trx_map_.end())
if (trx_id != wsrep_trx_id_t(-1))
{
TrxMap::iterator i;
if ((i = trx_map_.find(trx_id)) != trx_map_.end())
{
i->second->unref();
trx_map_.erase(i);
}
}
else
{
i->second->unref();
trx_map_.erase(i);
ConnTrxMap::iterator i;
gu_thread_t id = gu_thread_self();
if ((i = conn_trx_map_.find(id)) != conn_trx_map_.end())
{
i->second->unref();
conn_trx_map_.erase(i);
}
}
}

Expand Down
19 changes: 19 additions & 0 deletions galera/src/wsdb.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "trx_handle.hpp"
#include "wsrep_api.h"
#include "gu_unordered.hpp"
#include "gu_thread.hpp"

namespace galera
{
Expand Down Expand Up @@ -55,6 +56,23 @@ namespace galera

typedef gu::UnorderedMap<wsrep_trx_id_t, TrxHandle*, TrxHash> TrxMap;

/* TrxMap structure doesn't take into consideration presence of
two trx objects with same trx_id (2^64 - 1 which is default trx_id)
belonging to two different connections.
This eventually causes same trx object to get shared among two
different unrelated connections which causes state inconsistency
leading to crash (RACE CONDITION).
This problem could be solved by taking into consideration conn-id,
but that would invite interface change. To avoid this we maintain a
separate map of such trx objects based on gu_thread_id: */
class ConnTrxHash
{
public:
size_t operator()(const gu_thread_t& key) const { return key; }
};

typedef gu::UnorderedMap<gu_thread_t, TrxHandle*, ConnTrxHash> ConnTrxMap;

class ConnHash
{
public:
Expand Down Expand Up @@ -117,6 +135,7 @@ namespace galera
TrxHandle::LocalPool trx_pool_;

TrxMap trx_map_;
ConnTrxMap conn_trx_map_;
gu::Mutex trx_mutex_;
ConnMap conn_map_;
gu::Mutex conn_mutex_;
Expand Down