mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joseph Wu <jos...@mesosphere.io>
Subject Re: Review Request 69064: Added unit tests for Master HTTP endpoints.
Date Sat, 01 Dec 2018 01:06:16 GMT

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



I've given a shot at making the `MasterLoadTest.SimultaneousBatchedRequests` deterministic.
 My goal is to get the master backlogged on purpose, because when the master's backlog looks
like:
```
1) HTTP GET /state
2) HTTP GET /state
3) HTTP GET /state
4) HTTP GET /state
5) HTTP GET /state
6) <Other event>
```
The master will be forced to process the 5 HTTP GETs in a row, which leads to 5 dispatches
to the `ObjectApprover::create()`, which are chained to `deferBatchedRequest`.  The `ObjectApprover::create()`
runs on the Authorizer actor.  We can tweak the Authorizer to wait for X number of `getObjectApprover`
calls before it returns (all at once).  If we do this correctly, the master's backlog should
then look like:
```
1) <Event blocking the master from the test body>
2) deferBatchedRequest(/state)
3) deferBatchedRequest(/state)
4) deferBatchedRequest(/state)
5) deferBatchedRequest(/state)
6) deferBatchedRequest(/state)
```
At which point, the test can unblock the (1) event and let the master process each batched
request.  The call to `processRequestsBatch` would be scheduled after all the `deferBatchedRequest`s,
so there should be 4 cache hits.  You can use the metric added in the next review to verify
the cache hits.

I'll post the major additions/changes below, roughly in-line below:


src/tests/master_load_tests.cpp
Lines 73-101 (patched)
<https://reviews.apache.org/r/69064/#comment295820>

    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;
    };
    ```



src/tests/master_load_tests.cpp
Lines 152-155 (patched)
<https://reviews.apache.org/r/69064/#comment295821>

    I replaced this with a
    `BlockingAuthorizer authorizer;`



src/tests/master_load_tests.cpp
Lines 186-198 (patched)
<https://reviews.apache.org/r/69064/#comment295824>

    In my attempt, I did not try to launch multiple requests on multiple threads.  Instead,
I (messily/hackily/lazily) just sent all the requests in the test body.
    
    ```
      // Send all the HTTP requests.
      std::vector<const char*> endpoints {
          "/state", "/state-summary", "/frameworks", "/slaves", "/roles",
          "/state", "/state-summary", "/frameworks", "/slaves", "/roles",
          "/state", "/state-summary", "/frameworks", "/slaves", "/roles",
          "/state", "/state-summary", "/frameworks", "/slaves", "/roles",
          "/state", "/state-summary", "/frameworks", "/slaves", "/roles",
          "/state", "/state-summary", "/frameworks", "/slaves", "/roles",
          "/state", "/state-summary", "/frameworks", "/slaves", "/roles",
          "/state", "/state-summary", "/frameworks", "/slaves", "/roles",
          "/state", "/state-summary", "/frameworks", "/slaves", "/roles",
          "/state", "/state-summary", "/frameworks", "/slaves", "/roles",
      };
    
      process::http::Headers headers = createBasicAuthHeaders(DEFAULT_CREDENTIAL);
      std::vector<Future<Response>> responses;
      foreach (const char* endpoint, endpoints) {
        responses.push_back(
            process::http::get(master.get()->pid, endpoint, "", headers));
      }
    ```
    
    If we write the other parts of the test correctly, it shouldn't matter how the requests
are made.



src/tests/master_load_tests.cpp
Lines 200-203 (patched)
<https://reviews.apache.org/r/69064/#comment295825>

    This is how I ensure there will be some cache hits:
    ```
      // Wait until all the HTTP events have reached the master and are now
      // awaiting authorization.  There might be some other requests that get
      // mixed into the authorizer, so we must have ample requests in the
      // test body to ensure cache hits.
      Future<size_t> pendingHttpCalls;
      do {
        pendingHttpCalls = authorizer.pending();
        AWAIT_READY(pendingHttpCalls);
      } while (pendingHttpCalls.get() < 50u);
    ```
    
    Note: This method of blocking the master means you cannot pause the clock, as that may
deadlock the master process when the `await()` spawns a latch and then `ProcessManager::settle()`s.
 The `settle()` will never complete since the master is "running".
    Using a non-libprocess way of blocking would not deadlock.
    ```
      // Now block the master actor, since we don't want the master to start
      // batching until it is queued up with all the HTTP requests again.
      Promise<Nothing> masterBlocker;
      process::dispatch(master.get()->pid, [&masterBlocker]() {
        masterBlocker.future().await();
      });
    ```
    
    This part is still a little racy, in terms of the exact number of requests that make it
onto the master's queue before we un-gate the Authorizer.  
    ```
      // Unblock the BlockingAuthorizer.
      // This should trigger all the deferals onto the master from the Authorizer's
      // thread. When this future completes, the master's queue should be full
      // of batched requests.
      AWAIT_READY(authorizer.unleash());
    
      // Unblock the master now, so it can perform the batching.
      masterBlocker.set(Nothing());
    
      // Unblock the master and let the batched HTTP handler do its magic.
      AWAIT_READY(process::collect(responses));
    ```
    
    After this, you can check the response as you've done.  Plus check cache hits.


- Joseph Wu


On Nov. 21, 2018, 5:22 p.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69064/
> -----------------------------------------------------------
> 
> (Updated Nov. 21, 2018, 5:22 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 8da1a05b618f17542fec9b5057484a9bae57658a 
>   src/tests/master_load_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/69064/diff/3/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


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