Skip to content

Commit

Permalink
ENG-3612: Fix to execute GetSegmentsSnapshot using lock
Browse files Browse the repository at this point in the history
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
  • Loading branch information
spolitov committed Aug 13, 2018
1 parent 1c6e237 commit b052e13
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 1 deletion.
9 changes: 9 additions & 0 deletions src/yb/consensus/log.cc
Original file line number Diff line number Diff line change
Expand Up @@ -862,6 +862,15 @@ LogReader* Log::GetLogReader() const {
return reader_.get();
}

Status Log::GetSegmentsSnapshot(SegmentSequence* segments) const {
boost::shared_lock<rw_spinlock> read_lock(state_lock_.get_lock());
if (!reader_) {
return STATUS(IllegalState, "Log already closed");
}

return reader_->GetSegmentsSnapshot(segments);
}

uint64_t Log::OnDiskSize() {
SegmentSequence segments;
{
Expand Down
2 changes: 2 additions & 0 deletions src/yb/consensus/log.h
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,8 @@ class Log : public RefCountedThreadSafe<Log> {
// 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;
}
Expand Down
2 changes: 1 addition & 1 deletion src/yb/tserver/remote_bootstrap_session.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<ReadableLogSegment>& segment : log_segments_) {
RETURN_NOT_OK(OpenLogSegmentUnlocked(segment->header().sequence_number()));
}
Expand Down

0 comments on commit b052e13

Please sign in to comment.