Delete FileRotatingStream read support.

Followup to https://webrtc-review.googlesource.com/c/src/+/115302

Bug: webrtc:7811
Change-Id: I81b4eeb4c244b7125ab8e84bc92b03a20e65478f
Reviewed-on: https://webrtc-review.googlesource.com/c/4821
Commit-Queue: Niels Moller <nisse@webrtc.org>
Reviewed-by: Kári Helgason <kthelgason@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#26156}
This commit is contained in:
Niels Möller 2019-01-08 13:22:57 +01:00 committed by Commit Bot
parent d716fb9ecb
commit 6ffe62a437
3 changed files with 69 additions and 201 deletions

View File

@ -169,65 +169,31 @@ absl::optional<size_t> GetFileSize(const std::string& file) {
} // namespace
FileRotatingStream::FileRotatingStream(const std::string& dir_path,
const std::string& file_prefix)
: FileRotatingStream(dir_path, file_prefix, 0, 0, kRead) {}
FileRotatingStream::FileRotatingStream(const std::string& dir_path,
const std::string& file_prefix,
size_t max_file_size,
size_t num_files)
: FileRotatingStream(dir_path,
file_prefix,
max_file_size,
num_files,
kWrite) {
RTC_DCHECK_GT(max_file_size, 0);
RTC_DCHECK_GT(num_files, 1);
}
FileRotatingStream::FileRotatingStream(const std::string& dir_path,
const std::string& file_prefix,
size_t max_file_size,
size_t num_files,
Mode mode)
: dir_path_(AddTrailingPathDelimiterIfNeeded(dir_path)),
file_prefix_(file_prefix),
mode_(mode),
file_stream_(nullptr),
max_file_size_(max_file_size),
current_file_index_(0),
rotation_index_(0),
current_bytes_written_(0),
disable_buffering_(false) {
RTC_DCHECK_GT(max_file_size, 0);
RTC_DCHECK_GT(num_files, 1);
RTC_DCHECK(IsFolder(dir_path));
switch (mode) {
case kWrite: {
file_names_.clear();
for (size_t i = 0; i < num_files; ++i) {
file_names_.push_back(GetFilePath(i, num_files));
}
rotation_index_ = num_files - 1;
break;
}
case kRead: {
file_names_ = GetFilesWithPrefix(dir_path_, file_prefix_);
std::sort(file_names_.begin(), file_names_.end());
if (file_names_.size() > 0) {
// |file_names_| is sorted newest first, so read from the end.
current_file_index_ = file_names_.size() - 1;
}
break;
}
file_names_.clear();
for (size_t i = 0; i < num_files; ++i) {
file_names_.push_back(GetFilePath(i, num_files));
}
rotation_index_ = num_files - 1;
}
FileRotatingStream::~FileRotatingStream() {}
StreamState FileRotatingStream::GetState() const {
if (mode_ == kRead && current_file_index_ < file_names_.size()) {
return SS_OPEN;
}
if (!file_stream_) {
return SS_CLOSED;
}
@ -239,59 +205,14 @@ StreamResult FileRotatingStream::Read(void* buffer,
size_t* read,
int* error) {
RTC_DCHECK(buffer);
if (mode_ != kRead) {
return SR_EOS;
}
if (current_file_index_ >= file_names_.size()) {
return SR_EOS;
}
// We will have no file stream initially, and when we are finished with the
// previous file.
if (!file_stream_) {
if (!OpenCurrentFile()) {
return SR_ERROR;
}
}
int local_error = 0;
if (!error) {
error = &local_error;
}
StreamResult result = file_stream_->Read(buffer, buffer_len, read, error);
if (result == SR_EOS || result == SR_ERROR) {
if (result == SR_ERROR) {
RTC_LOG(LS_ERROR) << "Failed to read from: "
<< file_names_[current_file_index_]
<< "Error: " << error;
}
// Reached the end of the file, read next file. If there is an error return
// the error status but allow for a next read by reading next file.
CloseCurrentFile();
if (current_file_index_ == 0) {
// Just finished reading the last file, signal EOS by setting index.
current_file_index_ = file_names_.size();
} else {
--current_file_index_;
}
if (read) {
*read = 0;
}
return result == SR_EOS ? SR_SUCCESS : result;
} else if (result == SR_SUCCESS) {
// Succeeded, continue reading from this file.
return SR_SUCCESS;
} else {
RTC_NOTREACHED();
}
return result;
RTC_NOTREACHED();
return SR_EOS;
}
StreamResult FileRotatingStream::Write(const void* data,
size_t data_len,
size_t* written,
int* error) {
if (mode_ != kWrite) {
return SR_EOS;
}
if (!file_stream_) {
std::fprintf(stderr, "Open() must be called before Write.\n");
return SR_ERROR;
@ -323,19 +244,9 @@ bool FileRotatingStream::Flush() {
}
bool FileRotatingStream::GetSize(size_t* size) const {
if (mode_ != kRead) {
// Not possible to get accurate size on disk when writing because of
// potential buffering.
return false;
}
RTC_DCHECK(size);
*size = 0;
size_t total_size = 0;
for (auto file_name : file_names_) {
total_size += GetFileSize(file_name).value_or(0);
}
*size = total_size;
return true;
// Not possible to get accurate size on disk when writing because of
// potential buffering.
return false;
}
void FileRotatingStream::Close() {
@ -343,24 +254,15 @@ void FileRotatingStream::Close() {
}
bool FileRotatingStream::Open() {
switch (mode_) {
case kRead:
// Defer opening to when we first read since we want to return read error
// if we fail to open next file.
return true;
case kWrite: {
// Delete existing files when opening for write.
std::vector<std::string> matching_files =
GetFilesWithPrefix(dir_path_, file_prefix_);
for (const auto& matching_file : matching_files) {
if (!DeleteFile(matching_file)) {
std::fprintf(stderr, "Failed to delete: %s\n", matching_file.c_str());
}
}
return OpenCurrentFile();
// Delete existing files when opening for write.
std::vector<std::string> matching_files =
GetFilesWithPrefix(dir_path_, file_prefix_);
for (const auto& matching_file : matching_files) {
if (!DeleteFile(matching_file)) {
std::fprintf(stderr, "Failed to delete: %s\n", matching_file.c_str());
}
}
return false;
return OpenCurrentFile();
}
bool FileRotatingStream::DisableBuffering() {
@ -384,19 +286,11 @@ bool FileRotatingStream::OpenCurrentFile() {
RTC_DCHECK_LT(current_file_index_, file_names_.size());
std::string file_path = file_names_[current_file_index_];
file_stream_.reset(new FileStream());
const char* mode = nullptr;
switch (mode_) {
case kWrite:
mode = "w+";
// We should always we writing to the zero-th file.
RTC_DCHECK_EQ(current_file_index_, 0);
break;
case kRead:
mode = "r";
break;
}
// We should always be writing to the zero-th file.
RTC_DCHECK_EQ(current_file_index_, 0);
int error = 0;
if (!file_stream_->Open(file_path, mode, &error)) {
if (!file_stream_->Open(file_path, "w+", &error)) {
std::fprintf(stderr, "Failed to open: %s Error: %i\n", file_path.c_str(),
error);
file_stream_.reset();
@ -417,7 +311,6 @@ void FileRotatingStream::CloseCurrentFile() {
}
void FileRotatingStream::RotateFiles() {
RTC_DCHECK_EQ(mode_, kWrite);
CloseCurrentFile();
// Rotates the files by deleting the file at |rotation_index_|, which is the
// oldest file and then renaming the newer files to have an incremented index.
@ -458,12 +351,6 @@ std::string FileRotatingStream::GetFilePath(size_t index,
return dir_path_ + file_prefix_ + file_postfix;
}
CallSessionFileRotatingStream::CallSessionFileRotatingStream(
const std::string& dir_path)
: FileRotatingStream(dir_path, kCallSessionLogPrefix),
max_total_log_size_(0),
num_rotations_(0) {}
CallSessionFileRotatingStream::CallSessionFileRotatingStream(
const std::string& dir_path,
size_t max_total_log_size)

View File

@ -83,14 +83,6 @@ class FileRotatingStream : public StreamInterface {
virtual void OnRotation() {}
private:
enum Mode { kRead, kWrite };
FileRotatingStream(const std::string& dir_path,
const std::string& file_prefix,
size_t max_file_size,
size_t num_files,
Mode mode);
bool OpenCurrentFile();
void CloseCurrentFile();
@ -107,7 +99,6 @@ class FileRotatingStream : public StreamInterface {
const std::string dir_path_;
const std::string file_prefix_;
const Mode mode_;
// FileStream is used to write to the current file.
std::unique_ptr<FileStream> file_stream_;
@ -127,7 +118,7 @@ class FileRotatingStream : public StreamInterface {
};
// CallSessionFileRotatingStream is meant to be used in situations where we will
// have limited disk space. Its purpose is to read and write logs up to a
// have limited disk space. Its purpose is to write logs up to a
// maximum size. Once the maximum size is exceeded, logs from the middle are
// deleted whereas logs from the beginning and end are preserved. The reason for
// this is because we anticipate that in WebRTC the beginning and end of the
@ -140,11 +131,11 @@ class FileRotatingStream : public StreamInterface {
// (earliest) file on rotate, and that that file's size is bigger.
//
// Open() must be called before using this stream.
// To read the logs produced by this class, one can use the companion class
// CallSessionFileRotatingStreamReader.
class CallSessionFileRotatingStream : public FileRotatingStream {
public:
// Use this constructor for reading a directory previously written to with
// this stream.
explicit CallSessionFileRotatingStream(const std::string& dir_path);
// Use this constructor for writing to a directory. Files in the directory
// matching what's used by the stream will be deleted. |max_total_log_size|
// must be at least 4.

View File

@ -80,24 +80,9 @@ class MAYBE_FileRotatingStreamTest : public ::testing::Test {
const size_t expected_length,
const std::string& dir_path,
const char* file_prefix) {
// TODO(nisse): Delete the part of this method using read-mode
// FileRotatingStream, together with support for read-mode.
std::unique_ptr<FileRotatingStream> stream;
stream.reset(new FileRotatingStream(dir_path, file_prefix));
ASSERT_TRUE(stream->Open());
size_t read = 0;
size_t stream_size = 0;
EXPECT_TRUE(stream->GetSize(&stream_size));
std::unique_ptr<uint8_t[]> buffer(new uint8_t[expected_length]);
EXPECT_EQ(SR_SUCCESS,
stream->ReadAll(buffer.get(), expected_length, &read, nullptr));
EXPECT_EQ(0, memcmp(expected_contents, buffer.get(), expected_length));
EXPECT_EQ(SR_EOS, stream->ReadAll(buffer.get(), 1, nullptr, nullptr));
EXPECT_EQ(stream_size, read);
// Test also with the FileRotatingStreamReader class.
FileRotatingStreamReader reader(dir_path, file_prefix);
EXPECT_EQ(reader.GetSize(), expected_length);
std::unique_ptr<uint8_t[]> buffer(new uint8_t[expected_length]);
memset(buffer.get(), 0, expected_length);
EXPECT_EQ(expected_length, reader.ReadAll(buffer.get(), expected_length));
EXPECT_EQ(0, memcmp(expected_contents, buffer.get(), expected_length));
@ -109,9 +94,12 @@ class MAYBE_FileRotatingStreamTest : public ::testing::Test {
std::unique_ptr<uint8_t[]> buffer(new uint8_t[expected_length]);
FileStream stream;
ASSERT_TRUE(stream.Open(file_path, "r", nullptr));
EXPECT_EQ(rtc::SR_SUCCESS,
stream.ReadAll(buffer.get(), expected_length, nullptr, nullptr));
EXPECT_EQ(0, memcmp(expected_contents, buffer.get(), expected_length));
size_t size_read = 0;
EXPECT_EQ(rtc::SR_SUCCESS, stream.ReadAll(buffer.get(), expected_length,
&size_read, nullptr));
EXPECT_EQ(size_read, expected_length);
EXPECT_EQ(0, memcmp(expected_contents, buffer.get(),
std::min(expected_length, size_read)));
size_t file_size = 0;
EXPECT_TRUE(stream.GetSize(&file_size));
EXPECT_EQ(file_size, expected_length);
@ -302,24 +290,9 @@ class MAYBE_CallSessionFileRotatingStreamTest : public ::testing::Test {
void VerifyStreamRead(const char* expected_contents,
const size_t expected_length,
const std::string& dir_path) {
// TODO(nisse): Delete the part of this method using read-mode
// CallSessionFileRotatingStream, together with support for read-mode.
std::unique_ptr<CallSessionFileRotatingStream> stream(
new CallSessionFileRotatingStream(dir_path));
ASSERT_TRUE(stream->Open());
size_t read = 0;
size_t stream_size = 0;
EXPECT_TRUE(stream->GetSize(&stream_size));
std::unique_ptr<uint8_t[]> buffer(new uint8_t[expected_length]);
EXPECT_EQ(SR_SUCCESS,
stream->ReadAll(buffer.get(), expected_length, &read, nullptr));
EXPECT_EQ(0, memcmp(expected_contents, buffer.get(), expected_length));
EXPECT_EQ(SR_EOS, stream->ReadAll(buffer.get(), 1, nullptr, nullptr));
EXPECT_EQ(stream_size, read);
// Test also with the CallSessionFileRotatingStreamReader class.
CallSessionFileRotatingStreamReader reader(dir_path);
EXPECT_EQ(reader.GetSize(), expected_length);
std::unique_ptr<uint8_t[]> buffer(new uint8_t[expected_length]);
memset(buffer.get(), 0, expected_length);
EXPECT_EQ(expected_length, reader.ReadAll(buffer.get(), expected_length));
EXPECT_EQ(0, memcmp(expected_contents, buffer.get(), expected_length));
@ -369,17 +342,25 @@ TEST_F(MAYBE_CallSessionFileRotatingStreamTest, WriteAndReadLarge) {
stream_->WriteAll(buffer.get(), buffer_size, nullptr, nullptr));
}
stream_.reset(new CallSessionFileRotatingStream(dir_path_));
ASSERT_TRUE(stream_->Open());
std::unique_ptr<uint8_t[]> expected_buffer(new uint8_t[buffer_size]);
int expected_vals[] = {0, 1, 2, 6, 7};
const int expected_vals[] = {0, 1, 2, 6, 7};
const size_t expected_size = buffer_size * arraysize(expected_vals);
CallSessionFileRotatingStreamReader reader(dir_path_);
EXPECT_EQ(reader.GetSize(), expected_size);
std::unique_ptr<uint8_t[]> contents(new uint8_t[expected_size + 1]);
EXPECT_EQ(reader.ReadAll(contents.get(), expected_size + 1), expected_size);
for (size_t i = 0; i < arraysize(expected_vals); ++i) {
memset(expected_buffer.get(), expected_vals[i], buffer_size);
EXPECT_EQ(SR_SUCCESS,
stream_->ReadAll(buffer.get(), buffer_size, nullptr, nullptr));
EXPECT_EQ(0, memcmp(buffer.get(), expected_buffer.get(), buffer_size));
const uint8_t* block = contents.get() + i * buffer_size;
bool match = true;
for (size_t j = 0; j < buffer_size; j++) {
if (block[j] != expected_vals[i]) {
match = false;
break;
}
}
// EXPECT call at end of loop, to limit the number of messages on failure.
EXPECT_TRUE(match);
}
EXPECT_EQ(SR_EOS, stream_->ReadAll(buffer.get(), 1, nullptr, nullptr));
}
// Tests that writing and reading to a stream where only the first file is
@ -396,17 +377,26 @@ TEST_F(MAYBE_CallSessionFileRotatingStreamTest, WriteAndReadFirstHalf) {
stream_->WriteAll(buffer.get(), buffer_size, nullptr, nullptr));
}
stream_.reset(new CallSessionFileRotatingStream(dir_path_));
ASSERT_TRUE(stream_->Open());
std::unique_ptr<uint8_t[]> expected_buffer(new uint8_t[buffer_size]);
int expected_vals[] = {0, 1};
const int expected_vals[] = {0, 1};
const size_t expected_size = buffer_size * arraysize(expected_vals);
CallSessionFileRotatingStreamReader reader(dir_path_);
EXPECT_EQ(reader.GetSize(), expected_size);
std::unique_ptr<uint8_t[]> contents(new uint8_t[expected_size + 1]);
EXPECT_EQ(reader.ReadAll(contents.get(), expected_size + 1), expected_size);
for (size_t i = 0; i < arraysize(expected_vals); ++i) {
memset(expected_buffer.get(), expected_vals[i], buffer_size);
EXPECT_EQ(SR_SUCCESS,
stream_->ReadAll(buffer.get(), buffer_size, nullptr, nullptr));
EXPECT_EQ(0, memcmp(buffer.get(), expected_buffer.get(), buffer_size));
const uint8_t* block = contents.get() + i * buffer_size;
bool match = true;
for (size_t j = 0; j < buffer_size; j++) {
if (block[j] != expected_vals[i]) {
match = false;
break;
}
}
// EXPECT call at end of loop, to limit the number of messages on failure.
EXPECT_TRUE(match);
}
EXPECT_EQ(SR_EOS, stream_->ReadAll(buffer.get(), 1, nullptr, nullptr));
}
} // namespace rtc