Deflake ThreadStatus related unit tests

[cbi42]

Summary: Unit tests DBTest.ThreadStatusFlush and DBTestWithParam.ThreadStatusSingleCompaction have been flaky and fail with error message

[ RUN      ] DBTest.ThreadStatusFlush
op_count: 0, expected_count 1
thread id: 718113, thread status: , cf_name 
thread id: 718114, thread status: , cf_name pikachu
/__w/rocksdb/rocksdb/db/db_test.cc:4817: Failure
Value of: VerifyOperationCount(env_, ThreadStatus::OP_FLUSH, 1)
  Actual: false
Expected: true
[  FAILED  ] DBTest.ThreadStatusFlush (106 ms)


[ RUN      ] DBTestWithParam/DBTestWithParam.ThreadStatusSingleCompaction/0
db/db_test.cc:4673: Failure
Expected equality of these values:
  op_count
    Which is: 0
  expected_count
    Which is: 1
[  FAILED  ] DBTestWithParam/DBTestWithParam.ThreadStatusSingleCompaction/0, where GetParam() = (1, false) 

One cause for this is that before flush/compaction finishes, we will go through ~WritableFileWriter(), either for WAL or SST file, and temporarily set thread_operation to UNKNOWN. This UNKNOWN thread operation seem to be there for some stress test verification. This PR fixes these tests by setting the IOActivity in ~WritableFileWriter() for debug build.

Test plan: monitor future test failure.

[facebook-github-bot]

@cbi42 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[hx235]

We kind of rely on NOT having this exception "options.io_activity == Env::IOActivity::kUnknown" to find incorrectly passed in IOOptions.... Is there anyway to fix the UT instead? By the way I like the refactory.

[cbi42]

I could change the sync point used in unit test to make it work. But I think it makes sense to keep the thread_operation as Flush/Compaction instead of setting it to UNKNOWN. I assume that setting the thread_operation to UNKNOWN is only there to pass the checks in stress test. One alternative is to set the IOActivity in ~WritableFileWriter() based on thread_operation, what do you think?

[hx235]

But I think it makes sense to keep the thread_operation as Flush/Compaction instead of setting it to UNKNOWN

Right unfortunately we can't pass parameter into destructor

I assume that setting the thread_operation to UNKNOWN is only there to pass the checks in stress test.

Right

One alternative is to set the IOActivity in ~WritableFileWriter() based on thread_operation, what do you think?

Yeah without refactoring the cleanup out of destructor you may do that under dbg mode.

[cbi42]

Updated to set IOActivity in destructor.

[facebook-github-bot]

@cbi42 has updated the pull request. You must reimport the pull request before landing.

[facebook-github-bot]

@cbi42 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[facebook-github-bot]

@cbi42 merged this pull request in b800b5e.