> 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
>
>
