RepoMirrors/ceph
1
0
Fork 0
mirror of https://github.com/ceph/ceph synced 2024-12-28 22:43:29 +00:00
Code Issues Packages Projects Releases Wiki Activity

rbd-mirror: ImageMap memory leak fixes

Browse Source 
Signed-off-by: Venky Shankar <vshankar@redhat.com>
This commit is contained in:
Venky Shankar 2017-11-21 11:27:40 -05:00
parent 993b5f304e
commit 156359c9d0
7 changed files with 70 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
src/test/rbd_mirror/test_mock_ImageMap.cc
View File

@ -395,6 +395,8 @@ TEST_F(TestMockImageMap, SetLocalImages) {
// remote peer ACKs image acquire request
remote_peer_ack_nowait(mock_image_map.get(), global_image_ids_ack, 0);
wait_for_scheduled_task();
ASSERT_EQ(0, when_shut_down(mock_image_map.get()));
}
@ -452,6 +454,8 @@ TEST_F(TestMockImageMap, AddRemoveLocalImage) {
ASSERT_TRUE(wait_for_listener_notify(remove_global_image_ids_ack.size()));
remote_peer_ack_wait(mock_image_map.get(), remove_global_image_ids_ack, 0);
wait_for_scheduled_task();
ASSERT_EQ(0, when_shut_down(mock_image_map.get()));
}
@ -510,6 +514,8 @@ TEST_F(TestMockImageMap, AddRemoveRemoteImage) {
ASSERT_TRUE(wait_for_listener_notify(remove_global_image_ids_ack.size() * 2));
remote_peer_ack_wait(mock_image_map.get(), remove_global_image_ids_ack, 0);
wait_for_scheduled_task();
ASSERT_EQ(0, when_shut_down(mock_image_map.get()));
}
@ -578,6 +584,7 @@ TEST_F(TestMockImageMap, AddRemoveRemoteImageDuplicateNotification) {
// trigger duplicate "remove" notification
mock_image_map->update_images("uuid1", {}, std::move(remove_global_image_ids_dup));
wait_for_scheduled_task();
ASSERT_EQ(0, when_shut_down(mock_image_map.get()));
}
@ -623,6 +630,8 @@ TEST_F(TestMockImageMap, AcquireImageErrorRetry) {
// remote peer ACKs image acquire request
remote_peer_ack_nowait(mock_image_map.get(), initial_global_image_ids_ack, 0);
wait_for_scheduled_task();
ASSERT_EQ(0, when_shut_down(mock_image_map.get()));
}
@ -702,6 +711,8 @@ TEST_F(TestMockImageMap, RemoveRemoteAndLocalImage) {
ASSERT_TRUE(wait_for_listener_notify(local_remove_global_image_ids_ack.size()));
remote_peer_ack_wait(mock_image_map.get(), local_remove_global_image_ids_ack, 0);
wait_for_scheduled_task();
ASSERT_EQ(0, when_shut_down(mock_image_map.get()));
}
@ -761,6 +772,8 @@ TEST_F(TestMockImageMap, AddInstance) {
// completion shuffle action for now (re)mapped images
remote_peer_ack_nowait(mock_image_map.get(), shuffled_global_image_ids, 0);
wait_for_scheduled_task();
ASSERT_EQ(0, when_shut_down(mock_image_map.get()));
}
@ -839,6 +852,8 @@ TEST_F(TestMockImageMap, RemoveInstance) {
// completion shuffle action for now (re)mapped images
remote_peer_ack_nowait(mock_image_map.get(), shuffled_global_image_ids, 0);
wait_for_scheduled_task();
ASSERT_EQ(0, when_shut_down(mock_image_map.get()));
}
@ -929,6 +944,8 @@ TEST_F(TestMockImageMap, AddInstancePingPongImageTest) {
shuffled_global_image_ids.begin(), shuffled_global_image_ids.end(),
std::inserter(reshuffled, reshuffled.begin()));
ASSERT_TRUE(reshuffled.empty());
wait_for_scheduled_task();
ASSERT_EQ(0, when_shut_down(mock_image_map.get()));
}
@ -1007,6 +1024,8 @@ TEST_F(TestMockImageMap, RemoveInstanceWithRemoveImage) {
mock_image_map->update_instances_removed({"9876"});
remote_peer_ack_nowait(mock_image_map.get(), shuffled_global_image_ids, -EBLACKLISTED);
wait_for_scheduled_task();
ASSERT_EQ(0, when_shut_down(mock_image_map.get()));
}
@ -1085,6 +1104,8 @@ TEST_F(TestMockImageMap, AddErrorWithRetryAndResume) {
// remote peer ACKs image acquire request
remote_peer_ack_nowait(mock_image_map.get(), shuffled_global_image_ids, 0);
wait_for_scheduled_task();
ASSERT_EQ(0, when_shut_down(mock_image_map.get()));
}
@ -1163,6 +1184,8 @@ TEST_F(TestMockImageMap, MirrorUUIDUpdated) {
// remote peer ACKs image acquire request
remote_peer_ack_nowait(mock_image_map.get(), remote_added_global_image_ids_ack, 0);
wait_for_scheduled_task();
ASSERT_EQ(0, when_shut_down(mock_image_map.get()));
}

9
src/tools/rbd_mirror/ImageMap.cc
View File

@ -327,9 +327,8 @@ void ImageMap<I>::schedule_add_action(const std::string &global_image_id) {
Context *on_acquire = new FunctionContext([this, global_image_id](int r) {
queue_acquire_image(global_image_id);
});
Context *on_finish = new FunctionContext([this, global_image_id, on_update](int r) {
Context *on_finish = new FunctionContext([this, global_image_id](int r) {
handle_add_action(global_image_id, r);
delete on_update;
});
if (m_policy->add_image(global_image_id, on_update, on_acquire, on_finish)) {
@ -346,9 +345,8 @@ void ImageMap<I>::schedule_remove_action(const std::string &global_image_id) {
queue_release_image(global_image_id);
});
Context *on_remove = new C_RemoveMap(this, global_image_id);
Context *on_finish = new FunctionContext([this, global_image_id, on_remove](int r) {
Context *on_finish = new FunctionContext([this, global_image_id](int r) {
handle_remove_action(global_image_id, r);
delete on_remove;
});
if (m_policy->remove_image(global_image_id, on_release, on_remove, on_finish)) {
@ -370,9 +368,8 @@ void ImageMap<I>::schedule_shuffle_action(const std::string &global_image_id) {
Context *on_acquire = new FunctionContext([this, global_image_id](int r) {
queue_acquire_image(global_image_id);
});
Context *on_finish = new FunctionContext([this, global_image_id, on_update](int r) {
Context *on_finish = new FunctionContext([this, global_image_id](int r) {
handle_shuffle_action(global_image_id, r);
delete on_update;
});
if (m_policy->shuffle_image(global_image_id, on_release, on_update, on_acquire, on_finish)) {

2
src/tools/rbd_mirror/ImageMap.h
View File

@ -109,6 +109,8 @@ private:
}
};
// context callbacks which are retry-able get deleted after
// transiting to the next state.
struct C_UpdateMap : Context {
ImageMap *image_map;
std::string global_image_id;

28
src/tools/rbd_mirror/image_map/Action.cc
View File

@ -58,12 +58,32 @@ void Action::execute_state_callback(StateTransition::State state) {
}
}
void Action::execute_completion_callback(int r) {
auto it = context_map.find(StateTransition::STATE_COMPLETE);
assert(it != context_map.end());
void Action::state_callback_complete(StateTransition::State state, bool delete_context) {
Context *on_state = nullptr;
auto it = context_map.find(state);
if (it != context_map.end()) {
std::swap(it->second, on_state);
}
if (on_state && delete_context) {
delete on_state;
}
}
void Action::execute_completion_callback(int r) {
Context *on_finish = nullptr;
std::swap(it->second, on_finish); // just called once so its swap'd
for (auto &ctx : context_map) {
Context *on_state = nullptr;
std::swap(ctx.second, on_state);
if (ctx.first == StateTransition::STATE_COMPLETE) {
on_finish = on_state;
} else if (on_state != nullptr) {
delete on_state;
}
}
if (on_finish != nullptr) {
on_finish->complete(r);

1
src/tools/rbd_mirror/image_map/Action.h
View File

@ -21,6 +21,7 @@ public:
Context *on_finish);
void execute_state_callback(StateTransition::State state);
void state_callback_complete(StateTransition::State state, bool delete_context);
void execute_completion_callback(int r);
StateTransition::ActionType get_action_type() const;

24
src/tools/rbd_mirror/image_map/Policy.cc
View File

@ -173,9 +173,9 @@ bool Policy::finish_action(const std::string &global_image_id, int r) {
bool complete;
if (r == 0) {
post_execute_state_callback(global_image_id, action_state.transition.next_state);
complete = perform_transition(&action_state, action.get_action_type());
complete = perform_transition(&action_state, &action);
} else {
complete = abort_or_retry(&action_state);
complete = abort_or_retry(&action_state, &action);
}
if (complete) {
@ -229,13 +229,15 @@ bool Policy::is_transition_complete(StateTransition::ActionType action_type, Sta
return complete;
}
bool Policy::perform_transition(ActionState *action_state, StateTransition::ActionType action_type) {
bool Policy::perform_transition(ActionState *action_state, Action *action) {
dout(20) << dendl;
assert(m_map_lock.is_wlocked());
StateTransition::State state = action_state->transition.next_state;
// delete context based on retry_on_error flag
action->state_callback_complete(state, action_state->transition.retry_on_error);
bool complete = is_transition_complete(action_type, &state);
bool complete = is_transition_complete(action->get_action_type(), &state);
dout(10) << ": advancing state: " << action_state->current_state << " -> "
<< state << dendl;
@ -248,15 +250,19 @@ bool Policy::perform_transition(ActionState *action_state, StateTransition::Acti
return complete;
}
bool Policy::abort_or_retry(ActionState *action_state) {
bool Policy::abort_or_retry(ActionState *action_state, Action *action) {
dout(20) << dendl;
assert(m_map_lock.is_wlocked());
bool complete = !action_state->transition.retry_on_error;
if (complete && action_state->last_idle_state) {
dout(10) << ": using last idle state=" << action_state->last_idle_state.get()
<< " as current state" << dendl;
action_state->current_state = action_state->last_idle_state.get();
if (complete) {
// we aborted, so the context need not be freed
action->state_callback_complete(action_state->transition.next_state, false);
if (action_state->last_idle_state) {
dout(10) << ": using last idle state=" << action_state->last_idle_state.get()
<< " as current state" << dendl;
action_state->current_state = action_state->last_idle_state.get();
}
}
return complete;

4
src/tools/rbd_mirror/image_map/Policy.h
View File

@ -114,8 +114,8 @@ private:
void post_execute_state_callback(const std::string &global_image_id, StateTransition::State state);
bool is_transition_complete(StateTransition::ActionType action_type, StateTransition::State *state);
bool perform_transition(ActionState *action_state, StateTransition::ActionType action_type);
bool abort_or_retry(ActionState *action_state);
bool perform_transition(ActionState *action_state, Action *action);
bool abort_or_retry(ActionState *action_state, Action *action);
protected:
typedef std::map<std::string, std::set<std::string> > InstanceToImageMap;