BlobDB: is_fifo=true also evict non-TTL blob files #4049
Conversation
There was a problem hiding this comment.
@yiwu-arbug has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@yiwu-arbug has updated the pull request. View: changes, changes since last import |
There was a problem hiding this comment.
@yiwu-arbug has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
utilities/blob_db/blob_db_impl.cc
Outdated
| @@ -54,14 +54,7 @@ WalFilter::WalProcessingOption BlobReconcileWalFilter::LogRecordFound( | |||
|
|
|||
| bool blobf_compare_ttl::operator()(const std::shared_ptr<BlobFile>& lhs, | |||
There was a problem hiding this comment.
This name needs to be updated now that we are no longer using ttl to compare.
|
@yiwu-arbug has updated the pull request. View: changes, changes since last import |
There was a problem hiding this comment.
@yiwu-arbug has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
| @@ -373,7 +376,7 @@ class BlobDBImpl : public BlobDB { | |||
|
|
|||
| // all the blob files which are currently being appended to based | |||
| // on variety of incoming TTL's | |||
| std::set<std::shared_ptr<BlobFile>, blobf_compare_ttl> open_ttl_files_; | |||
There was a problem hiding this comment.
Almost make a mistake.. The TTL version of comparator is also needed here. I'm keeping it now.
There was a problem hiding this comment.
Oh, ok. Wow, at least updating the name to be consistent with the logic caught a sleeper-bug.
There was a problem hiding this comment.
yup.. not sure why tests not catching it.
There was a problem hiding this comment.
@yiwu-arbug is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Summary: Previously with is_fifo=true we only evict TTL file. Changing it to also evict non-TTL files from oldest to newest, after exhausted TTL files. Closes facebook#4049 Differential Revision: D8604597 Pulled By: yiwu-arbug fbshipit-source-id: bc4209ee27c1528ce4b72833e6f1e1bff80082c1
Summary:
Previously with is_fifo=true we only evict TTL file. Changing it to also evict non-TTL files from oldest to newest, after exhausted TTL files.
Test Plan:
updated blob_db_test