Fix FIFO Compaction with TTL tests #2650
Conversation
db/db_test.cc
Outdated
| // sleep for 5 seconds | ||
| env_->SleepForMicroseconds(5 * 1000 * 1000); | ||
| // sleep for 2 seconds | ||
| env_->SleepForMicroseconds(2 * 1000 * 1000); |
There was a problem hiding this comment.
What this sleep waiting for? Will it work if we remove the sleep?
There was a problem hiding this comment.
No, the test will not work if the the sleep is removed. This sleep is waiting for the ttl to expire.
On line 2863, I initially set the TTL to 1 hour (arbitrary), and created all the files. On line 2885, I then change the TTL to 1 second ... but since at least 2 seconds have passed by now due to sleeping, all the files are ready for compaction/deletion because of ttl expriy. Line 2889 shows that all the files are deleted.
MemEnv provides a method to mock sleep by doing a FakeSleepForMicroseconds. I wish I could use it to sleep longer to mimic realistic TTL values, say, like 1 day or 1 week. But I couldn't yet get FakeSleep to work even after spending considerable time and effort.
There was a problem hiding this comment.
I see. Put a inline comment here?
There was a problem hiding this comment.
Good idea, will add a comment.
There was a problem hiding this comment.
Can we use the simulated clock instead?
https://github.com/facebook/rocksdb/blob/master/db/db_test_util.h#L478-L480
There was a problem hiding this comment.
Oh, didn't know about this. Sure, let me try. It'd be great if this works.
There was a problem hiding this comment.
Done. Able to fake sleep now, based on Siying's suggestion.
|
@sagar0 updated the pull request - view changes - changes since last import |
|
@sagar0 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
|
@sagar0 updated the pull request - view changes - changes since last import |
|
@sagar0 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
Test Plan:
make check