> On Aug. 30, 2017, 9:45 p.m., Anand Mazumdar wrote: > > src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp > > Lines 360 (patched) > > <https://reviews.apache.org/r/61991/diff/1/?file=1807837#file1807837line360> > > > > hmm, I think we should only log things that are relevant to the > > business logic of the adapter itself and different than the already > > existing logic of the driver it already wraps. > > > > In this case, this should already be logged by the v0 driver > > implementation. Hence, I don't see much utility in double logging it again > > here?
I think there is value in this log entry for two reasons: 1. The driver logs the `registered` event, while we log here that we are about to send the `connected` event. 2. The presence of the log entry makes it clear that there is an adapter behind the scenes and how it works. Since it is not a periodical event, I would argue adding this log entry adds value. > On Aug. 30, 2017, 9:45 p.m., Anand Mazumdar wrote: > > src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp > > Lines 408 (patched) > > <https://reviews.apache.org/r/61991/diff/1/?file=1807837#file1807837line408> > > > > We should certainly log this as this is pertaining to the business > > logic of the adapter itself. > > > > > > Also mention the reason i.e., we are dropping these pending events due > > to a master disconnection? Sure. > On Aug. 30, 2017, 9:45 p.m., Anand Mazumdar wrote: > > src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp > > Lines 418 (patched) > > <https://reviews.apache.org/r/61991/diff/1/?file=1807837#file1807837line418> > > > > Ditto as above. We shouldn't double log here. Why double log? IIUC, `SchedulerProcess::detected()` does not log before disconnecting the scheduler. Moreover, even if it would, I'm convinced there is value in this log entry to show the reader how the adapter reacted to the disconnected event because connected-disconnected and registered/disconnected logics are not symmetrical. - Alexander ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61991/#review184205 ----------------------------------------------------------- On Aug. 30, 2017, 2:14 p.m., Alexander Rukletsov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/61991/ > ----------------------------------------------------------- > > (Updated Aug. 30, 2017, 2:14 p.m.) > > > Review request for mesos, Anand Mazumdar, Till Toenshoff, and Vinod Kone. > > > Repository: mesos > > > Description > ------- > > See summary. > > > Diffs > ----- > > src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp > 1f58fbff4e8414e4d2ae4c8f69b637ee3315e411 > > > Diff: https://reviews.apache.org/r/61991/diff/1/ > > > Testing > ------- > > None: Not a functional change. > > > Thanks, > > Alexander Rukletsov > >
