You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Neil Conway <ne...@gmail.com> on 2015/10/07 23:41:48 UTC
Review Request 39104: Added source address to logging when we receive
replicated log events.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39104/
-----------------------------------------------------------
Review request for mesos, Adam B and Joris Van Remoortere.
Bugs: MESOS-3417
https://issues.apache.org/jira/browse/MESOS-3417
Repository: mesos
Description
-------
MESOS-3417.
Diffs
-----
src/log/replica.cpp 2bfcc3eccb0dc019dd0dfe6027a3560c6fdbcff0
Diff: https://reviews.apache.org/r/39104/diff/
Testing
-------
"make check", ran mesos-master by hand. Example output:
I1007 21:33:05.434449 27039 replica.cpp:478] Replica received implicit promise request from (4)@10.0.2.15:5050 with proposal 5
[...]
I1007 21:33:05.444855 27042 replica.cpp:512] Replica received write request for position 17 from (5)@10.0.2.15:5050
[...]
I1007 21:33:05.446867 27039 replica.cpp:660] Replica received learned notice for position 17 from @0.0.0.0:0
[...]
I1007 21:33:05.451514 27042 replica.cpp:512] Replica received write request for position 18 from (6)@10.0.2.15:5050
[...]
I1007 21:33:05.452555 27042 replica.cpp:660] Replica received learned notice for position 18 from @0.0.0.0:0
Thanks,
Neil Conway
Re: Review Request 39104: Added source address to logging when we
receive replicated log events.
Posted by Anand Mazumdar <ma...@gmail.com>.
> On Oct. 7, 2015, 10:06 p.m., Anand Mazumdar wrote:
> > src/log/replica.cpp, line 378
> > <https://reviews.apache.org/r/39104/diff/1/?file=1092535#file1092535line378>
> >
> > Nit : This line is > 80 chars. Can we wrap this ?
>
> Neil Conway wrote:
> That line is 74 characters long, by my count :)
Dropped. My bad. Mistaken vimrc settings :)
- Anand
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39104/#review101822
-----------------------------------------------------------
On Oct. 7, 2015, 9:41 p.m., Neil Conway wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39104/
> -----------------------------------------------------------
>
> (Updated Oct. 7, 2015, 9:41 p.m.)
>
>
> Review request for mesos, Adam B and Joris Van Remoortere.
>
>
> Bugs: MESOS-3417
> https://issues.apache.org/jira/browse/MESOS-3417
>
>
> Repository: mesos
>
>
> Description
> -------
>
> MESOS-3417.
>
>
> Diffs
> -----
>
> src/log/replica.cpp 2bfcc3eccb0dc019dd0dfe6027a3560c6fdbcff0
>
> Diff: https://reviews.apache.org/r/39104/diff/
>
>
> Testing
> -------
>
> "make check", ran mesos-master by hand. Example output:
>
> I1007 21:33:05.434449 27039 replica.cpp:478] Replica received implicit promise request from (4)@10.0.2.15:5050 with proposal 5
> [...]
> I1007 21:33:05.444855 27042 replica.cpp:512] Replica received write request for position 17 from (5)@10.0.2.15:5050
> [...]
> I1007 21:33:05.446867 27039 replica.cpp:660] Replica received learned notice for position 17 from @0.0.0.0:0
> [...]
> I1007 21:33:05.451514 27042 replica.cpp:512] Replica received write request for position 18 from (6)@10.0.2.15:5050
> [...]
> I1007 21:33:05.452555 27042 replica.cpp:660] Replica received learned notice for position 18 from @0.0.0.0:0
>
>
> Thanks,
>
> Neil Conway
>
>
Re: Review Request 39104: Added source address to logging when we
receive replicated log events.
Posted by Neil Conway <ne...@gmail.com>.
> On Oct. 7, 2015, 10:06 p.m., Anand Mazumdar wrote:
> > src/log/replica.cpp, line 378
> > <https://reviews.apache.org/r/39104/diff/1/?file=1092535#file1092535line378>
> >
> > Nit : This line is > 80 chars. Can we wrap this ?
That line is 74 characters long, by my count :)
- Neil
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39104/#review101822
-----------------------------------------------------------
On Oct. 7, 2015, 9:41 p.m., Neil Conway wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39104/
> -----------------------------------------------------------
>
> (Updated Oct. 7, 2015, 9:41 p.m.)
>
>
> Review request for mesos, Adam B and Joris Van Remoortere.
>
>
> Bugs: MESOS-3417
> https://issues.apache.org/jira/browse/MESOS-3417
>
>
> Repository: mesos
>
>
> Description
> -------
>
> MESOS-3417.
>
>
> Diffs
> -----
>
> src/log/replica.cpp 2bfcc3eccb0dc019dd0dfe6027a3560c6fdbcff0
>
> Diff: https://reviews.apache.org/r/39104/diff/
>
>
> Testing
> -------
>
> "make check", ran mesos-master by hand. Example output:
>
> I1007 21:33:05.434449 27039 replica.cpp:478] Replica received implicit promise request from (4)@10.0.2.15:5050 with proposal 5
> [...]
> I1007 21:33:05.444855 27042 replica.cpp:512] Replica received write request for position 17 from (5)@10.0.2.15:5050
> [...]
> I1007 21:33:05.446867 27039 replica.cpp:660] Replica received learned notice for position 17 from @0.0.0.0:0
> [...]
> I1007 21:33:05.451514 27042 replica.cpp:512] Replica received write request for position 18 from (6)@10.0.2.15:5050
> [...]
> I1007 21:33:05.452555 27042 replica.cpp:660] Replica received learned notice for position 18 from @0.0.0.0:0
>
>
> Thanks,
>
> Neil Conway
>
>
Re: Review Request 39104: Added source address to logging when we
receive replicated log events.
Posted by Anand Mazumdar <ma...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39104/#review101822
-----------------------------------------------------------
Ship it!
LGTM
src/log/replica.cpp (line 378)
<https://reviews.apache.org/r/39104/#comment159319>
Nit : This line is > 80 chars. Can we wrap this ?
src/log/replica.cpp (line 642)
<https://reviews.apache.org/r/39104/#comment159320>
Nit: Not yours, but can we reduce jaggedness here i.e. two lines should suffice ?
- Anand Mazumdar
On Oct. 7, 2015, 9:41 p.m., Neil Conway wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39104/
> -----------------------------------------------------------
>
> (Updated Oct. 7, 2015, 9:41 p.m.)
>
>
> Review request for mesos, Adam B and Joris Van Remoortere.
>
>
> Bugs: MESOS-3417
> https://issues.apache.org/jira/browse/MESOS-3417
>
>
> Repository: mesos
>
>
> Description
> -------
>
> MESOS-3417.
>
>
> Diffs
> -----
>
> src/log/replica.cpp 2bfcc3eccb0dc019dd0dfe6027a3560c6fdbcff0
>
> Diff: https://reviews.apache.org/r/39104/diff/
>
>
> Testing
> -------
>
> "make check", ran mesos-master by hand. Example output:
>
> I1007 21:33:05.434449 27039 replica.cpp:478] Replica received implicit promise request from (4)@10.0.2.15:5050 with proposal 5
> [...]
> I1007 21:33:05.444855 27042 replica.cpp:512] Replica received write request for position 17 from (5)@10.0.2.15:5050
> [...]
> I1007 21:33:05.446867 27039 replica.cpp:660] Replica received learned notice for position 17 from @0.0.0.0:0
> [...]
> I1007 21:33:05.451514 27042 replica.cpp:512] Replica received write request for position 18 from (6)@10.0.2.15:5050
> [...]
> I1007 21:33:05.452555 27042 replica.cpp:660] Replica received learned notice for position 18 from @0.0.0.0:0
>
>
> Thanks,
>
> Neil Conway
>
>
Re: Review Request 39104: Added source address to logging when we
receive replicated log events.
Posted by Neil Conway <ne...@gmail.com>.
> On Oct. 7, 2015, 9:52 p.m., Adam B wrote:
> > LGTM.
> > Since the 'Summary' and 'Description fields are used to create the git commit message, I'd like to see the 'Description' be more descriptive; or just say "See summary", which means "do not include in commit message".
Okay. Just to make sure I understand, I was proposing a Git commit message with two lines:
```
Added source address to logging when we receive replicated log events.
MESOS-3417
```
i.e., I think it is important to link to the Jira for context, but otherwise that seemed sufficient as a Git message. lmk if I should do that differently...
- Neil
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39104/#review101817
-----------------------------------------------------------
On Oct. 7, 2015, 9:41 p.m., Neil Conway wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39104/
> -----------------------------------------------------------
>
> (Updated Oct. 7, 2015, 9:41 p.m.)
>
>
> Review request for mesos, Adam B and Joris Van Remoortere.
>
>
> Bugs: MESOS-3417
> https://issues.apache.org/jira/browse/MESOS-3417
>
>
> Repository: mesos
>
>
> Description
> -------
>
> MESOS-3417.
>
>
> Diffs
> -----
>
> src/log/replica.cpp 2bfcc3eccb0dc019dd0dfe6027a3560c6fdbcff0
>
> Diff: https://reviews.apache.org/r/39104/diff/
>
>
> Testing
> -------
>
> "make check", ran mesos-master by hand. Example output:
>
> I1007 21:33:05.434449 27039 replica.cpp:478] Replica received implicit promise request from (4)@10.0.2.15:5050 with proposal 5
> [...]
> I1007 21:33:05.444855 27042 replica.cpp:512] Replica received write request for position 17 from (5)@10.0.2.15:5050
> [...]
> I1007 21:33:05.446867 27039 replica.cpp:660] Replica received learned notice for position 17 from @0.0.0.0:0
> [...]
> I1007 21:33:05.451514 27042 replica.cpp:512] Replica received write request for position 18 from (6)@10.0.2.15:5050
> [...]
> I1007 21:33:05.452555 27042 replica.cpp:660] Replica received learned notice for position 18 from @0.0.0.0:0
>
>
> Thanks,
>
> Neil Conway
>
>
Re: Review Request 39104: Added source address to logging when we
receive replicated log events.
Posted by Adam B <ad...@mesosphere.io>.
> On Oct. 7, 2015, 2:52 p.m., Adam B wrote:
> > LGTM.
> > Since the 'Summary' and 'Description fields are used to create the git commit message, I'd like to see the 'Description' be more descriptive; or just say "See summary", which means "do not include in commit message".
>
> Neil Conway wrote:
> Okay. Just to make sure I understand, I was proposing a Git commit message with two lines:
>
> ```
> Added source address to logging when we receive replicated log events.
>
> MESOS-3417
> ```
>
> i.e., I think it is important to link to the Jira for context, but otherwise that seemed sufficient as a Git message. lmk if I should do that differently...
Yeah, that's ok, but I don't like such short second lines. Better would be:
`MESOS-3417: Added source address to logging when we receive replicated log events.`
or
`Added source address to logging when we receive replicated log events.`
`Fixes MESOS-3417: Log source address replicated log recieved broadcasts`
But that's just a personal nit. I'll commit it either way.
- Adam
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39104/#review101817
-----------------------------------------------------------
On Oct. 7, 2015, 2:41 p.m., Neil Conway wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39104/
> -----------------------------------------------------------
>
> (Updated Oct. 7, 2015, 2:41 p.m.)
>
>
> Review request for mesos, Adam B and Joris Van Remoortere.
>
>
> Bugs: MESOS-3417
> https://issues.apache.org/jira/browse/MESOS-3417
>
>
> Repository: mesos
>
>
> Description
> -------
>
> MESOS-3417.
>
>
> Diffs
> -----
>
> src/log/replica.cpp 2bfcc3eccb0dc019dd0dfe6027a3560c6fdbcff0
>
> Diff: https://reviews.apache.org/r/39104/diff/
>
>
> Testing
> -------
>
> "make check", ran mesos-master by hand. Example output:
>
> I1007 21:33:05.434449 27039 replica.cpp:478] Replica received implicit promise request from (4)@10.0.2.15:5050 with proposal 5
> [...]
> I1007 21:33:05.444855 27042 replica.cpp:512] Replica received write request for position 17 from (5)@10.0.2.15:5050
> [...]
> I1007 21:33:05.446867 27039 replica.cpp:660] Replica received learned notice for position 17 from @0.0.0.0:0
> [...]
> I1007 21:33:05.451514 27042 replica.cpp:512] Replica received write request for position 18 from (6)@10.0.2.15:5050
> [...]
> I1007 21:33:05.452555 27042 replica.cpp:660] Replica received learned notice for position 18 from @0.0.0.0:0
>
>
> Thanks,
>
> Neil Conway
>
>
Re: Review Request 39104: Added source address to logging when we
receive replicated log events.
Posted by Adam B <ad...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39104/#review101817
-----------------------------------------------------------
Ship it!
LGTM.
Since the 'Summary' and 'Description fields are used to create the git commit message, I'd like to see the 'Description' be more descriptive; or just say "See summary", which means "do not include in commit message".
- Adam B
On Oct. 7, 2015, 2:41 p.m., Neil Conway wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39104/
> -----------------------------------------------------------
>
> (Updated Oct. 7, 2015, 2:41 p.m.)
>
>
> Review request for mesos, Adam B and Joris Van Remoortere.
>
>
> Bugs: MESOS-3417
> https://issues.apache.org/jira/browse/MESOS-3417
>
>
> Repository: mesos
>
>
> Description
> -------
>
> MESOS-3417.
>
>
> Diffs
> -----
>
> src/log/replica.cpp 2bfcc3eccb0dc019dd0dfe6027a3560c6fdbcff0
>
> Diff: https://reviews.apache.org/r/39104/diff/
>
>
> Testing
> -------
>
> "make check", ran mesos-master by hand. Example output:
>
> I1007 21:33:05.434449 27039 replica.cpp:478] Replica received implicit promise request from (4)@10.0.2.15:5050 with proposal 5
> [...]
> I1007 21:33:05.444855 27042 replica.cpp:512] Replica received write request for position 17 from (5)@10.0.2.15:5050
> [...]
> I1007 21:33:05.446867 27039 replica.cpp:660] Replica received learned notice for position 17 from @0.0.0.0:0
> [...]
> I1007 21:33:05.451514 27042 replica.cpp:512] Replica received write request for position 18 from (6)@10.0.2.15:5050
> [...]
> I1007 21:33:05.452555 27042 replica.cpp:660] Replica received learned notice for position 18 from @0.0.0.0:0
>
>
> Thanks,
>
> Neil Conway
>
>
Re: Review Request 39104: Added source address to logging when we
receive replicated log events.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39104/#review101849
-----------------------------------------------------------
Patch looks great!
Reviews applied: [39104]
All tests passed.
- Mesos ReviewBot
On Oct. 7, 2015, 9:41 p.m., Neil Conway wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39104/
> -----------------------------------------------------------
>
> (Updated Oct. 7, 2015, 9:41 p.m.)
>
>
> Review request for mesos, Adam B and Joris Van Remoortere.
>
>
> Bugs: MESOS-3417
> https://issues.apache.org/jira/browse/MESOS-3417
>
>
> Repository: mesos
>
>
> Description
> -------
>
> MESOS-3417.
>
>
> Diffs
> -----
>
> src/log/replica.cpp 2bfcc3eccb0dc019dd0dfe6027a3560c6fdbcff0
>
> Diff: https://reviews.apache.org/r/39104/diff/
>
>
> Testing
> -------
>
> "make check", ran mesos-master by hand. Example output:
>
> I1007 21:33:05.434449 27039 replica.cpp:478] Replica received implicit promise request from (4)@10.0.2.15:5050 with proposal 5
> [...]
> I1007 21:33:05.444855 27042 replica.cpp:512] Replica received write request for position 17 from (5)@10.0.2.15:5050
> [...]
> I1007 21:33:05.446867 27039 replica.cpp:660] Replica received learned notice for position 17 from @0.0.0.0:0
> [...]
> I1007 21:33:05.451514 27042 replica.cpp:512] Replica received write request for position 18 from (6)@10.0.2.15:5050
> [...]
> I1007 21:33:05.452555 27042 replica.cpp:660] Replica received learned notice for position 18 from @0.0.0.0:0
>
>
> Thanks,
>
> Neil Conway
>
>
Re: Review Request 39104: Added source address to logging when we
receive replicated log events.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39104/#review101818
-----------------------------------------------------------
Ship it!
Ship It!
- Jie Yu
On Oct. 7, 2015, 9:41 p.m., Neil Conway wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39104/
> -----------------------------------------------------------
>
> (Updated Oct. 7, 2015, 9:41 p.m.)
>
>
> Review request for mesos, Adam B and Joris Van Remoortere.
>
>
> Bugs: MESOS-3417
> https://issues.apache.org/jira/browse/MESOS-3417
>
>
> Repository: mesos
>
>
> Description
> -------
>
> MESOS-3417.
>
>
> Diffs
> -----
>
> src/log/replica.cpp 2bfcc3eccb0dc019dd0dfe6027a3560c6fdbcff0
>
> Diff: https://reviews.apache.org/r/39104/diff/
>
>
> Testing
> -------
>
> "make check", ran mesos-master by hand. Example output:
>
> I1007 21:33:05.434449 27039 replica.cpp:478] Replica received implicit promise request from (4)@10.0.2.15:5050 with proposal 5
> [...]
> I1007 21:33:05.444855 27042 replica.cpp:512] Replica received write request for position 17 from (5)@10.0.2.15:5050
> [...]
> I1007 21:33:05.446867 27039 replica.cpp:660] Replica received learned notice for position 17 from @0.0.0.0:0
> [...]
> I1007 21:33:05.451514 27042 replica.cpp:512] Replica received write request for position 18 from (6)@10.0.2.15:5050
> [...]
> I1007 21:33:05.452555 27042 replica.cpp:660] Replica received learned notice for position 18 from @0.0.0.0:0
>
>
> Thanks,
>
> Neil Conway
>
>