Deprecate db delete file public API #13284
Conversation
|
@mszeszko-meta has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@mszeszko-meta has updated the pull request. You must reimport the pull request before landing. |
1 similar comment
|
@mszeszko-meta has updated the pull request. You must reimport the pull request before landing. |
|
@mszeszko-meta has updated the pull request. You must reimport the pull request before landing. |
mszeszko-meta
force-pushed
the
deprecate_db_delete_file_public_api
branch
from
22f7123 to
edcd146
Compare
|
@mszeszko-meta has updated the pull request. You must reimport the pull request before landing. |
mszeszko-meta
changed the title
|
@mszeszko-meta has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
mszeszko-meta
force-pushed
the
deprecate_db_delete_file_public_api
branch
from
edcd146 to
1b973a2
Compare
|
@mszeszko-meta has updated the pull request. You must reimport the pull request before landing. |
|
@mszeszko-meta has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
| @@ -1768,8 +1768,6 @@ class DB { | |||
| const TransactionLogIterator::ReadOptions& read_options = | |||
| TransactionLogIterator::ReadOptions()) = 0; | |||
|
|
|||
| // Windows API macro interference | |||
| #undef DeleteFile | |||
There was a problem hiding this comment.
Hooray! Though there's another above in db.h that should be removed.
(Can't remove the #undef DeleteFile in other files because of other symbols.
|
@mszeszko-meta has updated the pull request. You must reimport the pull request before landing. |
|
@mszeszko-meta has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@mszeszko-meta merged this pull request in 6e97a81. |
Summary:
We added a removal warning for public
DB::DeleteFileAPI ~4 years ago in #7337. This API seems to sit at wrong layer of abstraction, where instead of exposing a clear interface to delete specific range of keys, callers rely on their own discovery / interpretation of where their data / log possibly resides 'as-of-now'. For example, in case of data, the physical location of the keys might very well change after user obtained their mapping from key(s) to specific SST file. This will lead toInvalidArgumentresponse, which if repeated, would put a user in a race condition spinning wheel - the behavior that's inefficient, fairly indeterministic and therefore one that should be strongly discouraged. We're employing a graceful approach to prefixing the public API withDEPRECATED_first for better discoverability and ease of self service for product teams should they still use that legacy API. If everything goes smoothly, we intend to remove all the deprecated API references in the next release.