logd: simplify Clear() + Prune() logic
Clear() and Prune() return a boolean indicating whether or not their
operations failed because the log buffer was 'busy'. This means that
they return false upon success and true upon failure, which is not
intuitive.
This change inverts their return value to simply be true if they were
successful or false otherwise. It also simplifies the return value of
ChattyLogBuffer::Prune() to true if the requested number of rows have
been pruned or if all rows in the log buffer have been pruned, and
otherwise return false.
Test: logging unit tests
Test: clearing works even under logging pressure
Change-Id: I346bb945496ef62bf8e973298f81c5163f49bc57
diff --git a/logd/SimpleLogBuffer.cpp b/logd/SimpleLogBuffer.cpp
index 292a7e4..ec08d54 100644
--- a/logd/SimpleLogBuffer.cpp
+++ b/logd/SimpleLogBuffer.cpp
@@ -212,46 +212,38 @@
return true;
}
-// clear all rows of type "id" from the buffer.
bool SimpleLogBuffer::Clear(log_id_t id, uid_t uid) {
- bool busy = true;
- // If it takes more than 4 tries (seconds) to clear, then kill reader(s)
- for (int retry = 4;;) {
- if (retry == 1) { // last pass
- // Check if it is still busy after the sleep, we say prune
- // one entry, not another clear run, so we are looking for
- // the quick side effect of the return value to tell us if
- // we have a _blocked_ reader.
- {
- auto lock = std::lock_guard{lock_};
- busy = Prune(id, 1, uid);
- }
- // It is still busy, blocked reader(s), lets kill them all!
- // otherwise, lets be a good citizen and preserve the slow
- // readers and let the clear run (below) deal with determining
- // if we are still blocked and return an error code to caller.
- if (busy) {
- auto reader_threads_lock = std::lock_guard{reader_list_->reader_threads_lock()};
- for (const auto& reader_thread : reader_list_->reader_threads()) {
- if (reader_thread->IsWatching(id)) {
- LOG(WARNING) << "Kicking blocked reader, " << reader_thread->name()
- << ", from LogBuffer::clear()";
- reader_thread->release_Locked();
- }
- }
- }
- }
+ // Try three times to clear, then disconnect the readers and try one final time.
+ for (int retry = 0; retry < 3; ++retry) {
{
auto lock = std::lock_guard{lock_};
- busy = Prune(id, ULONG_MAX, uid);
+ if (Prune(id, ULONG_MAX, uid)) {
+ return true;
+ }
}
-
- if (!busy || !--retry) {
- break;
- }
- sleep(1); // Let reader(s) catch up after notification
+ sleep(1);
}
- return busy;
+ // Check if it is still busy after the sleep, we try to prune one entry, not another clear run,
+ // so we are looking for the quick side effect of the return value to tell us if we have a
+ // _blocked_ reader.
+ bool busy = false;
+ {
+ auto lock = std::lock_guard{lock_};
+ busy = !Prune(id, 1, uid);
+ }
+ // It is still busy, disconnect all readers.
+ if (busy) {
+ auto reader_threads_lock = std::lock_guard{reader_list_->reader_threads_lock()};
+ for (const auto& reader_thread : reader_list_->reader_threads()) {
+ if (reader_thread->IsWatching(id)) {
+ LOG(WARNING) << "Kicking blocked reader, " << reader_thread->name()
+ << ", from LogBuffer::clear()";
+ reader_thread->release_Locked();
+ }
+ }
+ }
+ auto lock = std::lock_guard{lock_};
+ return Prune(id, ULONG_MAX, uid);
}
// get the total space allocated to "id"
@@ -313,16 +305,16 @@
if (oldest && oldest->start() <= element.sequence()) {
KickReader(oldest, id, prune_rows);
- return true;
+ return false;
}
stats_->Subtract(element.ToLogStatisticsElement());
it = Erase(it);
if (--prune_rows == 0) {
- return false;
+ return true;
}
}
- return false;
+ return true;
}
std::list<LogBufferElement>::iterator SimpleLogBuffer::Erase(