Hawkin
/
arangodb
mirror of https://gitee.com/bigwinds/arangodb
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

[3.5] Move Shard Bug 4567124 (#9747) 

* Fixed abort in moveshard in the case new leader is not in Current.

* Fixed special case + updated changelog.

* Update CHANGELOG
This commit is contained in:
Lars Maier 2019-08-19 18:23:12 +02:00 committed by KVS85
parent a608e38992
commit 87ae6165d7
3 changed files with 99 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
CHANGELOG
View File

@ -1,6 +1,9 @@
v3.5.1 (XXXX-XX-XX)
-------------------
* Fixed internal issue #4378: fix bug in MoveShard::abort which causes a
duplicate entry in the follower list.
* Fixed cut'n'pasting code from the documentation into arangosh.
* Fixed issue #9652: fix ArangoSearch wrong name collision and raising

32
arangod/Agency/MoveShard.cpp
View File

@ -840,20 +840,22 @@ arangodb::Result MoveShard::abort(std::string const& reason) {
if (_isLeader) {
// All changes to Plan for all shards:
doForAllShards(_snapshot, _database, shardsLikeMe,
[this, &trx](Slice plan, Slice current, std::string& planPath, std::string& curPath) {
// Restore leader to be _from:
trx.add(VPackValue(planPath));
{
VPackArrayBuilder guard(&trx);
trx.add(VPackValue(_from));
VPackArrayIterator iter(plan);
++iter; // skip the first
while (iter.valid()) {
trx.add(iter.value());
++iter;
}
}
});
[this, &trx](Slice plan, Slice current, std::string& planPath, std::string& curPath) {
// Restore leader to be _from:
trx.add(VPackValue(planPath));
{
VPackArrayBuilder guard(&trx);
trx.add(VPackValue(_from));
for (auto const& srv : VPackArrayIterator(plan)) {
// from could be in plan as <from> or <_from>. Exclude to server always.
if (srv.isEqualString(_from) || srv.isEqualString("_" + _from) || srv.isEqualString(_to)) {
continue ;
}
trx.add(srv);
}
trx.add(VPackValue(_to));
}
});
} else {
// All changes to Plan for all shards:
doForAllShards(_snapshot, _database, shardsLikeMe,
@ -863,7 +865,7 @@ arangodb::Result MoveShard::abort(std::string const& reason) {
{
VPackArrayBuilder guard(&trx);
for (auto const& srv : VPackArrayIterator(plan)) {
if (srv.copyString() != _to) {
if (false == srv.isEqualString(_to)) {
trx.add(srv);
}
}

79
tests/Agency/MoveShardTest.cpp
View File

@ -2057,6 +2057,85 @@ TEST_F(MoveShardTest, aborting_the_job_while_a_leader_transition_is_in_progress_
Verify(Method(mockAgent, write));
}
TEST_F(MoveShardTest, aborting_the_job_while_the_new_leader_is_already_in_place_should_not_break_plan) {
std::function<std::unique_ptr<VPackBuilder>(VPackSlice const&, std::string const&)> createTestStructure = [&](VPackSlice const& s, std::string const& path) {
std::unique_ptr<VPackBuilder> builder;
builder.reset(new VPackBuilder());
if (s.isObject()) {
builder->add(VPackValue(VPackValueType::Object));
for (auto const& it: VPackObjectIterator(s)) {
auto childBuilder = createTestStructure(it.value, path + "/" + it.key.copyString());
if (childBuilder) {
builder->add(it.key.copyString(), childBuilder->slice());
}
}
if (path == "/arango/Target/Pending") {
VPackBuilder pendingJob;
{
VPackObjectBuilder b(&pendingJob);
auto plainJob = createJob(COLLECTION, SHARD_LEADER, FREE_SERVER);
for (auto const& it: VPackObjectIterator(plainJob.slice())) {
pendingJob.add(it.key.copyString(), it.value);
}
pendingJob.add("timeCreated", VPackValue(timepointToString(std::chrono::system_clock::now())));
}
builder->add(jobId, pendingJob.slice());
} else if (path == "/arango/Supervision/DBServers") {
builder->add(FREE_SERVER, VPackValue("1"));
} else if (path == "/arango/Supervision/Shards") {
builder->add(SHARD, VPackValue("1"));
}
builder->close();
} else {
if (path == "/arango/Current/Collections/" + DATABASE + "/" + COLLECTION + "/" + SHARD + "/servers") {
builder->add(VPackValue(VPackValueType::Array));
builder->add(VPackValue("_" + SHARD_LEADER));
builder->add(VPackValue(SHARD_FOLLOWER1));
builder->close();
} else if (path == "/arango/Plan/Collections/" + DATABASE + "/" + COLLECTION + "/shards/" + SHARD) {
builder->add(VPackValue(VPackValueType::Array));
builder->add(VPackValue(FREE_SERVER));
builder->add(VPackValue(SHARD_LEADER));
builder->add(VPackValue(SHARD_FOLLOWER1));
builder->close();
} else {
builder->add(s);
}
}
return builder;
};
Mock<AgentInterface> mockAgent;
When(Method(mockAgent, waitFor)).AlwaysReturn();
When(Method(mockAgent, write)).Do([&](query_t const& q, consensus::AgentInterface::WriteMode w) -> write_ret_t {
auto writes = q->slice()[0][0];
EXPECT_TRUE(writes.get("/arango/Target/Pending/1").get("op").copyString() == "delete");
EXPECT_TRUE(q->slice()[0].length() == 2); // Precondition: to Server not leader yet
EXPECT_TRUE(writes.get("/arango/Supervision/DBServers/" + FREE_SERVER).get("op").copyString() == "delete");
EXPECT_TRUE(writes.get("/arango/Supervision/Shards/" + SHARD).get("op").copyString() == "delete");
EXPECT_TRUE(std::string(writes.get("/arango/Plan/Collections/" + DATABASE + "/" + COLLECTION + "/shards/" + SHARD).typeName()) == "array");
// well apparently this job is not responsible to cleanup its mess
EXPECT_TRUE(writes.get("/arango/Plan/Collections/" + DATABASE + "/" + COLLECTION + "/shards/" + SHARD).length() >= 3);
EXPECT_TRUE(writes.get("/arango/Plan/Collections/" + DATABASE + "/" + COLLECTION + "/shards/" + SHARD)[0].copyString() == SHARD_LEADER);
EXPECT_TRUE(writes.get("/arango/Plan/Collections/" + DATABASE + "/" + COLLECTION + "/shards/" + SHARD)[1].copyString() == SHARD_FOLLOWER1);
EXPECT_TRUE(writes.get("/arango/Plan/Collections/" + DATABASE + "/" + COLLECTION + "/shards/" + SHARD)[2].copyString() == FREE_SERVER);
EXPECT_TRUE(std::string(writes.get("/arango/Target/Failed/1").typeName()) == "object");
return fakeWriteResult;
});
AgentInterface& agent = mockAgent.get();
auto builder = createTestStructure(baseStructure.toBuilder().slice(), "");
EXPECT_TRUE(builder);
Node agency = createAgencyFromBuilder(*builder);
auto moveShard = MoveShard(agency, &agent, PENDING, jobId);
moveShard.abort("test abort");
Verify(Method(mockAgent,write));
}
TEST_F(MoveShardTest, if_we_are_ready_to_resign_the_old_server_then_finally_move_to_the_new_leader) {
std::function<std::unique_ptr<VPackBuilder>(VPackSlice const&, std::string const&)> createTestStructure =
[&](VPackSlice const& s, std::string const& path) {