You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Andrei Sekretenko <as...@mesosphere.io> on 2020/02/07 17:31:32 UTC

Review Request 72090: Introduced a weak_ptr-based cache abstraction.

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

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


Re: Review Request 72090: Introduced a weak_ptr-based cache abstraction.

Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
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
> 
>