-
Notifications
You must be signed in to change notification settings - Fork 170
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Store blob storage ptr in column family handle to avoid mutex #298
Conversation
Signed-off-by: Yang Zhang <[email protected]>
Signed-off-by: Yang Zhang <[email protected]>
Signed-off-by: Yang Zhang <[email protected]>
tools/db_bench_tool.cc
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Irrelevant to #294, but need. to be checked in to get my cmake and compiler happy.
CMakeLists.txt
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Irrelevant to #294, but need. to be checked in to get my cmake and compiler happy.
@@ -355,21 +355,8 @@ TEST_F(TitanDBTest, Open) { | |||
background_job_started = true; | |||
assert(false); | |||
}); | |||
SyncPoint::GetInstance()->SetCallBack( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test case does not have any special meaning, I guess. The new change breaks this, because by the time "BeforeOpenBlobFileSet", default_cf_handle_
is not ready yet. We have other means to fix it, but the most graceful way is to remove it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
checked_before_blob_file_set
is unused now, please remove it
@@ -139,7 +139,7 @@ class CheckpointTest : public testing::Test { | |||
|
|||
void Close() { | |||
for (auto h : handles_) { | |||
delete h; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does not follow the API protocol. And the new change breaks these tests due to the complicity of handling default_cf_handle_
's lifetime.
Need to merge tikv/rocksdb#353 in advance. |
Signed-off-by: Yang Zhang <[email protected]>
Signed-off-by: Yang Zhang <[email protected]>
aec5d29
to
f66ad99
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Signed-off-by: Yang Zhang <[email protected]>
952bfae
to
842ec98
Compare
Ref #294
Store a shared ptr of
BlobStorage
inColumnFamilyHandle
, so that, while reading, Titan does not need to lock and fetch it every time.