chaoyli commented on a change in pull request #4110:
URL: https://github.com/apache/incubator-doris/pull/4110#discussion_r457803906
##########
File path: be/src/env/env.h
##########
@@ -271,13 +260,10 @@ class RandomAccessFile {
// one of Append or PositionedAppend. We support only Append here.
class WritableFile {
public:
- enum FlushMode {
- FLUSH_SYNC,
- FLUSH_ASYNC
- };
+ enum FlushMode { FLUSH_SYNC, FLUSH_ASYNC };
Review comment:
I think every line for every enum type is better to read.
##########
File path: be/src/env/env.h
##########
@@ -35,15 +35,10 @@ class Env {
// CREATE_OR_OPEN | opens | creates
// MUST_CREATE | fails | creates
// MUST_EXIST | opens | fails
- enum OpenMode {
- CREATE_OR_OPEN_WITH_TRUNCATE,
- CREATE_OR_OPEN,
- MUST_CREATE,
- MUST_EXIST
- };
+ enum OpenMode { CREATE_OR_OPEN_WITH_TRUNCATE, CREATE_OR_OPEN, MUST_CREATE,
MUST_EXIST };
Review comment:
I think every line for every enum type is better to read.
##########
File path: be/src/env/env_util.h
##########
@@ -30,14 +31,21 @@ struct WritableFileOptions;
namespace env_util {
-Status open_file_for_write(Env *env, const std::string& path,
std::shared_ptr<WritableFile> *file);
+Status open_file_for_write(Env* env, const std::string& path,
std::shared_ptr<WritableFile>* file);
-Status open_file_for_write(const WritableFileOptions& opts, Env *env,
- const std::string& path,
std::shared_ptr<WritableFile> *file);
+Status open_file_for_write(const WritableFileOptions& opts, Env* env, const
std::string& path,
+ std::shared_ptr<WritableFile>* file);
-Status open_file_for_random(Env *env, const std::string& path,
- std::shared_ptr<RandomAccessFile> *file);
+Status open_file_for_random(Env* env, const std::string& path,
+ std::shared_ptr<RandomAccessFile>* file);
+
+// A utility routine: write "data" to the named file.
+extern Status write_string_to_file(Env* env, const Slice& data, const
std::string& fname);
+// Like above but also fsyncs the new file.
+extern Status write_string_to_file_sync(Env* env, const Slice& data, const
std::string& fname);
Review comment:
extern here seems useless can can be removed.
##########
File path: be/src/util/thread.cpp
##########
@@ -74,18 +78,17 @@ class ThreadMgr {
// already been removed, this is a no-op.
void remove_thread(const pthread_t& pthread_id, const std::string&
category);
-private:
+ void display_thread_callback(const WebPageHandler::ArgumentMap& args,
EasyJson* ej) const;
Review comment:
EasyJson is not imported ?
##########
File path: be/src/util/thread.cpp
##########
@@ -397,8 +477,15 @@ Status ThreadJoiner::join() {
}
waited_ms += wait_for;
}
- return Status::Aborted(strings::Substitute("Timed out after $0ms joining
on $1",
- waited_ms, _thread->_name));
+ return Status::Aborted(
+ strings::Substitute("Timed out after $0ms joining on $1",
waited_ms, _thread->_name));
}
+void register_thread_display_page(WebPageHandler* web_page_handler) {
+ web_page_handler->register_template_page(
+ "/threadz", "Threads",
+ boost::bind(&ThreadMgr::display_thread_callback,
thread_manager.get(),
Review comment:
If we can use std::bind for the same purpose, It's better to replace it
using C++ standard library.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
