mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benno Evers <bev...@mesosphere.com>
Subject Re: Review Request 69064: Added unit tests for Master HTTP endpoints.
Date Wed, 12 Dec 2018 21:00:50 GMT


> On Dec. 1, 2018, 1:06 a.m., Joseph Wu wrote:
> > src/tests/master_load_tests.cpp
> > Lines 73-101 (patched)
> > <https://reviews.apache.org/r/69064/diff/3/?file=2109726#file2109726line73>
> >
> >     In terms of helpers, I added a replacement Authorizer:
> >     ```
> >     // This authorizer is like the `MockAuthorizer`, which returns an
> >     // `AcceptingObjectApprover` for every authorization request, i.e.
> >     // everybody is authorized to view everything by default.
> >     //
> >     // The difference is that this authorizer will not satisfy any futures
> >     // from `getObjectApprover` until it is told to, presumably from the test body.
> >     // The class is basically a giant gate for certain requests.
> >     class BlockingAuthorizerProcess
> >       : public process::Process<BlockingAuthorizerProcess>
> >     {
> >     public:
> >       BlockingAuthorizerProcess()
> >         : ProcessBase(process::ID::generate("blocking-authorizer")),
> >           blocked(true) {}
> >     
> >       // NOTE: Should be unused in this test.
> >       Future<bool> authorized(const authorization::Request& request)
> >       {
> >         return true;
> >       }
> >     
> >       Future<Owned<ObjectApprover>> getObjectApprover(
> >           const Option<authorization::Subject>& subject,
> >           const authorization::Action& action)
> >       {
> >         if (blocked) {
> >           promises.emplace();
> >           return promises.back().future();
> >         }
> >     
> >         return Owned<ObjectApprover>(new AcceptingObjectApprover());
> >       }
> >     
> >       // Returns the number of pending calls to `getObjectApprover`.
> >       Future<size_t> pending()
> >       {
> >         return promises.size();
> >       }
> >     
> >       // Satisfies all future and prior calls made to `getObjectApprover`.
> >       Future<Nothing> unleash()
> >       {
> >         while (!promises.empty()) {
> >           promises.front().set(
> >               Owned<ObjectApprover>(new AcceptingObjectApprover()));
> >     
> >           promises.pop();
> >         }
> >     
> >         blocked = false;
> >     
> >         return Nothing();
> >       }
> >     
> >     private:
> >       std::queue<Promise<Owned<ObjectApprover>>> promises;
> >       bool blocked;
> >     };
> >     
> >     
> >     class BlockingAuthorizer : public Authorizer
> >     {
> >     public:
> >       BlockingAuthorizer()
> >         : process(new BlockingAuthorizerProcess())
> >       {
> >         process::spawn(process.get());
> >       }
> >     
> >       ~BlockingAuthorizer()
> >       {
> >         process::terminate(process.get());
> >         process::wait(process.get());
> >       }
> >     
> >       Future<bool> authorized(const authorization::Request& request) override
> >       {
> >         return process::dispatch(
> >             process.get(),
> >             &BlockingAuthorizerProcess::authorized,
> >             request);
> >       }
> >     
> >       Future<Owned<ObjectApprover>> getObjectApprover(
> >           const Option<authorization::Subject>& subject,
> >           const authorization::Action& action) override
> >       {
> >         return process::dispatch(
> >             process.get(),
> >             &BlockingAuthorizerProcess::getObjectApprover,
> >             subject,
> >             action);
> >       }
> >     
> >       Future<size_t> pending()
> >       {
> >         return process::dispatch(
> >             process.get(),
> >             &BlockingAuthorizerProcess::pending);
> >       }
> >     
> >       Future<Nothing> unleash()
> >       {
> >         return process::dispatch(
> >             process.get(),
> >             &BlockingAuthorizerProcess::unleash);
> >       }
> >     
> >     private:
> >       Owned<BlockingAuthorizerProcess> process;
> >     };
> >     ```

Thanks! I've implemented this approach, with some modifications that turned out to be necessary.

I still didn't use the new metric, due to this part:

    Future<size_t> pendingHttpCalls;
    do {
      pendingHttpCalls = authorizer.pending();
      AWAIT_READY(pendingHttpCalls);
    } while (pendingHttpCalls.get() < 50u);
    
still ultimately being racy (since other things besides HTTP requests are calling into the
authorizer), so actually asserting a cache hit still seems like it would be introducing a
flaky test.

However, the likelihood of "no batching" happenning is probably at least an order of magnitude
lower than in the previous version, so overall it seems like a useful change.


- Benno


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


On Dec. 12, 2018, 8:53 p.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69064/
> -----------------------------------------------------------
> 
> (Updated Dec. 12, 2018, 8:53 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This commit adds a set of unit test to verify that
> basic Master HTTP endpoints still work correctly
> under the presence of request caching.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 35500516e18ff251357b9e8d6bba894c44a9253b 
>   src/tests/master_load_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/69064/diff/4/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


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