mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Qian Zhang <zhq527...@gmail.com>
Subject Re: Review Request 72779: Initialized plugins lazily in the CSI server.
Date Wed, 19 Aug 2020 07:43:08 GMT

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




src/slave/csi_server.cpp
Lines 130-132 (patched)
<https://reviews.apache.org/r/72779/#comment310706>

    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?



src/slave/csi_server.cpp
Line 127 (original), 145 (patched)
<https://reviews.apache.org/r/72779/#comment310707>

    When is this hashmap populated? I see what is populated in `CSIServerProcess::initializePlugin()`
is a local variable with the exactly same name.



src/slave/csi_server.cpp
Line 132 (original), 149 (patched)
<https://reviews.apache.org/r/72779/#comment310709>

    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.



src/slave/csi_server.cpp
Line 136 (original), 155 (patched)
<https://reviews.apache.org/r/72779/#comment310708>

    If `name` is specified, do we need to do this `os::ls`? I think we can just load and initialize
the specified plugin directly without traversing the entries in `pluginConfigDir`, right?


- Qian Zhang


On Aug. 19, 2020, 2:23 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, 2:23 p.m.)
> 
> 
> Review request for mesos and Qian Zhang.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch updates the CSI server to attempt to
> initialize unknown plugins when it receives publish
> or unpublish calls meant for plugins that it does
> not recognize.
> 
> 
> Diffs
> -----
> 
>   src/slave/csi_server.hpp f5ec7663926731483589c5e30e7060751ed01e55 
>   src/slave/csi_server.cpp 2ba4f22b92370b019722d845d6113fb37f1b876a 
> 
> 
> Diff: https://reviews.apache.org/r/72779/diff/2/
> 
> 
> Testing
> -------
> 
> `sudo bin/mesos-tests.sh --gtest_filter="*CSIServerTest*"`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


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