Re: Review Request 48387: Delay before initiating a connection with master.

[Mesos ReviewBot]

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



Bad review!

Reviews applied: []

Error:
No reviewers specified. Please find a reviewer by asking on JIRA or the mailing 
list.

- Mesos ReviewBot


On June 8, 2016, 1:21 a.m., Jose Guilherme Vanz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48387/
> ---
> 
> (Updated June 8, 2016, 1:21 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The scheduler library has been updated to wait a little deley before
> initiate a connection with the master. The maximum amount of time waited
> by the scheduler is defined by a flag: CONNECTION_BACKOFF_MAX. After
> the master has been detected the scheduler picks a random delay that
> can be between 0 and the CONNECTION_BACKOFF_MAX value. MESOS-5359
> 
> 
> Diffs
> -
> 
>   src/scheduler/constants.hpp PRE-CREATION 
>   src/scheduler/flags.hpp PRE-CREATION 
>   src/scheduler/scheduler.cpp c79837c93e7329dbfa22e4288b44237f33408ba9 
> 
> Diff: https://reviews.apache.org/r/48387/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jose Guilherme Vanz
> 
>

Re: Review Request 48387: Delay before initiating a connection with master.

[haosdent huang]

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




src/scheduler/scheduler.cpp (line 73)


Mesos style require header file sort by alphabectial.

Should add it like

```
#include "messages/messages.hpp"

#include "scheduler/flags.hpp"

using namespace mesos;
```



src/scheduler/scheduler.cpp (line 196)


According to mesos style, this variable name should be 
`connectionBackoffMax`

I saw `slave.cpp`, `executor.cpp`, `sched.cpp` use `maxBackoff` as variable 
name.



src/scheduler/scheduler.cpp (line 480)


`Will retry connecting with the master again in `



src/scheduler/scheduler.cpp (line 739)


Need change the name here.


- haosdent huang


On June 8, 2016, 8:53 a.m., Jose Guilherme Vanz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48387/
> ---
> 
> (Updated June 8, 2016, 8:53 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The scheduler library has been updated to wait a little deley before
> initiate a connection with the master. The maximum amount of time waited
> by the scheduler is defined by a flag: CONNECTION_BACKOFF_MAX. After
> the master has been detected the scheduler picks a random delay that
> can be between 0 and the CONNECTION_BACKOFF_MAX value. MESOS-5359
> 
> 
> Diffs
> -
> 
>   src/scheduler/constants.hpp PRE-CREATION 
>   src/scheduler/flags.hpp PRE-CREATION 
>   src/scheduler/scheduler.cpp c79837c93e7329dbfa22e4288b44237f33408ba9 
> 
> Diff: https://reviews.apache.org/r/48387/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jose Guilherme Vanz
> 
>

Re: Review Request 48387: Delay before initiating a connection with master.

[Anand Mazumdar]

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



Thanks for taking this on. Looks pretty good minus a small bug that can lead to 
a crash (see review comments).


src/scheduler/constants.hpp (line 28)


I won't call it a backoff since it's just a _delay_ i.e. a wait time before 
the scheduler library tries to establish a connection with the master.

How about: `DEFAULT_CONNECTION_DELAY_MAX`



src/scheduler/flags.hpp (lines 35 - 36)


How about:

```cpp
The maximum amount of time to wait before trying to initiate a connection 
with the master. The library waits for a random amount of time between [0, b], 
where `b = connection_delay_max` before initiating a (re-)connection attempt 
with the master.
```



src/scheduler/flags.hpp (line 40)


s/connection_backoff_max/connection_delay_max



src/scheduler/scheduler.cpp (line 73)


Let's reorder these as per comments from @haosdent.



src/scheduler/scheduler.cpp (line 196)


Why do we need this variable? Can't we directly use 
`flags.connection_delay_max`?



src/scheduler/scheduler.cpp (lines 475 - 476)


How about:

```
// Wait for a random duration between 0 and `flags.connection_delay_max` 
before (re-)connecting with the master.
```



src/scheduler/scheduler.cpp (line 480)


How about:

```cpp
VLOG(1) << "Waiting for " << delay << " before initiating a (re-)connection 
attempt with the master";
```



src/scheduler/scheduler.cpp (line 481)


hmmm, this can crash the library in its current form due to a failed 
invariant check:

Consider this scenario:
- A new master is detected. You introduce a random amount of delay in 
connecting with this master.
- The new master fails over and a new master is detected before the `delay` 
can be invoked.
- The `delay` for the second master is lesser and the library connects with 
the master.
- The original `delay` now fires and we fail this `CHECK` invariant on: 
https://github.com/apache/mesos/blob/master/src/scheduler/scheduler.cpp#L320

Here is how to get around this:

Let's initialize `connectionId` here and not in `connect()` on L474.

The signature of `connect()` would then change to something like:
`void connect(const UUID& _connectionId)`

This is how a modified `connect` would look like:

```cpp
void connect(const UUID& _connectionId)
{
  // It is possible that a new master was detected while we were waiting to 
establish a connection with the old master.
  if (connectionId != _connectionId) {
VLOG(1) << "Ignoring connection attempt from stale connection";
return;
  }
   // existing content of `connect()` thereafter.
```

Makes sense?



src/scheduler/scheduler.cpp (line 739)


Kill this.


- Anand Mazumdar


On June 8, 2016, 8:53 a.m., Jose Guilherme Vanz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48387/
> ---
> 
> (Updated June 8, 2016, 8:53 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The scheduler library has been updated to wait a little deley before
> initiate a connection with the master. The maximum amount of time waited
> by the scheduler is defined by a flag: CONNECTION_BACKOFF_MAX. After
> the master has been detected the scheduler picks a random delay that
> can be between 0 and the CONNECTION_BACKOFF_MAX value. MESOS-5359
> 
> 
> Diffs
> -
> 
>   src/scheduler/constants.hpp PRE-CREATION 
>   src/scheduler/flags.hpp PRE-CREATION 
>   src/scheduler/scheduler.cpp c79837c93e7329dbfa22e4288b44237f33408ba9 
> 
> Diff: https://reviews.apache.org/r/48387/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jose Guilherme Vanz
> 
>

Re: Review Request 48387: Delay before initiating a connection with master.

[Anand Mazumdar]

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



Jose, any updates on this? I can see that you marked all the outstanding issues 
as fixed. Did you forgot publishing the updated diff?

- Anand Mazumdar


On June 8, 2016, 8:53 a.m., Jose Guilherme Vanz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48387/
> ---
> 
> (Updated June 8, 2016, 8:53 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The scheduler library has been updated to wait a little deley before
> initiate a connection with the master. The maximum amount of time waited
> by the scheduler is defined by a flag: CONNECTION_BACKOFF_MAX. After
> the master has been detected the scheduler picks a random delay that
> can be between 0 and the CONNECTION_BACKOFF_MAX value. MESOS-5359
> 
> 
> Diffs
> -
> 
>   src/scheduler/constants.hpp PRE-CREATION 
>   src/scheduler/flags.hpp PRE-CREATION 
>   src/scheduler/scheduler.cpp c79837c93e7329dbfa22e4288b44237f33408ba9 
> 
> Diff: https://reviews.apache.org/r/48387/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jose Guilherme Vanz
> 
>

Re: Review Request 48387: Delay before initiating a connection with master.

[Jose Guilherme Vanz]

> On June 13, 2016, 6:40 p.m., Anand Mazumdar wrote:
> > Jose, any updates on this? I can see that you marked all the outstanding 
> > issues as fixed. Did you forgot publishing the updated diff?

Hi!

Actually, I had to solve some personal issues and forgot to upload the diff. My 
bad
I'm buildind it again with the lastest commits again and I'll upload it right 
after that.


- Jose Guilherme


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


On June 8, 2016, 5:53 a.m., Jose Guilherme Vanz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48387/
> ---
> 
> (Updated June 8, 2016, 5:53 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The scheduler library has been updated to wait a little deley before
> initiate a connection with the master. The maximum amount of time waited
> by the scheduler is defined by a flag: CONNECTION_BACKOFF_MAX. After
> the master has been detected the scheduler picks a random delay that
> can be between 0 and the CONNECTION_BACKOFF_MAX value. MESOS-5359
> 
> 
> Diffs
> -
> 
>   src/scheduler/constants.hpp PRE-CREATION 
>   src/scheduler/flags.hpp PRE-CREATION 
>   src/scheduler/scheduler.cpp c79837c93e7329dbfa22e4288b44237f33408ba9 
> 
> Diff: https://reviews.apache.org/r/48387/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jose Guilherme Vanz
> 
>

Re: Review Request 48387: Delay before initiating a connection with master.

[Jose Guilherme Vanz]

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

(Updated June 13, 2016, 8:16 p.m.)


Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.


Repository: mesos


Description
---

The scheduler library has been updated to wait a little deley before
initiate a connection with the master. The maximum amount of time waited
by the scheduler is defined by a flag: CONNECTION_BACKOFF_MAX. After
the master has been detected the scheduler picks a random delay that
can be between 0 and the CONNECTION_BACKOFF_MAX value. MESOS-5359


Diffs (updated)
-

  src/scheduler/constants.hpp PRE-CREATION 
  src/scheduler/flags.hpp PRE-CREATION 
  src/scheduler/scheduler.cpp c79837c93e7329dbfa22e4288b44237f33408ba9 

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


Testing
---


Thanks,

Jose Guilherme Vanz

Re: Review Request 48387: Delay before initiating a connection with master.

[Jose Guilherme Vanz]

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

(Updated June 13, 2016, 8:20 p.m.)


Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.


Repository: mesos


Description (updated)
---

The scheduler library has been updated to wait a little deley before
initiate a connection with the master. The maximum amount of time waited
by the scheduler is defined by a flag: CONNECTION_DELAY_MAX. After
the master has been detected the scheduler picks a random delay that
can be between 0 and the CONNECTION_DELAY_MAX value. MESOS-5359


Diffs (updated)
-

  src/scheduler/constants.hpp PRE-CREATION 
  src/scheduler/flags.hpp PRE-CREATION 
  src/scheduler/scheduler.cpp c79837c93e7329dbfa22e4288b44237f33408ba9 

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


Testing
---


Thanks,

Jose Guilherme Vanz

Re: Review Request 48387: Delay before initiating a connection with master.

[Jose Guilherme Vanz]

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

(Updated June 13, 2016, 8:20 p.m.)


Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.


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


Repository: mesos


Description
---

The scheduler library has been updated to wait a little deley before
initiate a connection with the master. The maximum amount of time waited
by the scheduler is defined by a flag: CONNECTION_DELAY_MAX. After
the master has been detected the scheduler picks a random delay that
can be between 0 and the CONNECTION_DELAY_MAX value. MESOS-5359


Diffs
-

  src/scheduler/constants.hpp PRE-CREATION 
  src/scheduler/flags.hpp PRE-CREATION 
  src/scheduler/scheduler.cpp c79837c93e7329dbfa22e4288b44237f33408ba9 

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


Testing
---


Thanks,

Jose Guilherme Vanz

Re: Review Request 48387: Delay before initiating a connection with master.

[Anand Mazumdar]

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



This is looking much better. 

Can you include both `flags.hpp/flags.cpp` in `src/Makefile.am`? This would fix 
the ReviewBot build failures.


src/scheduler/constants.hpp (line 27)


s/connection/connecting with the master



src/scheduler/scheduler.cpp (line 319)


As per my review comment in an earlier version of the patch, let's have the 
signature of this method as:

```cpp
void connect(const UUID& _connectionId);
```

And you can invoke this as:

```cpp
process::delay(delay, self(), &MesosProcess::connect, connectionId.get());
```



src/scheduler/scheduler.cpp (line 477)


Bring this assignment down to L488 i.e. just before `Duration delay` where 
it is used.



src/scheduler/scheduler.cpp (lines 481 - 487)


hmm .. Why do you need to load the flags again here? You already did so on 
line L154, no?



src/scheduler/scheduler.cpp (line 496)


Nit: new line before

We typically have a new line after a statement spanning multiple lines for 
readability.


- Anand Mazumdar


On June 13, 2016, 11:20 p.m., Jose Guilherme Vanz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48387/
> ---
> 
> (Updated June 13, 2016, 11:20 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5359
> https://issues.apache.org/jira/browse/MESOS-5359
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The scheduler library has been updated to wait a little deley before
> initiate a connection with the master. The maximum amount of time waited
> by the scheduler is defined by a flag: CONNECTION_DELAY_MAX. After
> the master has been detected the scheduler picks a random delay that
> can be between 0 and the CONNECTION_DELAY_MAX value. MESOS-5359
> 
> 
> Diffs
> -
> 
>   src/scheduler/constants.hpp PRE-CREATION 
>   src/scheduler/flags.hpp PRE-CREATION 
>   src/scheduler/scheduler.cpp c79837c93e7329dbfa22e4288b44237f33408ba9 
> 
> Diff: https://reviews.apache.org/r/48387/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jose Guilherme Vanz
> 
>

Re: Review Request 48387: Delay before initiating a connection with master.

[Jose Guilherme Vanz]

> On June 14, 2016, 11:40 a.m., Anand Mazumdar wrote:
> > src/scheduler/scheduler.cpp, line 320
> > 
> >
> > As per my review comment in an earlier version of the patch, let's have 
> > the signature of this method as:
> > 
> > ```cpp
> > void connect(const UUID& _connectionId);
> > ```
> > 
> > And you can invoke this as:
> > 
> > ```cpp
> > process::delay(delay, self(), &MesosProcess::connect, 
> > connectionId.get());
> > ```

Considering that Option's == operator already checks the content, uses the UUID 
instead of Option is a standart or just a prefererence in the way to do 
stuff? ;D


> On June 14, 2016, 11:40 a.m., Anand Mazumdar wrote:
> > src/scheduler/scheduler.cpp, lines 484-490
> > 
> >
> > hmm .. Why do you need to load the flags again here? You already did so 
> > on line L154, no?

The flag variable is initializated in constructor stack, right? I can not 
access it from other function. Or... I'm missing something else around how 
stout and libprocess handle the process.


- Jose Guilherme


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


On June 13, 2016, 8:20 p.m., Jose Guilherme Vanz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48387/
> ---
> 
> (Updated June 13, 2016, 8:20 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5359
> https://issues.apache.org/jira/browse/MESOS-5359
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The scheduler library has been updated to wait a little deley before
> initiate a connection with the master. The maximum amount of time waited
> by the scheduler is defined by a flag: CONNECTION_DELAY_MAX. After
> the master has been detected the scheduler picks a random delay that
> can be between 0 and the CONNECTION_DELAY_MAX value. MESOS-5359
> 
> 
> Diffs
> -
> 
>   src/scheduler/constants.hpp PRE-CREATION 
>   src/scheduler/flags.hpp PRE-CREATION 
>   src/scheduler/scheduler.cpp c79837c93e7329dbfa22e4288b44237f33408ba9 
> 
> Diff: https://reviews.apache.org/r/48387/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jose Guilherme Vanz
> 
>

Re: Review Request 48387: Delay before initiating a connection with master.

[Anand Mazumdar]

> On June 14, 2016, 2:40 p.m., Anand Mazumdar wrote:
> > src/scheduler/scheduler.cpp, lines 484-490
> > 
> >
> > hmm .. Why do you need to load the flags again here? You already did so 
> > on line L154, no?
> 
> Jose Guilherme Vanz wrote:
> The flag variable is initializated in constructor stack, right? I can not 
> access it from other function. Or... I'm missing something else around how 
> stout and libprocess handle the process.

My bad, looks like I jumped the gun here. I was referring to having `Flags` as 
a member variable and then accessing it directly here instead of loading it 
again here. This is similar to other places in the code that deal with `Flags` 
like the driver, master, agent etc.


> On June 14, 2016, 2:40 p.m., Anand Mazumdar wrote:
> > src/scheduler/scheduler.cpp, line 320
> > 
> >
> > As per my review comment in an earlier version of the patch, let's have 
> > the signature of this method as:
> > 
> > ```cpp
> > void connect(const UUID& _connectionId);
> > ```
> > 
> > And you can invoke this as:
> > 
> > ```cpp
> > process::delay(delay, self(), &MesosProcess::connect, 
> > connectionId.get());
> > ```
> 
> Jose Guilherme Vanz wrote:
> Considering that Option's == operator already checks the content, uses 
> the UUID instead of Option is a standart or just a prefererence in the 
> way to do stuff? ;D

An `Option` can wrap any `T`. If a reader see the argument of a function as 
`Option arg`, it means that `arg` would either have a value of type `T` or 
would also have a `None()` in some scenarios. In this particular case though, 
`arg` would always be of type `UUID` and it can't ever be a `None`. This helps 
in readability and reasoning about code invariants/flow better. Makes sense?


- Anand


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


On June 13, 2016, 11:20 p.m., Jose Guilherme Vanz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48387/
> ---
> 
> (Updated June 13, 2016, 11:20 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5359
> https://issues.apache.org/jira/browse/MESOS-5359
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The scheduler library has been updated to wait a little deley before
> initiate a connection with the master. The maximum amount of time waited
> by the scheduler is defined by a flag: CONNECTION_DELAY_MAX. After
> the master has been detected the scheduler picks a random delay that
> can be between 0 and the CONNECTION_DELAY_MAX value. MESOS-5359
> 
> 
> Diffs
> -
> 
>   src/scheduler/constants.hpp PRE-CREATION 
>   src/scheduler/flags.hpp PRE-CREATION 
>   src/scheduler/scheduler.cpp c79837c93e7329dbfa22e4288b44237f33408ba9 
> 
> Diff: https://reviews.apache.org/r/48387/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jose Guilherme Vanz
> 
>

Re: Review Request 48387: Delay before initiating a connection with master.

[Jose Guilherme Vanz]

> On June 14, 2016, 11:40 a.m., Anand Mazumdar wrote:
> > src/scheduler/scheduler.cpp, line 320
> > 
> >
> > As per my review comment in an earlier version of the patch, let's have 
> > the signature of this method as:
> > 
> > ```cpp
> > void connect(const UUID& _connectionId);
> > ```
> > 
> > And you can invoke this as:
> > 
> > ```cpp
> > process::delay(delay, self(), &MesosProcess::connect, 
> > connectionId.get());
> > ```
> 
> Jose Guilherme Vanz wrote:
> Considering that Option's == operator already checks the content, uses 
> the UUID instead of Option is a standart or just a prefererence in the 
> way to do stuff? ;D
> 
> Anand Mazumdar wrote:
> An `Option` can wrap any `T`. If a reader see the argument of a 
> function as `Option arg`, it means that `arg` would either have a value of 
> type `T` or would also have a `None()` in some scenarios. In this particular 
> case though, `arg` would always be of type `UUID` and it can't ever be a 
> `None`. This helps in readability and reasoning about code invariants/flow 
> better. Makes sense?

Got it. Thanks =)


- Jose Guilherme


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


On June 13, 2016, 8:20 p.m., Jose Guilherme Vanz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48387/
> ---
> 
> (Updated June 13, 2016, 8:20 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5359
> https://issues.apache.org/jira/browse/MESOS-5359
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The scheduler library has been updated to wait a little deley before
> initiate a connection with the master. The maximum amount of time waited
> by the scheduler is defined by a flag: CONNECTION_DELAY_MAX. After
> the master has been detected the scheduler picks a random delay that
> can be between 0 and the CONNECTION_DELAY_MAX value. MESOS-5359
> 
> 
> Diffs
> -
> 
>   src/scheduler/constants.hpp PRE-CREATION 
>   src/scheduler/flags.hpp PRE-CREATION 
>   src/scheduler/scheduler.cpp c79837c93e7329dbfa22e4288b44237f33408ba9 
> 
> Diff: https://reviews.apache.org/r/48387/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jose Guilherme Vanz
> 
>

Re: Review Request 48387: Delay before initiating a connection with master.

[Anand Mazumdar]

> On June 14, 2016, 2:40 p.m., Anand Mazumdar wrote:
> > This is looking much better. 
> > 
> > Can you include both `flags.hpp/flags.cpp` in `src/Makefile.am`? This would 
> > fix the ReviewBot build failures.

Hey Jose, I see that you marked the issues as resolved. Did you forget 
publishing the updated diff? 

Also, did you add the new files to the Makefile as per my earlier comment?


- Anand


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


On June 13, 2016, 11:20 p.m., Jose Guilherme Vanz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48387/
> ---
> 
> (Updated June 13, 2016, 11:20 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5359
> https://issues.apache.org/jira/browse/MESOS-5359
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The scheduler library has been updated to wait a little deley before
> initiate a connection with the master. The maximum amount of time waited
> by the scheduler is defined by a flag: CONNECTION_DELAY_MAX. After
> the master has been detected the scheduler picks a random delay that
> can be between 0 and the CONNECTION_DELAY_MAX value. MESOS-5359
> 
> 
> Diffs
> -
> 
>   src/scheduler/constants.hpp PRE-CREATION 
>   src/scheduler/flags.hpp PRE-CREATION 
>   src/scheduler/scheduler.cpp c79837c93e7329dbfa22e4288b44237f33408ba9 
> 
> Diff: https://reviews.apache.org/r/48387/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jose Guilherme Vanz
> 
>

Re: Review Request 48387: Delay before initiating a connection with master.

[Jose Guilherme Vanz]

> On June 14, 2016, 11:40 a.m., Anand Mazumdar wrote:
> > This is looking much better. 
> > 
> > Can you include both `flags.hpp/flags.cpp` in `src/Makefile.am`? This would 
> > fix the ReviewBot build failures.
> 
> Anand Mazumdar wrote:
> Hey Jose, I see that you marked the issues as resolved. Did you forget 
> publishing the updated diff? 
> 
> Also, did you add the new files to the Makefile as per my earlier comment?

I did not update the diff yet because I was running a full build in my local 
machine with the last commits.
I just saw that some unit tests failed, I would like to solve the issues before 
upload the diff.


- Jose Guilherme


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


On June 13, 2016, 8:20 p.m., Jose Guilherme Vanz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48387/
> ---
> 
> (Updated June 13, 2016, 8:20 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5359
> https://issues.apache.org/jira/browse/MESOS-5359
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The scheduler library has been updated to wait a little deley before
> initiate a connection with the master. The maximum amount of time waited
> by the scheduler is defined by a flag: CONNECTION_DELAY_MAX. After
> the master has been detected the scheduler picks a random delay that
> can be between 0 and the CONNECTION_DELAY_MAX value. MESOS-5359
> 
> 
> Diffs
> -
> 
>   src/scheduler/constants.hpp PRE-CREATION 
>   src/scheduler/flags.hpp PRE-CREATION 
>   src/scheduler/scheduler.cpp c79837c93e7329dbfa22e4288b44237f33408ba9 
> 
> Diff: https://reviews.apache.org/r/48387/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jose Guilherme Vanz
> 
>

Re: Review Request 48387: Delay before initiating a connection with master.

[Jose Guilherme Vanz]

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

(Updated June 18, 2016, 2:05 a.m.)


Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.


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


Repository: mesos


Description
---

The scheduler library has been updated to wait a little deley before
initiate a connection with the master. The maximum amount of time waited
by the scheduler is defined by a flag: CONNECTION_DELAY_MAX. After
the master has been detected the scheduler picks a random delay that
can be between 0 and the CONNECTION_DELAY_MAX value. MESOS-5359


Diffs (updated)
-

  src/Makefile.am 90b6c033054a09e9fbad9066e0763113a13a4d09 
  src/scheduler/constants.hpp PRE-CREATION 
  src/scheduler/flags.hpp PRE-CREATION 
  src/scheduler/scheduler.cpp c79837c93e7329dbfa22e4288b44237f33408ba9 
  src/tests/http_fault_tolerance_tests.cpp 
baa07395b9bd588daa5438369954584787d7952a 
  src/tests/master_maintenance_tests.cpp 
a8649fdf3cbd4996d1528393506b1b0ed0a7cf5c 

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


Testing
---


Thanks,

Jose Guilherme Vanz

Re: Review Request 48387: Delay before initiating a connection with master.

[Jose Guilherme Vanz]

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

(Updated June 18, 2016, 2:07 a.m.)


Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.


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


Repository: mesos


Description
---

The scheduler library has been updated to wait a little deley before
initiate a connection with the master. The maximum amount of time waited
by the scheduler is defined by a flag: CONNECTION_DELAY_MAX. After
the master has been detected the scheduler picks a random delay that
can be between 0 and the CONNECTION_DELAY_MAX value. MESOS-5359


Diffs
-

  src/Makefile.am 90b6c033054a09e9fbad9066e0763113a13a4d09 
  src/scheduler/constants.hpp PRE-CREATION 
  src/scheduler/flags.hpp PRE-CREATION 
  src/scheduler/scheduler.cpp c79837c93e7329dbfa22e4288b44237f33408ba9 
  src/tests/http_fault_tolerance_tests.cpp 
baa07395b9bd588daa5438369954584787d7952a 
  src/tests/master_maintenance_tests.cpp 
a8649fdf3cbd4996d1528393506b1b0ed0a7cf5c 

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


Testing (updated)
---

mesos-tests


Thanks,

Jose Guilherme Vanz

Re: Review Request 48387: Delay before initiating a connection with master.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [48387]

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

- Mesos ReviewBot


On June 18, 2016, 5:07 a.m., Jose Guilherme Vanz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48387/
> ---
> 
> (Updated June 18, 2016, 5:07 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5359
> https://issues.apache.org/jira/browse/MESOS-5359
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The scheduler library has been updated to wait a little deley before
> initiate a connection with the master. The maximum amount of time waited
> by the scheduler is defined by a flag: CONNECTION_DELAY_MAX. After
> the master has been detected the scheduler picks a random delay that
> can be between 0 and the CONNECTION_DELAY_MAX value. MESOS-5359
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 90b6c033054a09e9fbad9066e0763113a13a4d09 
>   src/scheduler/constants.hpp PRE-CREATION 
>   src/scheduler/flags.hpp PRE-CREATION 
>   src/scheduler/scheduler.cpp c79837c93e7329dbfa22e4288b44237f33408ba9 
>   src/tests/http_fault_tolerance_tests.cpp 
> baa07395b9bd588daa5438369954584787d7952a 
>   src/tests/master_maintenance_tests.cpp 
> a8649fdf3cbd4996d1528393506b1b0ed0a7cf5c 
> 
> Diff: https://reviews.apache.org/r/48387/diff/
> 
> 
> Testing
> ---
> 
> mesos-tests
> 
> 
> Thanks,
> 
> Jose Guilherme Vanz
> 
>

Re: Review Request 48387: Delay before initiating a connection with master.

[Jose Guilherme Vanz]

> On June 18, 2016, 4:38 a.m., Mesos ReviewBot wrote:
> > Patch looks great!
> > 
> > Reviews applied: [48387]
> > 
> > Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' 
> > COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 
> > MESOS_VERBOSE=1'; ./support/docker_build.sh

Finally! I've submitted the patch. Sorry for the delay. ;)


- Jose Guilherme


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


On June 18, 2016, 2:07 a.m., Jose Guilherme Vanz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48387/
> ---
> 
> (Updated June 18, 2016, 2:07 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5359
> https://issues.apache.org/jira/browse/MESOS-5359
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The scheduler library has been updated to wait a little deley before
> initiate a connection with the master. The maximum amount of time waited
> by the scheduler is defined by a flag: CONNECTION_DELAY_MAX. After
> the master has been detected the scheduler picks a random delay that
> can be between 0 and the CONNECTION_DELAY_MAX value. MESOS-5359
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 90b6c033054a09e9fbad9066e0763113a13a4d09 
>   src/scheduler/constants.hpp PRE-CREATION 
>   src/scheduler/flags.hpp PRE-CREATION 
>   src/scheduler/scheduler.cpp c79837c93e7329dbfa22e4288b44237f33408ba9 
>   src/tests/http_fault_tolerance_tests.cpp 
> baa07395b9bd588daa5438369954584787d7952a 
>   src/tests/master_maintenance_tests.cpp 
> a8649fdf3cbd4996d1528393506b1b0ed0a7cf5c 
> 
> Diff: https://reviews.apache.org/r/48387/diff/
> 
> 
> Testing
> ---
> 
> mesos-tests
> 
> 
> Thanks,
> 
> Jose Guilherme Vanz
> 
>

Re: Review Request 48387: Delay before initiating a connection with master.

[haosdent huang]

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


Fix it, then Ship it!




LGTM! Thank you for your patch!


src/tests/http_fault_tolerance_tests.cpp (line 63)


Move this above like
```
using mesos::v1::scheduler::Call;
using mesos::v1::scheduler::DEFAULT_CONNECTION_DELAY_MAX;
using mesos::v1::scheduler::Event;
```


- haosdent huang


On June 18, 2016, 5:07 a.m., Jose Guilherme Vanz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48387/
> ---
> 
> (Updated June 18, 2016, 5:07 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5359
> https://issues.apache.org/jira/browse/MESOS-5359
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The scheduler library has been updated to wait a little deley before
> initiate a connection with the master. The maximum amount of time waited
> by the scheduler is defined by a flag: CONNECTION_DELAY_MAX. After
> the master has been detected the scheduler picks a random delay that
> can be between 0 and the CONNECTION_DELAY_MAX value. MESOS-5359
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 90b6c033054a09e9fbad9066e0763113a13a4d09 
>   src/scheduler/constants.hpp PRE-CREATION 
>   src/scheduler/flags.hpp PRE-CREATION 
>   src/scheduler/scheduler.cpp c79837c93e7329dbfa22e4288b44237f33408ba9 
>   src/tests/http_fault_tolerance_tests.cpp 
> baa07395b9bd588daa5438369954584787d7952a 
>   src/tests/master_maintenance_tests.cpp 
> a8649fdf3cbd4996d1528393506b1b0ed0a7cf5c 
> 
> Diff: https://reviews.apache.org/r/48387/diff/
> 
> 
> Testing
> ---
> 
> mesos-tests
> 
> 
> Thanks,
> 
> Jose Guilherme Vanz
> 
>

Re: Review Request 48387: Delay before initiating a connection with master.

[Jose Guilherme Vanz]

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

(Updated June 19, 2016, 4:08 p.m.)


Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.


Changes
---

Reorder using statements


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


Repository: mesos


Description
---

The scheduler library has been updated to wait a little deley before
initiate a connection with the master. The maximum amount of time waited
by the scheduler is defined by a flag: CONNECTION_DELAY_MAX. After
the master has been detected the scheduler picks a random delay that
can be between 0 and the CONNECTION_DELAY_MAX value. MESOS-5359


Diffs (updated)
-

  src/Makefile.am a4931560f1a5b3fbe41ea181477341d3ac459b58 
  src/scheduler/constants.hpp PRE-CREATION 
  src/scheduler/flags.hpp PRE-CREATION 
  src/scheduler/scheduler.cpp c79837c93e7329dbfa22e4288b44237f33408ba9 
  src/tests/http_fault_tolerance_tests.cpp 
baa07395b9bd588daa5438369954584787d7952a 
  src/tests/master_maintenance_tests.cpp 
a8649fdf3cbd4996d1528393506b1b0ed0a7cf5c 

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


Testing
---

mesos-tests


Thanks,

Jose Guilherme Vanz

Re: Review Request 48387: Delay before initiating a connection with master.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [48387]

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

- Mesos ReviewBot


On June 19, 2016, 7:08 p.m., Jose Guilherme Vanz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48387/
> ---
> 
> (Updated June 19, 2016, 7:08 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5359
> https://issues.apache.org/jira/browse/MESOS-5359
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The scheduler library has been updated to wait a little deley before
> initiate a connection with the master. The maximum amount of time waited
> by the scheduler is defined by a flag: CONNECTION_DELAY_MAX. After
> the master has been detected the scheduler picks a random delay that
> can be between 0 and the CONNECTION_DELAY_MAX value. MESOS-5359
> 
> 
> Diffs
> -
> 
>   src/Makefile.am a4931560f1a5b3fbe41ea181477341d3ac459b58 
>   src/scheduler/constants.hpp PRE-CREATION 
>   src/scheduler/flags.hpp PRE-CREATION 
>   src/scheduler/scheduler.cpp c79837c93e7329dbfa22e4288b44237f33408ba9 
>   src/tests/http_fault_tolerance_tests.cpp 
> baa07395b9bd588daa5438369954584787d7952a 
>   src/tests/master_maintenance_tests.cpp 
> a8649fdf3cbd4996d1528393506b1b0ed0a7cf5c 
> 
> Diff: https://reviews.apache.org/r/48387/diff/
> 
> 
> Testing
> ---
> 
> mesos-tests
> 
> 
> Thanks,
> 
> Jose Guilherme Vanz
> 
>

Re: Review Request 48387: Delay before initiating a connection with master.

[Anand Mazumdar]

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


Fix it, then Ship it!




Thanks for your patience. This looks good!

Some other suggestions:
- Regarding the changes in tests, we can just resume the clock. This is much 
more simpler/intuitive.
- The newly introduced delay makes the tests run a lot slower sometimes ~1 sec! 
Let's default the delay to `0ms` for the tests.

I have included the changes from the above suggestions in this diff: 
https://gist.github.com/hatred/171deb74c3b28b13fc5e681fb697a64c

I would be happy to commit the changes once you post the updated diff from the 
gist + the other minor issues I raised below.


src/scheduler/constants.hpp (line 30)


Nit: Extra new line



src/scheduler/flags.hpp (line 49)


Nit: Extra new line



src/scheduler/scheduler.cpp (line 141)


Pass by const ref. `const Flags&`



src/scheduler/scheduler.cpp (line 307)


It is possible to compare a `Option` directly with `T`. You can just do 
`connectionId != _connectionId`.



src/scheduler/scheduler.cpp (line 461)


Nit: Kill this extra new line.



src/scheduler/scheduler.cpp (line 470)


Nit: new line before. We typically have a newline after a statement 
spanning multiple lines.



src/scheduler/scheduler.cpp (lines 747 - 751)


Kill this comment. It is no longer relevant. We are no longer loading from 
`local::Flags`.



src/scheduler/scheduler.cpp (line 765)


Nit: Kill this new line.


- Anand Mazumdar


On June 19, 2016, 7:08 p.m., Jose Guilherme Vanz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48387/
> ---
> 
> (Updated June 19, 2016, 7:08 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5359
> https://issues.apache.org/jira/browse/MESOS-5359
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The scheduler library has been updated to wait a little deley before
> initiate a connection with the master. The maximum amount of time waited
> by the scheduler is defined by a flag: CONNECTION_DELAY_MAX. After
> the master has been detected the scheduler picks a random delay that
> can be between 0 and the CONNECTION_DELAY_MAX value. MESOS-5359
> 
> 
> Diffs
> -
> 
>   src/Makefile.am a4931560f1a5b3fbe41ea181477341d3ac459b58 
>   src/scheduler/constants.hpp PRE-CREATION 
>   src/scheduler/flags.hpp PRE-CREATION 
>   src/scheduler/scheduler.cpp c79837c93e7329dbfa22e4288b44237f33408ba9 
>   src/tests/http_fault_tolerance_tests.cpp 
> baa07395b9bd588daa5438369954584787d7952a 
>   src/tests/master_maintenance_tests.cpp 
> a8649fdf3cbd4996d1528393506b1b0ed0a7cf5c 
> 
> Diff: https://reviews.apache.org/r/48387/diff/
> 
> 
> Testing
> ---
> 
> mesos-tests
> 
> 
> Thanks,
> 
> Jose Guilherme Vanz
> 
>

Re: Review Request 48387: Delay before initiating a connection with master.

[Jose Guilherme Vanz]

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




src/scheduler/constants.hpp (line 30)


Sorry my my fail, but what's "Nit" means? 
I suppose I should remove the extra line, right?


- Jose Guilherme Vanz


On June 19, 2016, 4:08 p.m., Jose Guilherme Vanz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48387/
> ---
> 
> (Updated June 19, 2016, 4:08 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5359
> https://issues.apache.org/jira/browse/MESOS-5359
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The scheduler library has been updated to wait a little deley before
> initiate a connection with the master. The maximum amount of time waited
> by the scheduler is defined by a flag: CONNECTION_DELAY_MAX. After
> the master has been detected the scheduler picks a random delay that
> can be between 0 and the CONNECTION_DELAY_MAX value. MESOS-5359
> 
> 
> Diffs
> -
> 
>   src/Makefile.am a4931560f1a5b3fbe41ea181477341d3ac459b58 
>   src/scheduler/constants.hpp PRE-CREATION 
>   src/scheduler/flags.hpp PRE-CREATION 
>   src/scheduler/scheduler.cpp c79837c93e7329dbfa22e4288b44237f33408ba9 
>   src/tests/http_fault_tolerance_tests.cpp 
> baa07395b9bd588daa5438369954584787d7952a 
>   src/tests/master_maintenance_tests.cpp 
> a8649fdf3cbd4996d1528393506b1b0ed0a7cf5c 
> 
> Diff: https://reviews.apache.org/r/48387/diff/
> 
> 
> Testing
> ---
> 
> mesos-tests
> 
> 
> Thanks,
> 
> Jose Guilherme Vanz
> 
>

Re: Review Request 48387: Delay before initiating a connection with master.

[Anand Mazumdar]

> On June 19, 2016, 11:56 p.m., Jose Guilherme Vanz wrote:
> > src/scheduler/constants.hpp, line 30
> > 
> >
> > Sorry my my fail, but what's "Nit" means? 
> > I suppose I should remove the extra line, right?

+1:  
http://stackoverflow.com/questions/27810522/what-does-nit-mean-in-hacker-speak


- Anand


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


On June 19, 2016, 7:08 p.m., Jose Guilherme Vanz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48387/
> ---
> 
> (Updated June 19, 2016, 7:08 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5359
> https://issues.apache.org/jira/browse/MESOS-5359
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The scheduler library has been updated to wait a little deley before
> initiate a connection with the master. The maximum amount of time waited
> by the scheduler is defined by a flag: CONNECTION_DELAY_MAX. After
> the master has been detected the scheduler picks a random delay that
> can be between 0 and the CONNECTION_DELAY_MAX value. MESOS-5359
> 
> 
> Diffs
> -
> 
>   src/Makefile.am a4931560f1a5b3fbe41ea181477341d3ac459b58 
>   src/scheduler/constants.hpp PRE-CREATION 
>   src/scheduler/flags.hpp PRE-CREATION 
>   src/scheduler/scheduler.cpp c79837c93e7329dbfa22e4288b44237f33408ba9 
>   src/tests/http_fault_tolerance_tests.cpp 
> baa07395b9bd588daa5438369954584787d7952a 
>   src/tests/master_maintenance_tests.cpp 
> a8649fdf3cbd4996d1528393506b1b0ed0a7cf5c 
> 
> Diff: https://reviews.apache.org/r/48387/diff/
> 
> 
> Testing
> ---
> 
> mesos-tests
> 
> 
> Thanks,
> 
> Jose Guilherme Vanz
> 
>

Re: Review Request 48387: Delay before initiating a connection with master.

[haosdent huang]

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




src/tests/master_maintenance_tests.cpp (line 1498)


This extra line need to kill as well.


- haosdent huang


On June 19, 2016, 7:08 p.m., Jose Guilherme Vanz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48387/
> ---
> 
> (Updated June 19, 2016, 7:08 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5359
> https://issues.apache.org/jira/browse/MESOS-5359
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The scheduler library has been updated to wait a little deley before
> initiate a connection with the master. The maximum amount of time waited
> by the scheduler is defined by a flag: CONNECTION_DELAY_MAX. After
> the master has been detected the scheduler picks a random delay that
> can be between 0 and the CONNECTION_DELAY_MAX value. MESOS-5359
> 
> 
> Diffs
> -
> 
>   src/Makefile.am a4931560f1a5b3fbe41ea181477341d3ac459b58 
>   src/scheduler/constants.hpp PRE-CREATION 
>   src/scheduler/flags.hpp PRE-CREATION 
>   src/scheduler/scheduler.cpp c79837c93e7329dbfa22e4288b44237f33408ba9 
>   src/tests/http_fault_tolerance_tests.cpp 
> baa07395b9bd588daa5438369954584787d7952a 
>   src/tests/master_maintenance_tests.cpp 
> a8649fdf3cbd4996d1528393506b1b0ed0a7cf5c 
> 
> Diff: https://reviews.apache.org/r/48387/diff/
> 
> 
> Testing
> ---
> 
> mesos-tests
> 
> 
> Thanks,
> 
> Jose Guilherme Vanz
> 
>

Re: Review Request 48387: Delay before initiating a connection with master.

[Jose Guilherme Vanz]

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

(Updated June 20, 2016, 6:38 a.m.)


Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.


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


Repository: mesos


Description
---

The scheduler library has been updated to wait a little deley before
initiate a connection with the master. The maximum amount of time waited
by the scheduler is defined by a flag: CONNECTION_DELAY_MAX. After
the master has been detected the scheduler picks a random delay that
can be between 0 and the CONNECTION_DELAY_MAX value. MESOS-5359


Diffs (updated)
-

  src/Makefile.am a4931560f1a5b3fbe41ea181477341d3ac459b58 
  src/scheduler/constants.hpp PRE-CREATION 
  src/scheduler/flags.hpp PRE-CREATION 
  src/scheduler/scheduler.cpp c79837c93e7329dbfa22e4288b44237f33408ba9 
  src/tests/http_fault_tolerance_tests.cpp 
baa07395b9bd588daa5438369954584787d7952a 
  src/tests/master_maintenance_tests.cpp 
a8649fdf3cbd4996d1528393506b1b0ed0a7cf5c 
  src/tests/mesos.hpp 6bd3a708db450eae21bd5c59f74c811bf87b3c89 
  src/tests/mesos.cpp e7792fde39a60536f9651f75f1cbfa09b28651a9 

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


Testing
---

mesos-tests


Thanks,

Jose Guilherme Vanz

Re: Review Request 48387: Delay before initiating a connection with master.

[haosdent huang]

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




src/tests/mesos.cpp (line 84)


Usually we don't set flag in this way. We prefer to modify `master::Flags 
MesosTest::CreateMasterFlags()` in `src/tests/mesos.cpp`.

Please see how we set global master flags for default test cases there.


- haosdent huang


On June 20, 2016, 9:38 a.m., Jose Guilherme Vanz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48387/
> ---
> 
> (Updated June 20, 2016, 9:38 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5359
> https://issues.apache.org/jira/browse/MESOS-5359
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The scheduler library has been updated to wait a little deley before
> initiate a connection with the master. The maximum amount of time waited
> by the scheduler is defined by a flag: CONNECTION_DELAY_MAX. After
> the master has been detected the scheduler picks a random delay that
> can be between 0 and the CONNECTION_DELAY_MAX value. MESOS-5359
> 
> 
> Diffs
> -
> 
>   src/Makefile.am a4931560f1a5b3fbe41ea181477341d3ac459b58 
>   src/scheduler/constants.hpp PRE-CREATION 
>   src/scheduler/flags.hpp PRE-CREATION 
>   src/scheduler/scheduler.cpp c79837c93e7329dbfa22e4288b44237f33408ba9 
>   src/tests/http_fault_tolerance_tests.cpp 
> baa07395b9bd588daa5438369954584787d7952a 
>   src/tests/master_maintenance_tests.cpp 
> a8649fdf3cbd4996d1528393506b1b0ed0a7cf5c 
>   src/tests/mesos.hpp 6bd3a708db450eae21bd5c59f74c811bf87b3c89 
>   src/tests/mesos.cpp e7792fde39a60536f9651f75f1cbfa09b28651a9 
> 
> Diff: https://reviews.apache.org/r/48387/diff/
> 
> 
> Testing
> ---
> 
> mesos-tests
> 
> 
> Thanks,
> 
> Jose Guilherme Vanz
> 
>

Re: Review Request 48387: Delay before initiating a connection with master.

[Jose Guilherme Vanz]

> On June 20, 2016, 6:51 a.m., haosdent huang wrote:
> > src/tests/mesos.cpp, line 84
> > 
> >
> > Usually we don't set flag in this way. We prefer to modify 
> > `master::Flags MesosTest::CreateMasterFlags()` in `src/tests/mesos.cpp`.
> > 
> > Please see how we set global master flags for default test cases there.

The MESOS_CONNECTION_DELAY_MAX is not a master's flag, it is a scheduler's 
flag. It is not possible pass it as a parameter. It's loaded from enviroment 
variables only.


- Jose Guilherme


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


On June 20, 2016, 6:38 a.m., Jose Guilherme Vanz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48387/
> ---
> 
> (Updated June 20, 2016, 6:38 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5359
> https://issues.apache.org/jira/browse/MESOS-5359
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The scheduler library has been updated to wait a little deley before
> initiate a connection with the master. The maximum amount of time waited
> by the scheduler is defined by a flag: CONNECTION_DELAY_MAX. After
> the master has been detected the scheduler picks a random delay that
> can be between 0 and the CONNECTION_DELAY_MAX value. MESOS-5359
> 
> 
> Diffs
> -
> 
>   src/Makefile.am a4931560f1a5b3fbe41ea181477341d3ac459b58 
>   src/scheduler/constants.hpp PRE-CREATION 
>   src/scheduler/flags.hpp PRE-CREATION 
>   src/scheduler/scheduler.cpp c79837c93e7329dbfa22e4288b44237f33408ba9 
>   src/tests/http_fault_tolerance_tests.cpp 
> baa07395b9bd588daa5438369954584787d7952a 
>   src/tests/master_maintenance_tests.cpp 
> a8649fdf3cbd4996d1528393506b1b0ed0a7cf5c 
>   src/tests/mesos.hpp 6bd3a708db450eae21bd5c59f74c811bf87b3c89 
>   src/tests/mesos.cpp e7792fde39a60536f9651f75f1cbfa09b28651a9 
> 
> Diff: https://reviews.apache.org/r/48387/diff/
> 
> 
> Testing
> ---
> 
> mesos-tests
> 
> 
> Thanks,
> 
> Jose Guilherme Vanz
> 
>

Re: Review Request 48387: Delay before initiating a connection with master.

[haosdent huang]

> On June 20, 2016, 9:51 a.m., haosdent huang wrote:
> > src/tests/mesos.cpp, line 84
> > 
> >
> > Usually we don't set flag in this way. We prefer to modify 
> > `master::Flags MesosTest::CreateMasterFlags()` in `src/tests/mesos.cpp`.
> > 
> > Please see how we set global master flags for default test cases there.
> 
> Jose Guilherme Vanz wrote:
> The MESOS_CONNECTION_DELAY_MAX is not a master's flag, it is a 
> scheduler's flag. It is not possible pass it as a parameter. It's loaded from 
> enviroment variables only.

Got it, thanks for explanation.


- haosdent


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


On June 20, 2016, 9:38 a.m., Jose Guilherme Vanz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48387/
> ---
> 
> (Updated June 20, 2016, 9:38 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5359
> https://issues.apache.org/jira/browse/MESOS-5359
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The scheduler library has been updated to wait a little deley before
> initiate a connection with the master. The maximum amount of time waited
> by the scheduler is defined by a flag: CONNECTION_DELAY_MAX. After
> the master has been detected the scheduler picks a random delay that
> can be between 0 and the CONNECTION_DELAY_MAX value. MESOS-5359
> 
> 
> Diffs
> -
> 
>   src/Makefile.am a4931560f1a5b3fbe41ea181477341d3ac459b58 
>   src/scheduler/constants.hpp PRE-CREATION 
>   src/scheduler/flags.hpp PRE-CREATION 
>   src/scheduler/scheduler.cpp c79837c93e7329dbfa22e4288b44237f33408ba9 
>   src/tests/http_fault_tolerance_tests.cpp 
> baa07395b9bd588daa5438369954584787d7952a 
>   src/tests/master_maintenance_tests.cpp 
> a8649fdf3cbd4996d1528393506b1b0ed0a7cf5c 
>   src/tests/mesos.hpp 6bd3a708db450eae21bd5c59f74c811bf87b3c89 
>   src/tests/mesos.cpp e7792fde39a60536f9651f75f1cbfa09b28651a9 
> 
> Diff: https://reviews.apache.org/r/48387/diff/
> 
> 
> Testing
> ---
> 
> mesos-tests
> 
> 
> Thanks,
> 
> Jose Guilherme Vanz
> 
>

Re: Review Request 48387: Delay before initiating a connection with master.

[Jose Guilherme Vanz]

> On June 20, 2016, 6:51 a.m., haosdent huang wrote:
> > src/tests/mesos.cpp, line 84
> > 
> >
> > Usually we don't set flag in this way. We prefer to modify 
> > `master::Flags MesosTest::CreateMasterFlags()` in `src/tests/mesos.cpp`.
> > 
> > Please see how we set global master flags for default test cases there.
> 
> Jose Guilherme Vanz wrote:
> The MESOS_CONNECTION_DELAY_MAX is not a master's flag, it is a 
> scheduler's flag. It is not possible pass it as a parameter. It's loaded from 
> enviroment variables only.
> 
> haosdent huang wrote:
> Got it, thanks for explanation.

;)


- Jose Guilherme


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


On June 20, 2016, 6:38 a.m., Jose Guilherme Vanz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48387/
> ---
> 
> (Updated June 20, 2016, 6:38 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5359
> https://issues.apache.org/jira/browse/MESOS-5359
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The scheduler library has been updated to wait a little deley before
> initiate a connection with the master. The maximum amount of time waited
> by the scheduler is defined by a flag: CONNECTION_DELAY_MAX. After
> the master has been detected the scheduler picks a random delay that
> can be between 0 and the CONNECTION_DELAY_MAX value. MESOS-5359
> 
> 
> Diffs
> -
> 
>   src/Makefile.am a4931560f1a5b3fbe41ea181477341d3ac459b58 
>   src/scheduler/constants.hpp PRE-CREATION 
>   src/scheduler/flags.hpp PRE-CREATION 
>   src/scheduler/scheduler.cpp c79837c93e7329dbfa22e4288b44237f33408ba9 
>   src/tests/http_fault_tolerance_tests.cpp 
> baa07395b9bd588daa5438369954584787d7952a 
>   src/tests/master_maintenance_tests.cpp 
> a8649fdf3cbd4996d1528393506b1b0ed0a7cf5c 
>   src/tests/mesos.hpp 6bd3a708db450eae21bd5c59f74c811bf87b3c89 
>   src/tests/mesos.cpp e7792fde39a60536f9651f75f1cbfa09b28651a9 
> 
> Diff: https://reviews.apache.org/r/48387/diff/
> 
> 
> Testing
> ---
> 
> mesos-tests
> 
> 
> Thanks,
> 
> Jose Guilherme Vanz
> 
>

Re: Review Request 48387: Delay before initiating a connection with master.

[Anand Mazumdar]

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


Fix it, then Ship it!




Thanks for the patience Jose. I would be committing this shortly with the 
following small fixes.


src/scheduler/constants.hpp (line 27)


Missing period at the end.



src/scheduler/scheduler.cpp (line 465)


s/connection_delay_max/connectionDelayMax



src/scheduler/scheduler.cpp (line 469)


Nit: Newline before



src/scheduler/scheduler.cpp (line 471)


Nit: Newline before



src/tests/mesos.cpp (line 86)


Insert an extra new line here.


- Anand Mazumdar


On June 20, 2016, 9:38 a.m., Jose Guilherme Vanz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48387/
> ---
> 
> (Updated June 20, 2016, 9:38 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5359
> https://issues.apache.org/jira/browse/MESOS-5359
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The scheduler library has been updated to wait a little deley before
> initiate a connection with the master. The maximum amount of time waited
> by the scheduler is defined by a flag: CONNECTION_DELAY_MAX. After
> the master has been detected the scheduler picks a random delay that
> can be between 0 and the CONNECTION_DELAY_MAX value. MESOS-5359
> 
> 
> Diffs
> -
> 
>   src/Makefile.am a4931560f1a5b3fbe41ea181477341d3ac459b58 
>   src/scheduler/constants.hpp PRE-CREATION 
>   src/scheduler/flags.hpp PRE-CREATION 
>   src/scheduler/scheduler.cpp c79837c93e7329dbfa22e4288b44237f33408ba9 
>   src/tests/http_fault_tolerance_tests.cpp 
> baa07395b9bd588daa5438369954584787d7952a 
>   src/tests/master_maintenance_tests.cpp 
> a8649fdf3cbd4996d1528393506b1b0ed0a7cf5c 
>   src/tests/mesos.hpp 6bd3a708db450eae21bd5c59f74c811bf87b3c89 
>   src/tests/mesos.cpp e7792fde39a60536f9651f75f1cbfa09b28651a9 
> 
> Diff: https://reviews.apache.org/r/48387/diff/
> 
> 
> Testing
> ---
> 
> mesos-tests
> 
> 
> Thanks,
> 
> Jose Guilherme Vanz
> 
>