Add FileSystem wrapper classes for IO tracing. #7002

akankshamahajan15 · 2020-06-18T23:30:49Z

Summary: 1. Add the wrapper classes FileSystemTracingWrapper, FSSequentialFileTracingWrapper, FSRandomAccessFileTracingWrapper, FSWritableFileTracingWrapper, FSRandomRWFileTracingWrapper that forward the calls to underlying storage system and then pass the file operation information to IOTracer. IOTracer dumps the record in binary format for tracing.
2. Add the wrapper classes FileSystemPtr, FSSequentialFilePtr, FSRandomAccessFilePtr, FSWritableFilePtr and FSRandomRWFilePtr that overload operator-> and return ptr to underlying storage system or Tracing wrapper class based on enabling/disabling of IO tracing. These classes are added to bypass Tracing Wrapper classes when we disable tracing.
3. Add enums in trace.h that distinguish which options need to be added for different file operations(Read, close, write etc) as part of tracing record.

Test Plan: make check -j64

Reviewers:

Subscribers:

Tasks:

Tags:

facebook-github-bot

@akankshamahajan15 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

anand1976

Thanks for the PR!

anand1976 · 2020-06-22T16:41:00Z

env/file_system_tracer.cc

+    const std::string& fname, const FileOptions& file_opts,
+    std::unique_ptr<FSWritableFile>* result, IODebugContext* dbg) {
+  IOStatus s = target()->NewWritableFile(fname, file_opts, result, dbg);
+  IOTraceRecord io_record(Timestamp(), TraceType::kIOFileName,


It would be good to trace the latency of each call as well.

anand1976 · 2020-06-22T16:44:54Z

env/file_system_tracer.cc

+    std::unique_ptr<FSWritableFile>* result, IODebugContext* dbg) {
+  IOStatus s = target()->NewWritableFile(fname, file_opts, result, dbg);
+  IOTraceRecord io_record(Timestamp(), TraceType::kIOFileName,
+                          "NewWritableFile", s.ToString(), fname);


Would it be easier to use __func__ instead?

anand1976 · 2020-06-22T16:49:42Z

env/file_system_tracer.cc

+                                                     const IOOptions& options,
+                                                     IODebugContext* dbg) {
+  IOStatus s = target()->MultiRead(reqs, num_reqs, options, dbg);
+  IOTraceRecord io_record(Timestamp(), TraceType::kIOGeneral, "MultiRead",


It would be good to trace each request, as reqs is really an array of individual read requests.

anand1976 · 2020-06-22T16:50:52Z

env/file_system_tracer.cc

+                                              IODebugContext* dbg) {
+  IOStatus s = target()->Append(data, options, dbg);
+  IOTraceRecord io_record(Timestamp(), TraceType::kIOGeneral, "Append",
+                          s.ToString());


Can we trace length too (data.size())?

anand1976 · 2020-06-22T16:51:12Z

env/file_system_tracer.cc

+    IODebugContext* dbg) {
+  IOStatus s = target()->PositionedAppend(data, offset, options, dbg);
+  IOTraceRecord io_record(Timestamp(), TraceType::kIOOffset, "PositionedAppend",
+                          s.ToString(), offset);


Same as above - trace the length

anand1976 · 2020-06-22T16:51:58Z

env/file_system_tracer.cc

+                                             IODebugContext* dbg) {
+  IOStatus s = target()->Write(offset, data, options, dbg);
+  IOTraceRecord io_record(Timestamp(), TraceType::kIOOffset, "Write",
+                          s.ToString(), offset);


Trace the length

anand1976 · 2020-06-22T16:53:15Z

env/file_system_tracer.h

+
+  std::shared_ptr<FileSystem> operator->() const {
+    if (fs_tracer_ && io_tracer_ && io_tracer_->is_tracing_enabled()) {
+      return std::static_pointer_cast<FileSystem>(fs_tracer_);


Is an explicit cast required?

anand1976 · 2020-06-22T16:57:04Z

env/file_system_tracer.h

+      : fs_(fs), io_tracer_(nullptr), fs_tracer_(nullptr) {}
+
+  std::shared_ptr<FileSystem> operator->() const {
+    if (fs_tracer_ && io_tracer_ && io_tracer_->is_tracing_enabled()) {


If fs_tracer_ is non-null, it implies io_tracer_ is also non-null right? Just trying to reduce the number of checks in the fast path :)

zhichao-cao · 2020-06-22T17:14:12Z

env/file_system_tracer.cc

+                                                Slice* result, char* scratch,
+                                                IODebugContext* dbg) const {
+  IOStatus s = target()->Read(offset, n, options, result, scratch, dbg);
+  IOTraceRecord io_record(Timestamp(), TraceType::kIOOffsetAndLen, "Read",


When the API like Read, Append is called, are we able to know which file is accessed by this request?

For some cases we can. I am not sure about all because its being used at multiple instances. My next PR that i am working on is to cache FileSystemPtr and other Ptrs instead of storing FileSystem and other pointers. So while making those changes i will have better idea where can I pass the file name as part of IODebugContext. I will add the filename in the next PR.

I see, that will be great.

zhichao-cao · 2020-06-22T17:21:39Z

trace_replay/trace_replay.h

@@ -46,6 +46,14 @@ enum TraceType : char {
  kBlockTraceDataBlock = 9,
  kBlockTraceUncompressionDictBlock = 10,
  kBlockTraceRangeDeletionBlock = 11,
+  // IO Trace related types based on options that will be added in trace file.
+  kIOGeneral = 12,
+  kIOFileName = 13,


I'm thinking we can put all the queries into two categories: 1) file related (e.g., NewWritableFile, Sync, Close) and 2) data related (e.g., Read, Write, Append, Truncate). For all the queries, type, timestamp, filename are the standard contents in the record. The rest of the payload is depends on the query types. Otherwise, when we replay or analyze the trace in the further, we may not be able to use the trace records in a precise way.

Yes Make sense. I was thinking in terms of that but then in IOtracer I need to check what options to write and what not because even same Functions (like GetFileSize) in different wrappers have different options (One passes filename and one doesn't). I will check once again if I can make it consistent and pass missing options that way it would be more scalable.

Oh, I see. I did realize that the options make it more complex.

facebook-github-bot · 2020-06-29T19:49:30Z

@akankshamahajan15 has updated the pull request. Re-import the pull request

facebook-github-bot

@akankshamahajan15 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

akankshamahajan15 · 2020-06-29T19:52:20Z

Addressed comments

anand1976 · 2020-07-01T21:56:01Z

env/file_system_tracer.cc

+IOStatus FileSystemTracingWrapper::NewWritableFile(
+    const std::string& fname, const FileOptions& file_opts,
+    std::unique_ptr<FSWritableFile>* result, IODebugContext* dbg) {
+  uint64_t start_time = Timestamp();


For consistency sake, we can use StopWatch, like here https://github.com/facebook/rocksdb/blob/master/db/db_impl/db_impl_write.cc#L1404. What do you think?

StopWatch requires Env pointer and in file_system_tracer.cc I don't have Env.
It also needs statistics pointer for StopWatch.

I can use StopWatchNano also as it requires Env not Statistics. Should I create Env object with Env::Default() in file_system_tracer.cc?

Yes, StopWatchNano with Env::Default() should be okay. I'm not sure if using StopWatch/StopWatchNano will make it more portable, but rest of the codebase uses those, so I think its better to be consistent.

facebook-github-bot · 2020-07-07T23:31:00Z

@akankshamahajan15 has updated the pull request. Re-import the pull request

facebook-github-bot

@akankshamahajan15 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

akankshamahajan15 · 2020-07-07T23:36:18Z

Resolved Conflicts

facebook-github-bot · 2020-07-13T18:28:16Z

@akankshamahajan15 has updated the pull request. Re-import the pull request

Summary: 1. Add the wrapper classes FileSystemTracingWrapper, FSSequentialFileTracingWrapper, FSRandomAccessFileTracingWrapper, FSWritableFileTracingWrapper, FSRandomRWFileTracingWrapper that forward the calls to underlying storage system and then pass the file operation information to IOTracer. IOTracer dumps the record in binary format for tracing. 2. Add the wrapper classes FileSystemPtr, FSSequentialFilePtr, FSRandomAccessFilePtr, FSWritableFilePtr and FSRandomRWFilePtr that overload operator-> and return ptr to underlying storage system or Tracing wrapper class based on enabling/disabling of IO tracing. These classes are added to bypass Tracing Wrapper classes when we disable tracing. 3. Add enums in trace.h that distinguish which options need to be added for different file operations(Read, close, write etc) as part of tracing record. Test Plan: make check -j64 Reviewers: Subscribers: Tasks: Tags:

facebook-github-bot · 2020-07-13T18:33:00Z

@akankshamahajan15 has updated the pull request. Re-import the pull request

akankshamahajan15 · 2020-07-13T21:13:30Z

Addressed comments

anand1976

Thanks @akankshamahajan15 for addressing all the comments!

facebook-github-bot

@akankshamahajan15 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

facebook-github-bot · 2020-07-14T00:40:32Z

@akankshamahajan15 merged this pull request in d93bd3c.

Summary: 1. Add the wrapper classes FileSystemTracingWrapper, FSSequentialFileTracingWrapper, FSRandomAccessFileTracingWrapper, FSWritableFileTracingWrapper, FSRandomRWFileTracingWrapper that forward the calls to underlying storage system and then pass the file operation information to IOTracer. IOTracer dumps the record in binary format for tracing. 2. Add the wrapper classes FileSystemPtr, FSSequentialFilePtr, FSRandomAccessFilePtr, FSWritableFilePtr and FSRandomRWFilePtr that overload operator-> and return ptr to underlying storage system or Tracing wrapper class based on enabling/disabling of IO tracing. These classes are added to bypass Tracing Wrapper classes when we disable tracing. 3. Add enums in trace.h that distinguish which options need to be added for different file operations(Read, close, write etc) as part of tracing record. Pull Request resolved: facebook/rocksdb#7002 Test Plan: make check -j64 Reviewed By: anand1976 Differential Revision: D22127897 Pulled By: akankshamahajan15 fbshipit-source-id: 74cff58ce5661c9a3832dfaa52483f3b2d8565e0 Signed-off-by: Changlong Chen <levisonchen@live.cn>

facebook-github-bot added the CLA Signed label Jun 18, 2020

facebook-github-bot reviewed Jun 18, 2020

View reviewed changes

akankshamahajan15 mentioned this pull request Jun 18, 2020

Add the wrapper classes for FileSystem APIs for IO tracing. #6991

Closed

akankshamahajan15 requested review from anand1976 and zhichao-cao June 19, 2020 18:02

anand1976 reviewed Jun 22, 2020

View reviewed changes

zhichao-cao reviewed Jun 22, 2020

View reviewed changes

akankshamahajan15 force-pushed the io_tracing branch from 5765356 to 71d2667 Compare June 29, 2020 19:49

facebook-github-bot reviewed Jun 29, 2020

View reviewed changes

anand1976 reviewed Jul 1, 2020

View reviewed changes

akankshamahajan15 force-pushed the io_tracing branch from 71d2667 to 516e5f8 Compare July 7, 2020 23:30

facebook-github-bot reviewed Jul 7, 2020

View reviewed changes

akankshamahajan15 force-pushed the io_tracing branch from 516e5f8 to 907bc18 Compare July 13, 2020 18:28

akankshamahajan15 force-pushed the io_tracing branch from 907bc18 to 450c12d Compare July 13, 2020 18:32

anand1976 approved these changes Jul 13, 2020

View reviewed changes

facebook-github-bot reviewed Jul 13, 2020

View reviewed changes

facebook-github-bot closed this in d93bd3c Jul 13, 2020

facebook-github-bot added the Merged label Jul 14, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add FileSystem wrapper classes for IO tracing. #7002

Add FileSystem wrapper classes for IO tracing. #7002

akankshamahajan15 commented Jun 18, 2020

facebook-github-bot left a comment

anand1976 left a comment

anand1976 Jun 22, 2020

anand1976 Jun 22, 2020

anand1976 Jun 22, 2020

anand1976 Jun 22, 2020

anand1976 Jun 22, 2020

anand1976 Jun 22, 2020

anand1976 Jun 22, 2020

anand1976 Jun 22, 2020

zhichao-cao Jun 22, 2020 •

edited

Loading

akankshamahajan15 Jun 29, 2020

zhichao-cao Jun 29, 2020

zhichao-cao Jun 22, 2020

akankshamahajan15 Jun 25, 2020

zhichao-cao Jun 25, 2020

facebook-github-bot commented Jun 29, 2020

facebook-github-bot left a comment

akankshamahajan15 commented Jun 29, 2020

anand1976 Jul 1, 2020

akankshamahajan15 Jul 7, 2020

anand1976 Jul 8, 2020

facebook-github-bot commented Jul 7, 2020

facebook-github-bot left a comment

akankshamahajan15 commented Jul 7, 2020

facebook-github-bot commented Jul 13, 2020

facebook-github-bot commented Jul 13, 2020

akankshamahajan15 commented Jul 13, 2020

anand1976 left a comment

facebook-github-bot left a comment

facebook-github-bot commented Jul 14, 2020

Add FileSystem wrapper classes for IO tracing. #7002

Add FileSystem wrapper classes for IO tracing. #7002

Conversation

akankshamahajan15 commented Jun 18, 2020

facebook-github-bot left a comment

Choose a reason for hiding this comment

anand1976 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zhichao-cao Jun 22, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

facebook-github-bot commented Jun 29, 2020

facebook-github-bot left a comment

Choose a reason for hiding this comment

akankshamahajan15 commented Jun 29, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

facebook-github-bot commented Jul 7, 2020

facebook-github-bot left a comment

Choose a reason for hiding this comment

akankshamahajan15 commented Jul 7, 2020

facebook-github-bot commented Jul 13, 2020

facebook-github-bot commented Jul 13, 2020

akankshamahajan15 commented Jul 13, 2020

anand1976 left a comment

Choose a reason for hiding this comment

facebook-github-bot left a comment

Choose a reason for hiding this comment

facebook-github-bot commented Jul 14, 2020

zhichao-cao Jun 22, 2020 •

edited

Loading