> On July 12, 2014, 12:08 a.m., Jie Yu wrote:
> > src/examples/low_level_scheduler_pthread.cpp, line 244
> > <https://reviews.apache.org/r/23216/diff/3/?file=624436#file624436line244>
> >
> >     Why sleep(1) here?

We do not want to send the REGISTER Calls too frequently.


> On July 12, 2014, 12:08 a.m., Jie Yu wrote:
> > src/examples/low_level_scheduler_pthread.cpp, lines 283-286
> > <https://reviews.apache.org/r/23216/diff/3/?file=624436#file624436line283>
> >
> >     Ditto.


> On July 12, 2014, 12:08 a.m., Jie Yu wrote:
> > src/examples/low_level_scheduler_pthread.cpp, lines 325-327
> > <https://reviews.apache.org/r/23216/diff/3/?file=624436#file624436line325>
> >
> >     No need to protect framework.


> On July 12, 2014, 12:08 a.m., Jie Yu wrote:
> > src/examples/low_level_scheduler_pthread.cpp, lines 372-374
> > <https://reviews.apache.org/r/23216/diff/3/?file=624436#file624436line372>
> >
> >     You don't really need to protect 'framework' here. Although all 
> > callbacks are executed using async(), but they are guarded by mutex.lock in 
> > src/scheduler/scheduler.cpp, so all the callbacks will be effectively 
> > executed serially. If 'framework' is only accessed by these callbacks, you 
> > don't need a lock.

Yeah, you are right once we solved the above race condition.


> On July 12, 2014, 12:08 a.m., Jie Yu wrote:
> > src/examples/low_level_scheduler_pthread.cpp, lines 231-236
> > <https://reviews.apache.org/r/23216/diff/3/?file=624436#file624436line231>
> >
> >     A subtle race condition here:
> >     
> >     1) the control reaches right before pthread_cond_wait(...);
> >     
> >     2) connected callback is called, and pthread_cond_signal(..) is called.
> >     
> >     3) pthread_cond_wait(...) is called.
> >     
> >     Looks like a deadlock bug to me.

Good catch! The new CONNECTED state is used once connected() is called.


- Zuyu


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


On July 11, 2014, 8:55 p.m., Zuyu Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23216/
> -----------------------------------------------------------
> 
> (Updated July 11, 2014, 8:55 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, Jie Yu, and Vinod 
> Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Added the low level scheduler example using pthread.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 45afcd145f3b502043424a6dac2197979aefbca2 
>   src/examples/low_level_scheduler_pthread.cpp PRE-CREATION 
>   src/tests/examples_tests.cpp 2b554d72f0058a68f589719373f3d3e37a3a7ba3 
>   src/tests/low_level_scheduler_pthread_test.sh PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/23216/diff/
> 
> 
> Testing
> -------
> 
> [==========] Running 1 test from 1 test case.
> [----------] Global test environment set-up.
> [----------] 1 test from ExamplesTest
> [ RUN      ] ExamplesTest.LowLevelSchedulerPthread
> [       OK ] ExamplesTest.LowLevelSchedulerPthread (1655 ms)
> [----------] 1 test from ExamplesTest (1655 ms total)
> 
> [----------] Global test environment tear-down
> [==========] 1 test from 1 test case ran. (1669 ms total)
> [  PASSED  ] 1 test.
> 
> 
> Thanks,
> 
> Zuyu Zhang
> 
>

Reply via email to