• P
    Fix+clean up handling of mock sleeps (#7101) · 6ac1d25f
    Peter Dillinger 提交于
    Summary:
    We have a number of tests hanging on MacOS and windows due to
    mishandling of code for mock sleeps. In addition, the code was in
    terrible shape because the same variable (addon_time_) would sometimes
    refer to microseconds and sometimes to seconds. One test even assumed it
    was nanoseconds but was written to pass anyway.
    
    This has been cleaned up so that DB tests generally use a SpecialEnv
    function to mock sleep, for either some number of microseconds or seconds
    depending on the function called. But to call one of these, the test must first
    call SetMockSleep (precondition enforced with assertion), which also turns
    sleeps in RocksDB into mock sleeps. To also removes accounting for actual
    clock time, call SetTimeElapseOnlySleepOnReopen, which implies
    SetMockSleep (on DB re-open). This latter setting only works by applying
    on DB re-open, otherwise havoc can ensue if Env goes back in time with
    DB open.
    
    More specifics:
    
    Removed some unused test classes, and updated comments on the general
    problem.
    
    Fixed DBSSTTest.GetTotalSstFilesSize using a sync point callback instead
    of mock time. For this we have the only modification to production code,
    inserting a sync point callback in flush_job.cc, which is not a change to
    production behavior.
    
    Removed unnecessary resetting of mock times to 0 in many tests. RocksDB
    deals in relative time. Any behaviors relying on absolute date/time are likely
    a bug. (The above test DBSSTTest.GetTotalSstFilesSize was the only one
    clearly injecting a specific absolute time for actual testing convenience.) Just
    in case I misunderstood some test, I put this note in each replacement:
    // NOTE: Presumed unnecessary and removed: resetting mock time in env
    
    Strengthened some tests like MergeTestTime, MergeCompactionTimeTest, and
    FilterCompactionTimeTest in db_test.cc
    
    stats_history_test and blob_db_test are each their own beast, rather deeply
    dependent on MockTimeEnv. Each gets its own variant of a work-around for
    TimedWait in a mock time environment. (Reduces redundancy and
    inconsistency in stats_history_test.)
    
    Intended follow-up:
    
    Remove TimedWait from the public API of InstrumentedCondVar, and only
    make that accessible through Env by passing in an InstrumentedCondVar and
    a deadline. Then the Env implementations mocking time can fix this problem
    without using sync points. (Test infrastructure using sync points interferes
    with individual tests' control over sync points.)
    
    With that change, we can simplify/consolidate the scattered work-arounds.
    
    Pull Request resolved: https://github.com/facebook/rocksdb/pull/7101
    
    Test Plan: make check on Linux and MacOS
    
    Reviewed By: zhichao-cao
    
    Differential Revision: D23032815
    
    Pulled By: pdillinger
    
    fbshipit-source-id: 7f33967ada8b83011fb54e8279365c008bd6610b
    6ac1d25f
db_test.cc 223.9 KB