-
Notifications
You must be signed in to change notification settings - Fork 411
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
Tracing: Add tracing_id to PageStorage snapshot #4330
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -52,17 +52,17 @@ class StoragePool : private boost::noncopyable | |
PageReader & dataReader() { return data_storage_reader; } | ||
PageReader & metaReader() { return meta_storage_reader; } | ||
|
||
PageReader newLogReader(ReadLimiterPtr read_limiter, bool snapshot_read) | ||
PageReader newLogReader(ReadLimiterPtr read_limiter, bool snapshot_read, const String & tracing_id) | ||
{ | ||
return PageReader(ns_id, log_storage, snapshot_read ? log_storage->getSnapshot() : nullptr, read_limiter); | ||
return PageReader(ns_id, log_storage, snapshot_read ? log_storage->getSnapshot(tracing_id) : nullptr, read_limiter); | ||
} | ||
PageReader newDataReader(ReadLimiterPtr read_limiter, bool snapshot_read) | ||
PageReader newDataReader(ReadLimiterPtr read_limiter, bool snapshot_read, const String & tracing_id) | ||
{ | ||
return PageReader(ns_id, data_storage, snapshot_read ? data_storage->getSnapshot() : nullptr, read_limiter); | ||
return PageReader(ns_id, data_storage, snapshot_read ? data_storage->getSnapshot(tracing_id) : nullptr, read_limiter); | ||
} | ||
PageReader newMetaReader(ReadLimiterPtr read_limiter, bool snapshot_read) | ||
PageReader newMetaReader(ReadLimiterPtr read_limiter, bool snapshot_read, const String & tracing_id) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would like to keep it without a default value, to let the caller and reviewer think about whether we need to add a tracing id to it or not. |
||
{ | ||
return PageReader(ns_id, meta_storage, snapshot_read ? meta_storage->getSnapshot() : nullptr, read_limiter); | ||
return PageReader(ns_id, meta_storage, snapshot_read ? meta_storage->getSnapshot(tracing_id) : nullptr, read_limiter); | ||
} | ||
|
||
// Caller must cancel gc tasks before drop | ||
|
@@ -112,10 +112,10 @@ class PageIdGenerator : private boost::noncopyable | |
|
||
struct StorageSnapshot : private boost::noncopyable | ||
{ | ||
StorageSnapshot(StoragePool & storage, ReadLimiterPtr read_limiter, bool snapshot_read = true) | ||
: log_reader(storage.newLogReader(read_limiter, snapshot_read)) | ||
, data_reader(storage.newDataReader(read_limiter, snapshot_read)) | ||
, meta_reader(storage.newMetaReader(read_limiter, snapshot_read)) | ||
StorageSnapshot(StoragePool & storage, ReadLimiterPtr read_limiter, const String & tracing_id, bool snapshot_read) | ||
: log_reader(storage.newLogReader(read_limiter, snapshot_read, tracing_id)) | ||
, data_reader(storage.newDataReader(read_limiter, snapshot_read, tracing_id)) | ||
, meta_reader(storage.newMetaReader(read_limiter, snapshot_read, tracing_id)) | ||
{} | ||
|
||
PageReader log_reader; | ||
|
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.
Maybe we can distinguish mpp task from background tasks using
snap->is_update
? For background tasks, it's always true. And for mpp task it's always false.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.
good idea, i will check it in the later pr