[IOTracing PR1] Store FileSystemPtr object that contains FileSystem ptr #7180
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
akankshamahajan15
force-pushed
the
IO_Tracing
branch
3 times, most recently
from
0b64fd7
to
62a11f0
Compare
akankshamahajan15
changed the title
akankshamahajan15
force-pushed
the
IO_Tracing
branch
2 times, most recently
from
475092a
to
facec29
Compare
|
Passed IOTracer as argument in existing constructor instead of writing a new one as list of arguments was long in the constructors and BlockCacheTrace was already passed as nullptr in many test cases so I passed IOTracer=nullptr as well in those testcases. |
There was a problem hiding this comment.
@akankshamahajan15 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
anand1976
reviewed
Aug 10, 2020
db/compaction/compaction_job.cc
Outdated
| @@ -1564,7 +1565,8 @@ Status CompactionJob::OpenCompactionOutputFile( | |||
| &syncpoint_arg); | |||
| #endif | |||
| Status s; | |||
| IOStatus io_s = NewWritableFile(fs_, fname, &writable_file, file_options_); | |||
| IOStatus io_s = NewWritableFile(db_options_.fs.get(), fname, &writable_file, | |||
There was a problem hiding this comment.
Shouldn't we pass the right pointer from fs_ instead of db_options_.fs? Otherwise, this won't get traced.
There was a problem hiding this comment.
Right now, this is not getting traced. But since its a one line function to trace FileSystem::NewWritableFile so I will pass the pointer.
env/file_system_tracer.h
Outdated
| @@ -80,6 +79,9 @@ class FileSystemPtr { | |||
| } | |||
| } | |||
|
|
|||
| /* Returns the underlying File System pointer */ | |||
| FileSystem* get() const { return fs_.get(); } | |||
There was a problem hiding this comment.
This should be conditional on whether tracing is on or off. If its on, it should return fs_tracer_.get().
Summary: As part of the IOTracing project, this PR
1. Caches "FileSystemPtr" object(wrapper class that returns file system pointer based on tracing enabled) instead of "FileSystem" pointer.
2. FileSystemPtr object is created using FileSystem pointer and IOTracer
pointer.
3. IOTracer shared_ptr is created in DBImpl and it is passed to different classes through constructor.
4. When tracing is enabled through DB::StartIOTrace, FileSystemPtr
returns FileSystemTracingWrapper pointer for tracing purpose and when
it is disabled underlying FileSystem pointer is returned.
Test Plan: make check -j64
COMPILE_WITH_TSAN=1 make check -j64
Reviewers:
Subscribers:
Tasks:
Tags:
Tags:
akankshamahajan15
force-pushed
the
IO_Tracing
branch
from
facec29
to
c1f2d1f
Compare
|
@akankshamahajan15 has updated the pull request. You must reimport the pull request before landing. |
|
Addressed Comments |
There was a problem hiding this comment.
@akankshamahajan15 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@akankshamahajan15 merged this pull request in 1f9f630. |
akankshamahajan15
added a commit
to akankshamahajan15/rocksdb
that referenced
this pull request
Aug 13, 2020
…ok#7180 Summary: As part of the IOTracing project, this PR 1. Caches "FSSequentialFilePtr" object(wrapper class that returns file system pointer based on tracing enabled) instead of "FSSequentialFile" pointer. 2. FSSequentialFilePtr object is created using FSSequentialFile pointer and IOTracer in SequentialFileReader. 3. IOTracer shared_ptr is created in DBImpl and it is passed to different classes through constructor. 4. When tracing is enabled through DB::StartIOTrace, FSSequentialFilePtr object returns FSSequentialFileTracingWrapper pointer for tracing purpose and when it is disabled underlying FSSequentialFile pointer is returned. Test Plan: make check -j64 COMPILE_WITH_TSAN=1 make check -j64 Reviewers: Subscribers: Tasks: Tags:
codingrhythm
pushed a commit
to SafetyCulture/rocksdb
that referenced
this pull request
Mar 5, 2021
Summary:
As part of the IOTracing project, this PR
1. Caches "FileSystemPtr" object(wrapper class that returns file system pointer based on tracing enabled) instead of "FileSystem" pointer.
2. FileSystemPtr object is created using FileSystem pointer and IOTracer
pointer.
3. IOTracer shared_ptr is created in DBImpl and it is passed to different classes through constructor.
4. When tracing is enabled through DB::StartIOTrace, FileSystemPtr
returns FileSystemTracingWrapper pointer for tracing purpose and when
it is disabled underlying FileSystem pointer is returned.
Pull Request resolved: facebook#7180
Test Plan:
make check -j64
COMPILE_WITH_TSAN=1 make check -j64
Reviewed By: anand1976
Differential Revision: D22987117
Pulled By: akankshamahajan15
fbshipit-source-id: 6073617e4c2d5bc363914f3a1f55ae3b0a58fbf1
levichen94
pushed a commit
to bytedance/terarkdb
that referenced
this pull request
Jun 18, 2021
Summary:
As part of the IOTracing project, this PR
1. Caches "FileSystemPtr" object(wrapper class that returns file system pointer based on tracing enabled) instead of "FileSystem" pointer.
2. FileSystemPtr object is created using FileSystem pointer and IOTracer
pointer.
3. IOTracer shared_ptr is created in DBImpl and it is passed to different classes through constructor.
4. When tracing is enabled through DB::StartIOTrace, FileSystemPtr
returns FileSystemTracingWrapper pointer for tracing purpose and when
it is disabled underlying FileSystem pointer is returned.
Pull Request resolved: facebook/rocksdb#7180
Test Plan:
make check -j64
COMPILE_WITH_TSAN=1 make check -j64
Reviewed By: anand1976
Differential Revision: D22987117
Pulled By: akankshamahajan15
fbshipit-source-id: 6073617e4c2d5bc363914f3a1f55ae3b0a58fbf1
Signed-off-by: Changlong Chen <levisonchen@live.cn>
mm304321141
pushed a commit
to bytedance/terarkdb
that referenced
this pull request
Jun 23, 2021
Summary:
As part of the IOTracing project, this PR
1. Caches "FileSystemPtr" object(wrapper class that returns file system pointer based on tracing enabled) instead of "FileSystem" pointer.
2. FileSystemPtr object is created using FileSystem pointer and IOTracer
pointer.
3. IOTracer shared_ptr is created in DBImpl and it is passed to different classes through constructor.
4. When tracing is enabled through DB::StartIOTrace, FileSystemPtr
returns FileSystemTracingWrapper pointer for tracing purpose and when
it is disabled underlying FileSystem pointer is returned.
Pull Request resolved: facebook/rocksdb#7180
Test Plan:
make check -j64
COMPILE_WITH_TSAN=1 make check -j64
Reviewed By: anand1976
Differential Revision: D22987117
Pulled By: akankshamahajan15
fbshipit-source-id: 6073617e4c2d5bc363914f3a1f55ae3b0a58fbf1
Signed-off-by: Changlong Chen <levisonchen@live.cn>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Test Plan: make check -j64
COMPILE_WITH_TSAN=1 make check -j64