mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Mahler <bmah...@apache.org>
Subject Re: Review Request 72090: Introduced a weak_ptr-based cache abstraction.
Date Sun, 09 Feb 2020 19:52:09 GMT

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



I'm a bit puzzled at why we need this: the value is handed out directly from the factory,
and the caching behavior this provides is that if a value is still in the hands of a caller,
another call will avoid calling the factory again. That seems like a pretty odd optimization
to have as opposed to just always calling the factory function?

In the context of authorization, can't we just call the factory function directly?

```
  return cache
    .get(
        {subject, action},
        [factory](const SubjectAction& subjectAction) {
          return factory->provideObjectApprover(subjectAction);
        })
    .then([](const std::pair<shared_ptr<const ObjectApprover>, bool>& r) {
      return r.first;
    });
```

Why can't this just be:

```
  return factory->provideObjectApprover(subjectAction);
```

- Benjamin Mahler


On Feb. 7, 2020, 5:31 p.m., Andrei Sekretenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72090/
> -----------------------------------------------------------
> 
> (Updated Feb. 7, 2020, 5:31 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Greg Mann.
> 
> 
> Bugs: MESOS-10056
>     https://issues.apache.org/jira/browse/MESOS-10056
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch adds a key-value cache that hands out shared pointers to
> cached items, but does not own them (hence, the items get deleted
> as soon as they are no longer referenced by cache users).
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/Makefile.am 82ae581583e820f079ac4c361d57af12493b9f3a 
>   3rdparty/libprocess/include/Makefile.am e1a6f1ea9f1bf7412a401ab6a5fe81f9d896c32c 
>   3rdparty/libprocess/include/process/weak_cache.hpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/CMakeLists.txt b4ec9907d16c89b45562f4fa33c9f3d2913f6991

>   3rdparty/libprocess/src/tests/weak_cache_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/72090/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>


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