> On Sept. 27, 2018, 6:23 p.m., Vinod Kone wrote: > > src/java/jni/org_apache_mesos_v1_scheduler_V1Mesos.cpp > > Lines 294 (patched) > > <https://reviews.apache.org/r/68866/diff/1/?file=2092453#file2092453line294> > > > > Looks like you are doing `call` here instead of `send` so that you have > > a future to wait on? Skipping `send` which does validation and > > authentication seems wrong. Let's not do that. > > > > I would recommend putting a sleep in the client code instead of here > > for now. > > Alexander Rukletsov wrote: > But `call` does > [validation](https://github.com/apache/mesos/blob/6e21e94ddca5b776d44636fe3eba8500bf88dc25/src/scheduler/scheduler.cpp#L269-L270) > and > [authentication](https://github.com/apache/mesos/blob/6e21e94ddca5b776d44636fe3eba8500bf88dc25/src/scheduler/scheduler.cpp#L301) > too. Not sure I follow, Vinod. > > Greg Mann wrote: > Yea I think we have the necessary validation/authentication in the > `call()` path as well. This solution is nice because it will wait for as > little time as possible, up to 5 seconds, rather than always waiting for 5 > seconds. > > In fact, since this code will return early if a response is actually > received, I would feel comfortable with a slightly longer await - maybe 10 or > 20 seconds?
I didn't realize `call` was a newish interface method added to the library. At a quick glance I thought it was the continuation of `send` and an internal method. My bad. So I'm ok with doing a `call` here. Suggestion: The comment above doesn't have enough context for a future to understand why the library could be destructured. I would recommend expanding the comment a bit more explaining. For example "The JVM could potentially call JNI `finalize` if the Java scheduler nullified its reference to `V1Mesos` library immediately after sending TEARDOWN." Also, maybe link to MESOS-9274 for posterity. - Vinod ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68866/#review209074 ----------------------------------------------------------- On Sept. 27, 2018, 5:41 p.m., Alexander Rukletsov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/68866/ > ----------------------------------------------------------- > > (Updated Sept. 27, 2018, 5:41 p.m.) > > > Review request for mesos, Gastón Kleiman, Greg Mann, and Nicholas Parker. > > > Bugs: MESOS-9274 > https://issues.apache.org/jira/browse/MESOS-9274 > > > Repository: mesos > > > Description > ------- > > See summary. > > > Diffs > ----- > > src/java/jni/org_apache_mesos_v1_scheduler_V1Mesos.cpp > 2a5fbd79ac7bad933067cd96e38186849af8edc4 > > > Diff: https://reviews.apache.org/r/68866/diff/1/ > > > Testing > ------- > > `make check`on various Linux distro > > > Thanks, > > Alexander Rukletsov > >
