mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Mann <g...@mesosphere.io>
Subject Re: Review Request 72779: Initialized plugins lazily in the CSI server.
Date Wed, 19 Aug 2020 19:04:29 GMT


> On Aug. 19, 2020, 7:43 a.m., Qian Zhang wrote:
> > src/slave/csi_server.cpp
> > Lines 130-132 (patched)
> > <https://reviews.apache.org/r/72779/diff/2/?file=2238466#file2238466line132>
> >
> >     I see this method will always be called with name specifed in `CSIServerProcess::publishVolume()`
and `CSIServerProcess::unpublishVolume()`. So in which case will it be called with no name
specified?

Sorry I forgot to include the call to `initializePlugin()` in `CSIServerProcess::start()`.


> On Aug. 19, 2020, 7:43 a.m., Qian Zhang wrote:
> > src/slave/csi_server.cpp
> > Line 132 (original), 149 (patched)
> > <https://reviews.apache.org/r/72779/diff/2/?file=2238466#file2238466line152>
> >
> >     So basically this method does two things: load the plugin config files and initialize
the plugins. I would suggest we still do the former in `CSIServer::create()` so if there is
any errors (like `JSON::parse()` or `protobuf::parse()` fails) in the plugin config files
we can error out eariler, otherwise users may only find the error in the runtime and they
have to fix the error with restarting agent.

Ah yea sorry, I forgot to include a call to `initializePlugin()` in `CSIServerProcess::start()`.
This will read any configs in the directory and return a Failure immediately if any of them
are invalid.

I could take care of this in `CSIServer::create()`, but I think it's a bit nicer from a code
organization perspective to keep all of the config handling in `CSIServerProcess`. But if
you think it would be better to handle this in `create()` rather than `start()`, that would
be possible.


- Greg


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


On Aug. 19, 2020, 7:04 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72779/
> -----------------------------------------------------------
> 
> (Updated Aug. 19, 2020, 7:04 p.m.)
> 
> 
> Review request for mesos and Qian Zhang.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Initialized plugins lazily in the CSI server.
> 
> 
> Diffs
> -----
> 
>   src/slave/csi_server.hpp f5ec7663926731483589c5e30e7060751ed01e55 
>   src/slave/csi_server.cpp 2ba4f22b92370b019722d845d6113fb37f1b876a 
> 
> 
> Diff: https://reviews.apache.org/r/72779/diff/3/
> 
> 
> Testing
> -------
> 
> `sudo bin/mesos-tests.sh --gtest_filter="*CSIServerTest*"`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


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