SingleLogSource handles rotation of log files

SingleLogSource handles rotation of log files

By observing underlying file inode values, SingleLogSource can tell
when a log file has been rotated (renamed and then replaced). When this
state is detected, SingleLogSource will read the remainder of the old
log file and then start reading from the new log file.

BUG=731906
TEST=unit tests pass

Review-Url: https://codereview.chromium.org/2940493002
Cr-Commit-Position: refs/heads/master@{#479194}
Committed: https://chromium.googlesource.com/chromium/src/+/bc3c128e15e218688cd067a17e5966fce4364ec1

[Simon Que, 2017-06-12 21:55:11]

sque@chromium.org changed reviewers:
+ afakhry@chromium.org

[afakhry, 2017-06-13 17:39:21]

lgtm with nits.

https://codereview.chromium.org/2940493002/diff/1/chrome/browser/chromeos/sys...
File chrome/browser/chromeos/system_logs/single_log_source.cc (right):

https://codereview.chromium.org/2940493002/diff/1/chrome/browser/chromeos/sys...
chrome/browser/chromeos/system_logs/single_log_source.cc:17: const int
kMaxNumAllowedLogRotationsDuringFileRead = 3;
Nit: constexpr for both.

https://codereview.chromium.org/2940493002/diff/1/chrome/browser/chromeos/sys...
chrome/browser/chromeos/system_logs/single_log_source.cc:113: file_was_rotated =
true;
Nit: replace all the above with:

const bool file_was_rotated = 
    file_inode_ != GetInodeValue(GetLogFilePath());

https://codereview.chromium.org/2940493002/diff/1/chrome/browser/chromeos/sys...
chrome/browser/chromeos/system_logs/single_log_source.cc:115: bool
handle_file_rotation = file_was_rotated && num_rotations_allowed > 0;
Nit: const bool should_handle_file_rotation = ...

https://codereview.chromium.org/2940493002/diff/1/chrome/browser/chromeos/sys...
File chrome/browser/chromeos/system_logs/single_log_source_unittest.cc (right):

https://codereview.chromium.org/2940493002/diff/1/chrome/browser/chromeos/sys...
chrome/browser/chromeos/system_logs/single_log_source_unittest.cc:330:
EXPECT_EQ("Also no newline here...yet\n", latest_response());
Nice test! Thanks!

[Simon Que, 2017-06-13 20:54:56]

https://codereview.chromium.org/2940493002/diff/1/chrome/browser/chromeos/sys...
File chrome/browser/chromeos/system_logs/single_log_source.cc (right):

https://codereview.chromium.org/2940493002/diff/1/chrome/browser/chromeos/sys...
chrome/browser/chromeos/system_logs/single_log_source.cc:17: const int
kMaxNumAllowedLogRotationsDuringFileRead = 3;
On 2017/06/13 17:39:21, afakhry wrote:
> Nit: constexpr for both.

Done.

https://codereview.chromium.org/2940493002/diff/1/chrome/browser/chromeos/sys...
chrome/browser/chromeos/system_logs/single_log_source.cc:113: file_was_rotated =
true;
On 2017/06/13 17:39:21, afakhry wrote:
> Nit: replace all the above with:
> 
> const bool file_was_rotated = 
>     file_inode_ != GetInodeValue(GetLogFilePath());

Done.

https://codereview.chromium.org/2940493002/diff/1/chrome/browser/chromeos/sys...
chrome/browser/chromeos/system_logs/single_log_source.cc:115: bool
handle_file_rotation = file_was_rotated && num_rotations_allowed > 0;
On 2017/06/13 17:39:21, afakhry wrote:
> Nit: const bool should_handle_file_rotation = ...

Done.

[Simon Que, 2017-06-13 20:54:57]

https://codereview.chromium.org/2940493002/diff/1/chrome/browser/chromeos/sys...
File chrome/browser/chromeos/system_logs/single_log_source.cc (right):

https://codereview.chromium.org/2940493002/diff/1/chrome/browser/chromeos/sys...
chrome/browser/chromeos/system_logs/single_log_source.cc:17: const int
kMaxNumAllowedLogRotationsDuringFileRead = 3;
On 2017/06/13 17:39:21, afakhry wrote:
> Nit: constexpr for both.

Done.

https://codereview.chromium.org/2940493002/diff/1/chrome/browser/chromeos/sys...
chrome/browser/chromeos/system_logs/single_log_source.cc:113: file_was_rotated =
true;
On 2017/06/13 17:39:21, afakhry wrote:
> Nit: replace all the above with:
> 
> const bool file_was_rotated = 
>     file_inode_ != GetInodeValue(GetLogFilePath());

Done.

https://codereview.chromium.org/2940493002/diff/1/chrome/browser/chromeos/sys...
chrome/browser/chromeos/system_logs/single_log_source.cc:115: bool
handle_file_rotation = file_was_rotated && num_rotations_allowed > 0;
On 2017/06/13 17:39:21, afakhry wrote:
> Nit: const bool should_handle_file_rotation = ...

Done.

[Simon Que, 2017-06-13 22:20:10]

The CQ bit was checked by sque@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-13 22:20:39]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-13 23:00:11]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-13 23:00:12]

Dry run: This issue passed the CQ dry run.

[Simon Que, 2017-06-13 23:00:26]

The CQ bit was checked by sque@chromium.org

[Simon Que, 2017-06-13 23:00:27]

The patchset sent to the CQ was uploaded after l-g-t-m from afakhry@chromium.org
Link to the patchset: https://codereview.chromium.org/2940493002/#ps20001
(title: "Use constexpr; Rename bool; Simplify logic")

[commit-bot: I haz the power, 2017-06-13 23:00:53]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-13 23:07:27]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1497394826944710,
"parent_rev": "cc66e92a5fcbb8810000303fc1d93094561479a5", "commit_rev":
"bc3c128e15e218688cd067a17e5966fce4364ec1"}

[commit-bot: I haz the power, 2017-06-13 23:07:43]

Description was changed from

==========
SingleLogSource handles rotation of log files

By observing underlying file inode values, SingleLogSource can tell
when a log file has been rotated (renamed and then replaced). When this
state is detected, SingleLogSource will read the remainder of the old
log file and then start reading from the new log file.

BUG=731906
TEST=unit tests pass
==========

to

==========
SingleLogSource handles rotation of log files

By observing underlying file inode values, SingleLogSource can tell
when a log file has been rotated (renamed and then replaced). When this
state is detected, SingleLogSource will read the remainder of the old
log file and then start reading from the new log file.

BUG=731906
TEST=unit tests pass

Review-Url: https://codereview.chromium.org/2940493002
Cr-Commit-Position: refs/heads/master@{#479194}
Committed:
https://chromium.googlesource.com/chromium/src/+/bc3c128e15e218688cd067a17e59...
==========

[commit-bot: I haz the power, 2017-06-13 23:07:44]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/bc3c128e15e218688cd067a17e59...