Re: Review Request 28218: Add more logging into replicated log recovery.

[Jie Yu]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28218/#review65412
-----------------------------------------------------------



src/log/log.cpp
<https://reviews.apache.org/r/28218/#comment108524>

    This log is missleading. It's LogProcess recovery fails (not reader 
process).
    
    Can you instead add this logging in LogProcess? (i.e., in 
```LogProcess::_recover()```)



src/log/log.cpp
<https://reviews.apache.org/r/28218/#comment108527>

    Ditto. Do the logging in LogProcess.



src/log/log.cpp
<https://reviews.apache.org/r/28218/#comment108528>

    Add a blank line after VLOG. HEre and everywhere else.
    
    Can you instead add this logging in Replica process because essentially 
this is calling replica->read(from, to).



src/log/log.cpp
<https://reviews.apache.org/r/28218/#comment108529>

    I don't think this is necessary. Mind killing it?



src/log/recover.cpp
<https://reviews.apache.org/r/28218/#comment108530>

    Can you move this log line into 'finished'?



src/state/log.cpp
<https://reviews.apache.org/r/28218/#comment108532>

    Remove the 'process' suffix?



src/state/log.cpp
<https://reviews.apache.org/r/28218/#comment108533>

    Looks like this is uncesseary given that all the subsequent code path has a 
VLOG.



src/state/log.cpp
<https://reviews.apache.org/r/28218/#comment108535>

    This is not precise.
    
    ```
    VLOG(2) << "Writer failed to get elected, retrying";
    ```



src/state/log.cpp
<https://reviews.apache.org/r/28218/#comment108536>

    VLOG(2) << "Writer got elected at position " << position.get() << ", index 
is " << index.get();



src/state/log.cpp
<https://reviews.apache.org/r/28218/#comment108541>

    Kill this.



src/state/log.cpp
<https://reviews.apache.org/r/28218/#comment108542>

    Kill this.



src/state/log.cpp
<https://reviews.apache.org/r/28218/#comment108545>

    "Applying Operations (" << entries.size() << " entries)"



src/state/log.cpp
<https://reviews.apache.org/r/28218/#comment108546>

    Kill this one.


- Jie Yu


On Nov. 19, 2014, 9:58 p.m., Timothy Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28218/
> -----------------------------------------------------------
> 
> (Updated Nov. 19, 2014, 9:58 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Jie Yu, and Niklas Nielsen.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> There has a quite few cases where we see a timed out fetch when master is 
> first launched, and I think there isn't enough logging to shed some lights 
> besides the failed to fetch after x minutes log line.
> This rb adds more logging into replicated log recovery.
> Not meant for merging yet, posting here for reviews and discuss places we 
> like to add more logging.
> Review: https://reviews.apache.org/r/28218
> 
> 
> Diffs
> -----
> 
>   src/log/log.cpp b3c6c20d734ea82d8531217d53decf1707183565 
>   src/log/recover.cpp 26140654fda5011e89d0081e916a88c9681cd783 
>   src/state/log.cpp c0c81d551f465534daeaf03f18c0baa4c19754e6 
> 
> Diff: https://reviews.apache.org/r/28218/diff/
> 
> 
> Testing
> -------
> 
> make
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>