mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alexander Rukletsov <ruklet...@gmail.com>
Subject Re: Review Request 61991: Added several logs to the C++ part of the v1-v0 adapter.
Date Thu, 31 Aug 2017 06:53:33 GMT


> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message