From b052e13ff88f1385bf331af54a56075fcc95ca04 Mon Sep 17 00:00:00 2001 From: Sergei Politov Date: Sun, 12 Aug 2018 13:31:19 +0300 Subject: [PATCH] ENG-3612: Fix to execute GetSegmentsSnapshot using lock Summary: There still could be a case when LogReader becomes null during remote bootstrap and shutdown process. So instead of calling GetSegmentsSnapshot directly from the remote bootstrap session, we introduce intermediate GetSegmentsSnapshot in Log, that will make sure the reader is alive while GetSegmentsSnapshot is executed. Test Plan: ybd --cxx-test redisserver_redisserver-test --gtest_filter TestRedisService.AbortQueueOnShutdown -n 1000 -- -p 12 Reviewers: mikhail, bogdan, bharat, hector Reviewed By: hector Subscribers: ybase Differential Revision: https://phabricator.dev.yugabyte.com/D5322 --- src/yb/consensus/log.cc | 9 +++++++++ src/yb/consensus/log.h | 2 ++ src/yb/tserver/remote_bootstrap_session.cc | 2 +- 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/yb/consensus/log.cc b/src/yb/consensus/log.cc index c53f3f60fd86..2f86db5c26b7 100644 --- a/src/yb/consensus/log.cc +++ b/src/yb/consensus/log.cc @@ -862,6 +862,15 @@ LogReader* Log::GetLogReader() const { return reader_.get(); } +Status Log::GetSegmentsSnapshot(SegmentSequence* segments) const { + boost::shared_lock read_lock(state_lock_.get_lock()); + if (!reader_) { + return STATUS(IllegalState, "Log already closed"); + } + + return reader_->GetSegmentsSnapshot(segments); +} + uint64_t Log::OnDiskSize() { SegmentSequence segments; { diff --git a/src/yb/consensus/log.h b/src/yb/consensus/log.h index d21f504bccba..25e25a74d049 100644 --- a/src/yb/consensus/log.h +++ b/src/yb/consensus/log.h @@ -161,6 +161,8 @@ class Log : public RefCountedThreadSafe { // guaranteed to be live as long as the log itself is initialized and live. LogReader* GetLogReader() const; + CHECKED_STATUS GetSegmentsSnapshot(SegmentSequence* segments) const; + void SetMaxSegmentSizeForTests(uint64_t max_segment_size) { max_segment_size_ = max_segment_size; } diff --git a/src/yb/tserver/remote_bootstrap_session.cc b/src/yb/tserver/remote_bootstrap_session.cc index 6db388c2ffff..f264eee5e755 100644 --- a/src/yb/tserver/remote_bootstrap_session.cc +++ b/src/yb/tserver/remote_bootstrap_session.cc @@ -259,7 +259,7 @@ Status RemoteBootstrapSession::Init() { // Get the current segments from the log, including the active segment. // The Log doesn't add the active segment to the log reader's list until // a header has been written to it (but it will not have a footer). - RETURN_NOT_OK(tablet_peer_->log()->GetLogReader()->GetSegmentsSnapshot(&log_segments_)); + RETURN_NOT_OK(tablet_peer_->log()->GetSegmentsSnapshot(&log_segments_)); for (const scoped_refptr& segment : log_segments_) { RETURN_NOT_OK(OpenLogSegmentUnlocked(segment->header().sequence_number())); }