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

jewel: The request lock RPC message might be incorrectly ignored #10865

Merged
1 commit merged into from Nov 11, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/librbd/ExclusiveLock.cc
Expand Up @@ -135,7 +135,7 @@ void ExclusiveLock<I>::shut_down(Context *on_shut_down) {
}

// if stalled in request state machine -- abort
handle_lock_released();
handle_peer_notification();
}

template <typename I>
Expand Down Expand Up @@ -209,7 +209,7 @@ void ExclusiveLock<I>::handle_watch_registered() {
}

template <typename I>
void ExclusiveLock<I>::handle_lock_released() {
void ExclusiveLock<I>::handle_peer_notification() {
Mutex::Locker locker(m_lock);
if (m_state != STATE_WAITING_FOR_PEER) {
return;
Expand Down
2 changes: 1 addition & 1 deletion src/librbd/ExclusiveLock.h
Expand Up @@ -42,7 +42,7 @@ class ExclusiveLock {
void release_lock(Context *on_released);

void handle_watch_registered();
void handle_lock_released();
void handle_peer_notification();

void assert_header_locked(librados::ObjectWriteOperation *op);

Expand Down
34 changes: 19 additions & 15 deletions src/librbd/ImageWatcher.cc
Expand Up @@ -481,7 +481,7 @@ void ImageWatcher<I>::handle_request_lock(int r) {
<< dendl;

// treat this is a dead client -- so retest acquiring the lock
m_image_ctx.exclusive_lock->handle_lock_released();
m_image_ctx.exclusive_lock->handle_peer_notification();
} else if (r < 0) {
lderr(m_image_ctx.cct) << this << " error requesting lock: "
<< cpp_strerror(r) << dendl;
Expand Down Expand Up @@ -624,6 +624,11 @@ bool ImageWatcher<I>::handle_payload(const AcquiredLockPayload &payload,
}

RWLock::RLocker owner_locker(m_image_ctx.owner_lock);
if (m_image_ctx.exclusive_lock != nullptr) {
// potentially wake up the exclusive lock state machine now that
// a lock owner has advertised itself
m_image_ctx.exclusive_lock->handle_peer_notification();
}
if (cancel_async_requests &&
(m_image_ctx.exclusive_lock == nullptr ||
!m_image_ctx.exclusive_lock->is_lock_owner())) {
Expand Down Expand Up @@ -661,7 +666,7 @@ bool ImageWatcher<I>::handle_payload(const ReleasedLockPayload &payload,
if (m_image_ctx.exclusive_lock != nullptr &&
!m_image_ctx.exclusive_lock->is_lock_owner()) {
m_task_finisher->cancel(TASK_CODE_REQUEST_LOCK);
m_image_ctx.exclusive_lock->handle_lock_released();
m_image_ctx.exclusive_lock->handle_peer_notification();
}
return true;
}
Expand All @@ -675,24 +680,23 @@ bool ImageWatcher<I>::handle_payload(const RequestLockPayload &payload,
}

RWLock::RLocker l(m_image_ctx.owner_lock);
if (m_image_ctx.exclusive_lock != nullptr) {
int r;
if (m_image_ctx.exclusive_lock->accept_requests(&r)) {
// need to send something back so the client can detect a missing leader
::encode(ResponseMessage(0), ack_ctx->out);

{
Mutex::Locker owner_client_id_locker(m_owner_client_id_lock);
if (!m_owner_client_id.is_valid()) {
return true;
}
if (m_image_ctx.exclusive_lock != nullptr &&
m_image_ctx.exclusive_lock->is_lock_owner()) {
int r = 0;
bool accept_request = m_image_ctx.exclusive_lock->accept_requests(&r);

// need to send something back so the client can detect a missing leader
::encode(ResponseMessage(r), ack_ctx->out);

if (accept_request) {
Mutex::Locker owner_client_id_locker(m_owner_client_id_lock);
if (!m_owner_client_id.is_valid()) {
return true;
}

ldout(m_image_ctx.cct, 10) << this << " queuing release of exclusive lock"
<< dendl;
m_image_ctx.get_exclusive_lock_policy()->lock_requested(payload.force);
} else if (r < 0) {
::encode(ResponseMessage(r), ack_ctx->out);
}
}
return true;
Expand Down
2 changes: 1 addition & 1 deletion src/test/librbd/test_mock_ExclusiveLock.cc
Expand Up @@ -142,7 +142,7 @@ class TestMockExclusiveLock : public TestMockFixture {
MockExclusiveLock &mock_exclusive_lock) {
EXPECT_CALL(*mock_image_ctx.image_watcher, notify_request_lock())
.WillRepeatedly(Invoke(&mock_exclusive_lock,
&MockExclusiveLock::handle_lock_released));
&MockExclusiveLock::handle_peer_notification));
}

void expect_notify_acquired_lock(MockExclusiveLockImageCtx &mock_image_ctx) {
Expand Down