Increase max_log_size in FlushJob to 1024 bytes #6258
Conversation
There was a problem hiding this comment.
@maysamyabandeh has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
logging/event_logger.h
Outdated
| return EventLoggerStream(log_buffer); | ||
| EventLoggerStream LogToBuffer( | ||
| LogBuffer* log_buffer, | ||
| const size_t max_log_size = LogBuffer::kDefaultMaxLogSize) { |
There was a problem hiding this comment.
Can we avoid default values for function parameters? Either explicitly specify the value at call sites or overload the function.
| @@ -249,7 +249,7 @@ Status FlushJob::Run(LogsWithPrepTracker* prep_tracker, | |||
| } | |||
| RecordFlushIOStats(); | |||
|
|
|||
| auto stream = event_logger_->LogToBuffer(log_buffer_); | |||
| auto stream = event_logger_->LogToBuffer(log_buffer_, 1024); | |||
There was a problem hiding this comment.
Add a comment to explain the reason for 1024.
|
@maysamyabandeh has updated the pull request. Re-import the pull request |
|
@maysamyabandeh has updated the pull request. Re-import the pull request |
There was a problem hiding this comment.
@maysamyabandeh has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@maysamyabandeh merged this pull request in 28e5a9a. |
Summary: When measure_io_stats_ is enabled, the volume of logging is beyond the default limit of 512 size. The patch allows the EventLoggerStream to change the limit, and also sets it to 1024 for FlushJob. Pull Request resolved: facebook/rocksdb#6258 Differential Revision: D19279269 Pulled By: maysamyabandeh fbshipit-source-id: 3fb5d468dad488f289ac99d713378177eb7504d6 Signed-off-by: Changlong Chen <levisonchen@live.cn>
Summary: When measure_io_stats_ is enabled, the volume of logging is beyond the default limit of 512 size. The patch allows the EventLoggerStream to change the limit, and also sets it to 1024 for FlushJob. Pull Request resolved: facebook/rocksdb#6258 Differential Revision: D19279269 Pulled By: maysamyabandeh fbshipit-source-id: 3fb5d468dad488f289ac99d713378177eb7504d6 Signed-off-by: Changlong Chen <levisonchen@live.cn>
When measure_io_stats_ is enabled, the volume of logging is beyond the default limit of 512 size. The patch allows the EventLoggerStream to change the limit, and also sets it to 1024 for FlushJob.