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



src/tests/master_allocator_tests.cpp
<https://reviews.apache.org/r/31263/#comment127666>

    While the TearDown() avoids flakiness by ensuring that an allocator process 
doesn't exist after a test, it doesn't address cases where a master is 
restarted with a new allocator in the same test (e.g., 
FrameworkReregistersFirst and SlaveReregistersFirst) causing both old and new 
allocators to coexist at the same time and talking to the same master.
    
    I suggest to have methods on the test fixture that creates and deletes 
allocator explicitly (can be used by tests that restart master). The TearDown() 
will still delete any active allocator. For safety, ensure that only one 
allocator is active  at any given time.
    
    Does that make sense?
    
    ```
    template <typename T>
    class MasterAllocatorTest : public MesosTest
    {
    protected: 
      MasterAllocatorTest() : allocator(Null) {}
      
      Try<TestAllocator> create()
      {
        // We don't support multiple allocators because...
        if (allocator != NULL) {
          return Error("Multiple active allocators are not supported");
        }
        
        allocator = new TestAllocator(process::Owned<Allocator>(new T));
        return *allocator;
      }
      
      void destroy()
      {
        delete allocator;
        allocator = NULL;
      }
      
      virtual void TearDown()
      {
        destroy();
        MeosTest::TearDown();
      }
    
    private:
      TestAllocator* allocator;
    };
    
    ```


- Vinod Kone


On April 1, 2015, 2:27 p.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31263/
> -----------------------------------------------------------
> 
> (Updated April 1, 2015, 2:27 p.m.)
> 
> 
> Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen.
> 
> 
> Bugs: MESOS-2160
>     https://issues.apache.org/jira/browse/MESOS-2160
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> TestAllocator stores a pointer to a real allocator and takes over its 
> ownership. MasterAllocatorTest fixture stores the test allocator in turn.
> 
> 
> Diffs
> -----
> 
>   src/tests/containerizer.cpp 26b87ac6b16dfeaf84888e80296ef540697bd775 
>   src/tests/master_allocator_tests.cpp 
> 03a1bb8c92b44bc1ad1b5f5cff8d1fb971df2302 
>   src/tests/mesos.hpp 0e98572a62ae05437bd2bc800c370ad1a0c43751 
>   src/tests/resource_offers_tests.cpp 
> 882a9ff4d09aace486182828bf43b643b0d0c519 
>   src/tests/slave_recovery_tests.cpp 87f4a6aab27d142fa8eb7a6571f684a6ce59687e 
> 
> Diff: https://reviews.apache.org/r/31263/diff/
> 
> 
> Testing
> -------
> 
> make check (Mac OS 10.9.5, CentOS 7.0)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Reply via email to