Re: Review Request 47209: Establish TCP connection after backing off.

[Cong Wang]

> On May 14, 2016, 2:01 a.m., Qian Zhang wrote:
> > src/slave/slave.cpp, line 1020
> > 
> >
> > If we do the link here, that means we still establish TCP connection 
> > with master right after a new master is detected, right? But I think we 
> > want to do the link after the initial backoff to avoid SYN flood.
> 
> Cong Wang wrote:
> Authentication code path doesn't have a backoff, so we don't want to 
> touch it in this patch.
> 
> Qian Zhang wrote:
> Thanks, so that means if there are a large number of agents in the 
> cluster and each agent has authentication enabled, when new master is 
> detected, it is still possible all those agents try to connect with master at 
> the same time?

If they use TCP, I think so. Also, there is already a TODO saying we need a 
backoff for authentication too.


- Cong


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


On May 13, 2016, 3:37 a.m., David Robinson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47209/
> ---
> 
> (Updated May 13, 2016, 3:37 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Ian Downes, and Cong Wang.
> 
> 
> Bugs: MESOS-5330
> https://issues.apache.org/jira/browse/MESOS-5330
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Establish TCP connection after backing off.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 116ea59b72950db4a7cd721b7ba5bfbb2e1c1454 
> 
> Diff: https://reviews.apache.org/r/47209/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Also started a master and agent and captured a tcpdump to verify the 
> connection is only established _after_ the registration backoff.
> 
> 
> Thanks,
> 
> David Robinson
> 
>

Re: Review Request 47209: Establish TCP connection after backing off.

[Qian Zhang]

> On May 14, 2016, 10:01 a.m., Qian Zhang wrote:
> > src/slave/slave.cpp, line 1020
> > 
> >
> > If we do the link here, that means we still establish TCP connection 
> > with master right after a new master is detected, right? But I think we 
> > want to do the link after the initial backoff to avoid SYN flood.
> 
> Cong Wang wrote:
> Authentication code path doesn't have a backoff, so we don't want to 
> touch it in this patch.

Thanks, so that means if there are a large number of agents in the cluster and 
each agent has authentication enabled, when new master is detected, it is still 
possible all those agents try to connect with master at the same time?


- Qian


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


On May 13, 2016, 11:37 a.m., David Robinson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47209/
> ---
> 
> (Updated May 13, 2016, 11:37 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Ian Downes, and Cong Wang.
> 
> 
> Bugs: MESOS-5330
> https://issues.apache.org/jira/browse/MESOS-5330
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Establish TCP connection after backing off.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 116ea59b72950db4a7cd721b7ba5bfbb2e1c1454 
> 
> Diff: https://reviews.apache.org/r/47209/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Also started a master and agent and captured a tcpdump to verify the 
> connection is only established _after_ the registration backoff.
> 
> 
> Thanks,
> 
> David Robinson
> 
>

Re: Review Request 47209: Establish TCP connection after backing off.

[Cong Wang]

> On May 14, 2016, 2:01 a.m., Qian Zhang wrote:
> > src/slave/slave.cpp, line 1020
> > 
> >
> > If we do the link here, that means we still establish TCP connection 
> > with master right after a new master is detected, right? But I think we 
> > want to do the link after the initial backoff to avoid SYN flood.

Authentication code path doesn't have a backoff, so we don't want to touch it 
in this patch.


- Cong


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


On May 13, 2016, 3:37 a.m., David Robinson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47209/
> ---
> 
> (Updated May 13, 2016, 3:37 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Ian Downes, and Cong Wang.
> 
> 
> Bugs: MESOS-5330
> https://issues.apache.org/jira/browse/MESOS-5330
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Establish TCP connection after backing off.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 116ea59b72950db4a7cd721b7ba5bfbb2e1c1454 
> 
> Diff: https://reviews.apache.org/r/47209/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Also started a master and agent and captured a tcpdump to verify the 
> connection is only established _after_ the registration backoff.
> 
> 
> Thanks,
> 
> David Robinson
> 
>

Re: Review Request 47209: Establish TCP connection after backing off.

[Qian Zhang]

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




src/slave/slave.cpp (line 1019)


If we do the link here, that means we still establish TCP connection with 
master right after a new master is detected, right? But I think we want to do 
the link after the initial backoff to avoid SYN flood.


- Qian Zhang


On May 13, 2016, 11:37 a.m., David Robinson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47209/
> ---
> 
> (Updated May 13, 2016, 11:37 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Ian Downes, and Cong Wang.
> 
> 
> Bugs: MESOS-5330
> https://issues.apache.org/jira/browse/MESOS-5330
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Establish TCP connection after backing off.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 116ea59b72950db4a7cd721b7ba5bfbb2e1c1454 
> 
> Diff: https://reviews.apache.org/r/47209/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Also started a master and agent and captured a tcpdump to verify the 
> connection is only established _after_ the registration backoff.
> 
> 
> Thanks,
> 
> David Robinson
> 
>

Re: Review Request 47209: Establish TCP connection after backing off.

[Ben Mahler]

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


Ship it!




Ship It!

- Ben Mahler


On May 13, 2016, 3:37 a.m., David Robinson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47209/
> ---
> 
> (Updated May 13, 2016, 3:37 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Ian Downes, and Cong Wang.
> 
> 
> Bugs: MESOS-5330
> https://issues.apache.org/jira/browse/MESOS-5330
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Establish TCP connection after backing off.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 116ea59b72950db4a7cd721b7ba5bfbb2e1c1454 
> 
> Diff: https://reviews.apache.org/r/47209/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Also started a master and agent and captured a tcpdump to verify the 
> connection is only established _after_ the registration backoff.
> 
> 
> Thanks,
> 
> David Robinson
> 
>

Re: Review Request 47209: Establish TCP connection after backing off.

[David Robinson]

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

(Updated May 13, 2016, 3:37 a.m.)


Review request for mesos, Ben Mahler, Ian Downes, and Cong Wang.


Bugs: MESOS-5330
https://issues.apache.org/jira/browse/MESOS-5330


Repository: mesos


Description
---

Establish TCP connection after backing off.


Diffs (updated)
-

  src/slave/slave.cpp 116ea59b72950db4a7cd721b7ba5bfbb2e1c1454 

Diff: https://reviews.apache.org/r/47209/diff/


Testing
---

make check

Also started a master and agent and captured a tcpdump to verify the connection 
is only established _after_ the registration backoff.


Thanks,

David Robinson

Re: Review Request 47209: Establish TCP connection after backing off.

[Ben Mahler]

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




src/slave/slave.cpp (line 996)


Whoops, this will crash when master is None. How about you move it down to 
where we've decided we need to authenticate (below the LOG(INFO)). This makes 
it a bit more symmetric with the registration path).

Also could you include the following?

```
// Ensure there is a link to the master before we start communicating with 
it.
```


- Ben Mahler


On May 12, 2016, 6:18 p.m., David Robinson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47209/
> ---
> 
> (Updated May 12, 2016, 6:18 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Ian Downes, and Cong Wang.
> 
> 
> Bugs: MESOS-5330
> https://issues.apache.org/jira/browse/MESOS-5330
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Establish TCP connection after backing off.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 116ea59b72950db4a7cd721b7ba5bfbb2e1c1454 
> 
> Diff: https://reviews.apache.org/r/47209/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Also started a master and agent and captured a tcpdump to verify the 
> connection is only established _after_ the registration backoff.
> 
> 
> Thanks,
> 
> David Robinson
> 
>

Re: Review Request 47209: Establish TCP connection after backing off.

[David Robinson]

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

(Updated May 12, 2016, 6:18 p.m.)


Review request for mesos, Ben Mahler, Ian Downes, and Cong Wang.


Bugs: MESOS-5330
https://issues.apache.org/jira/browse/MESOS-5330


Repository: mesos


Description
---

Establish TCP connection after backing off.


Diffs (updated)
-

  src/slave/slave.cpp 116ea59b72950db4a7cd721b7ba5bfbb2e1c1454 

Diff: https://reviews.apache.org/r/47209/diff/


Testing
---

make check

Also started a master and agent and captured a tcpdump to verify the connection 
is only established _after_ the registration backoff.


Thanks,

David Robinson

Re: Review Request 47209: Establish TCP connection after backing off.

[Ben Mahler]

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




src/slave/slave.cpp (line 966)


Why not call `link` inside the `authenticate()` function in a similar 
manner to how you've called it within `doReliableRegistration()`?



src/slave/slave.cpp (line 1361)


How about the following comment here:

```
  // Ensure there is a link to the master before we start
  // communicating with it. We want to `link` after the
  // initial registration backoff in order to avoid all
  // of the agents establishing connections with the
  // master at once. See MESOS-5330.
```


- Ben Mahler


On May 11, 2016, 12:58 a.m., David Robinson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47209/
> ---
> 
> (Updated May 11, 2016, 12:58 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Ian Downes, and Cong Wang.
> 
> 
> Bugs: MESOS-5330
> https://issues.apache.org/jira/browse/MESOS-5330
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Establish TCP connection after backing off.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 116ea59b72950db4a7cd721b7ba5bfbb2e1c1454 
> 
> Diff: https://reviews.apache.org/r/47209/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Also started a master and agent and captured a tcpdump to verify the 
> connection is only established _after_ the registration backoff.
> 
> 
> Thanks,
> 
> David Robinson
> 
>

Re: Review Request 47209: Establish TCP connection after backing off.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [47209]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On May 11, 2016, 12:58 a.m., David Robinson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47209/
> ---
> 
> (Updated May 11, 2016, 12:58 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Ian Downes, and Cong Wang.
> 
> 
> Bugs: MESOS-5330
> https://issues.apache.org/jira/browse/MESOS-5330
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Establish TCP connection after backing off.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 116ea59b72950db4a7cd721b7ba5bfbb2e1c1454 
> 
> Diff: https://reviews.apache.org/r/47209/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Also started a master and agent and captured a tcpdump to verify the 
> connection is only established _after_ the registration backoff.
> 
> 
> Thanks,
> 
> David Robinson
> 
>

Re: Review Request 47209: Establish TCP connection after backing off.

[Cong Wang]

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


Ship it!




Ship It!

- Cong Wang


On May 11, 2016, 12:58 a.m., David Robinson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47209/
> ---
> 
> (Updated May 11, 2016, 12:58 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Ian Downes, and Cong Wang.
> 
> 
> Bugs: MESOS-5330
> https://issues.apache.org/jira/browse/MESOS-5330
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Establish TCP connection after backing off.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 116ea59b72950db4a7cd721b7ba5bfbb2e1c1454 
> 
> Diff: https://reviews.apache.org/r/47209/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Also started a master and agent and captured a tcpdump to verify the 
> connection is only established _after_ the registration backoff.
> 
> 
> Thanks,
> 
> David Robinson
> 
>

Review Request 47209: Establish TCP connection after backing off.

[David Robinson]

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

Review request for mesos, Ben Mahler, Ian Downes, and Cong Wang.


Bugs: MESOS-5330
https://issues.apache.org/jira/browse/MESOS-5330


Repository: mesos


Description
---

Establish TCP connection after backing off.


Diffs
-

  src/slave/slave.cpp 116ea59b72950db4a7cd721b7ba5bfbb2e1c1454 

Diff: https://reviews.apache.org/r/47209/diff/


Testing
---

make check

Also started a master and agent and captured a tcpdump to verify the connection 
is only established _after_ the registration backoff.


Thanks,

David Robinson