From db027255eb7f07bcacc5e8d25de47d68a3cd9901 Mon Sep 17 00:00:00 2001
From: jackwener <30525741+jackwener@users.noreply.github.com>
Date: Thu, 13 Jan 2022 15:23:28 +0800
Subject: [PATCH 1/6] annotation

---
 src/graph/service/Authenticator.h        | 2 +-
 src/graph/service/CloudAuthenticator.cpp | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/graph/service/Authenticator.h b/src/graph/service/Authenticator.h
index dd53b7ba177..92a7b92895a 100644
--- a/src/graph/service/Authenticator.h
+++ b/src/graph/service/Authenticator.h
@@ -14,7 +14,7 @@ namespace graph {
 
 class Authenticator {
  public:
-  virtual ~Authenticator() {}
+  virtual ~Authenticator() = default;
 
   virtual Status NG_MUST_USE_RESULT auth(const std::string &user, const std::string &password) = 0;
 };
diff --git a/src/graph/service/CloudAuthenticator.cpp b/src/graph/service/CloudAuthenticator.cpp
index 09961b30ae4..a8c1423bb1d 100644
--- a/src/graph/service/CloudAuthenticator.cpp
+++ b/src/graph/service/CloudAuthenticator.cpp
@@ -31,7 +31,7 @@ Status CloudAuthenticator::auth(const std::string& user, const std::string& pass
   std::string userAndPasswd = user + ":" + password;
   std::string base64Str = proxygen::base64Encode(folly::StringPiece(userAndPasswd));
 
-  std::string header = "-H \"Content-Type: application/json\"  -H \"Authorization:Nebula ";
+  std::string header = R"(-H "Content-Type: application/json"  -H "Authorization:Nebula )";
   header = header + base64Str + "\"";
   auto result = http::HttpClient::post(FLAGS_cloud_http_url, header);
 

From 6caadda8b0cef24ec4b203b9c237cf59dc1efeda Mon Sep 17 00:00:00 2001
From: jackwener <30525741+jackwener@users.noreply.github.com>
Date: Fri, 14 Jan 2022 17:07:49 +0800
Subject: [PATCH 2/6] add annotation

---
 src/common/memory/MemoryUtils.cpp        |  9 ++------
 src/graph/service/Authenticator.h        |  1 +
 src/graph/service/CloudAuthenticator.cpp |  6 +-----
 src/graph/service/GraphServer.cpp        |  1 +
 src/graph/service/GraphService.cpp       | 27 ++++++++++++------------
 src/graph/service/GraphService.h         |  2 +-
 src/graph/service/PermissionManager.cpp  | 24 +++++++++------------
 src/graph/service/PermissionManager.h    |  1 +
 src/graph/service/QueryEngine.cpp        |  2 ++
 src/graph/service/QueryInstance.cpp      | 15 ++++++++++---
 src/graph/service/QueryInstance.h        |  3 ++-
 11 files changed, 47 insertions(+), 44 deletions(-)

diff --git a/src/common/memory/MemoryUtils.cpp b/src/common/memory/MemoryUtils.cpp
index 7e0bb7db7cc..bcd4b84ca02 100644
--- a/src/common/memory/MemoryUtils.cpp
+++ b/src/common/memory/MemoryUtils.cpp
@@ -8,11 +8,6 @@
 #include <folly/String.h>
 #include <gflags/gflags.h>
 
-#include <algorithm>
-#include <cstdio>
-#include <fstream>
-#include <regex>
-
 #include "common/fs/FileUtils.h"
 
 DEFINE_bool(containerized, false, "Whether run this process inside the docker container");
@@ -42,7 +37,7 @@ StatusOr<bool> MemoryUtils::hitsHighWatermark() {
     uint64_t cacheSize = 0;
     for (; iter.valid(); ++iter) {
       auto& sm = iter.matched();
-      cacheSize += std::stoul(sm[2].str(), NULL);
+      cacheSize += std::stoul(sm[2].str(), nullptr);
     }
 
     std::string limitPath =
@@ -64,7 +59,7 @@ StatusOr<bool> MemoryUtils::hitsHighWatermark() {
     std::vector<uint64_t> memorySize;
     for (; iter.valid(); ++iter) {
       auto& sm = iter.matched();
-      memorySize.emplace_back(std::stoul(sm[2].str(), NULL) << 10);
+      memorySize.emplace_back(std::stoul(sm[2].str(), nullptr) << 10);
     }
     std::sort(memorySize.begin(), memorySize.end());
     if (memorySize.size() >= 2u) {
diff --git a/src/graph/service/Authenticator.h b/src/graph/service/Authenticator.h
index 92a7b92895a..09a28b62831 100644
--- a/src/graph/service/Authenticator.h
+++ b/src/graph/service/Authenticator.h
@@ -12,6 +12,7 @@
 namespace nebula {
 namespace graph {
 
+// interface for authentication
 class Authenticator {
  public:
   virtual ~Authenticator() = default;
diff --git a/src/graph/service/CloudAuthenticator.cpp b/src/graph/service/CloudAuthenticator.cpp
index a8c1423bb1d..e642520ec94 100644
--- a/src/graph/service/CloudAuthenticator.cpp
+++ b/src/graph/service/CloudAuthenticator.cpp
@@ -34,11 +34,7 @@ Status CloudAuthenticator::auth(const std::string& user, const std::string& pass
   std::string header = R"(-H "Content-Type: application/json"  -H "Authorization:Nebula )";
   header = header + base64Str + "\"";
   auto result = http::HttpClient::post(FLAGS_cloud_http_url, header);
-
-  if (!result.ok()) {
-    LOG(ERROR) << result.status();
-    return result.status();
-  }
+  NG_LOG_AND_RETURN_IF_ERROR(result);
 
   try {
     auto json = folly::parseJson(result.value());
diff --git a/src/graph/service/GraphServer.cpp b/src/graph/service/GraphServer.cpp
index c9254d35295..541caa0a33b 100644
--- a/src/graph/service/GraphServer.cpp
+++ b/src/graph/service/GraphServer.cpp
@@ -40,6 +40,7 @@ bool GraphServer::start() {
     return false;
   }
 
+  // init worker id for snowflake generating unique id
   nebula::Snowflake::initWorkerId(interface->metaClient_.get());
 
   graphThread_ = std::make_unique<std::thread>([&] {
diff --git a/src/graph/service/GraphService.cpp b/src/graph/service/GraphService.cpp
index 4deaafc372b..02b13eb75e3 100644
--- a/src/graph/service/GraphService.cpp
+++ b/src/graph/service/GraphService.cpp
@@ -8,6 +8,7 @@
 #include <proxygen/lib/utils/CryptUtil.h>
 
 #include <boost/filesystem.hpp>
+#include <memory>
 
 #include "clients/storage/StorageClient.h"
 #include "common/base/Base.h"
@@ -73,7 +74,7 @@ folly::Future<AuthResponse> GraphService::future_authenticate(const std::string&
   auto authResult = auth(username, password);
   if (!authResult.ok()) {
     ctx->resp().errorCode = ErrorCode::E_BAD_USERNAME_PASSWORD;
-    ctx->resp().errorMsg.reset(new std::string(authResult.toString()));
+    ctx->resp().errorMsg = std::make_unique<std::string>(authResult.toString());
     ctx->finish();
     stats::StatsManager::addValue(kNumAuthFailedSessions);
     stats::StatsManager::addValue(kNumAuthFailedSessionsBadUserNamePassword);
@@ -82,7 +83,7 @@ folly::Future<AuthResponse> GraphService::future_authenticate(const std::string&
 
   if (sessionManager_->isOutOfConnections()) {
     ctx->resp().errorCode = ErrorCode::E_TOO_MANY_CONNECTIONS;
-    ctx->resp().errorMsg.reset(new std::string("Too many connections in the cluster"));
+    ctx->resp().errorMsg = std::make_unique<std::string>("Too many connections in the cluster");
     ctx->finish();
     stats::StatsManager::addValue(kNumAuthFailedSessions);
     stats::StatsManager::addValue(kNumAuthFailedSessionsOutOfMaxAllowed);
@@ -96,24 +97,24 @@ folly::Future<AuthResponse> GraphService::future_authenticate(const std::string&
       LOG(ERROR) << "Create session for userName: " << user << ", ip: " << cIp
                  << " failed: " << ret.status();
       ctx->resp().errorCode = ErrorCode::E_SESSION_INVALID;
-      ctx->resp().errorMsg.reset(new std::string(ret.status().toString()));
+      ctx->resp().errorMsg = std::make_unique<std::string>(ret.status().toString());
       return ctx->finish();
     }
     auto sessionPtr = std::move(ret).value();
     if (sessionPtr == nullptr) {
       LOG(ERROR) << "Get session for sessionId is nullptr";
       ctx->resp().errorCode = ErrorCode::E_SESSION_INVALID;
-      ctx->resp().errorMsg.reset(new std::string("Get session for sessionId is nullptr"));
+      ctx->resp().errorMsg = std::make_unique<std::string>("Get session for sessionId is nullptr");
       return ctx->finish();
     }
     stats::StatsManager::addValue(kNumOpenedSessions);
     stats::StatsManager::addValue(kNumActiveSessions);
     ctx->setSession(sessionPtr);
-    ctx->resp().sessionId.reset(new int64_t(ctx->session()->id()));
-    ctx->resp().timeZoneOffsetSeconds.reset(
-        new int32_t(time::Timezone::getGlobalTimezone().utcOffsetSecs()));
-    ctx->resp().timeZoneName.reset(
-        new std::string(time::Timezone::getGlobalTimezone().stdZoneName()));
+    ctx->resp().sessionId = std::make_unique<int64_t>(ctx->session()->id());
+    ctx->resp().timeZoneOffsetSeconds =
+        std::make_unique<int32_t>(time::Timezone::getGlobalTimezone().utcOffsetSecs());
+    ctx->resp().timeZoneName =
+        std::make_unique<std::string>(time::Timezone::getGlobalTimezone().stdZoneName());
     return ctx->finish();
   };
 
@@ -148,16 +149,16 @@ folly::Future<ExecutionResponse> GraphService::future_executeWithParameter(
     if (!ret.ok()) {
       LOG(ERROR) << "Get session for sessionId: " << sessionId << " failed: " << ret.status();
       ctx->resp().errorCode = ErrorCode::E_SESSION_INVALID;
-      ctx->resp().errorMsg.reset(new std::string(folly::stringPrintf(
-          "Get sessionId[%ld] failed: %s", sessionId, ret.status().toString().c_str())));
+      ctx->resp().errorMsg = std::make_unique<std::string>(folly::stringPrintf(
+          "Get sessionId[%ld] failed: %s", sessionId, ret.status().toString().c_str()));
       return ctx->finish();
     }
     auto sessionPtr = std::move(ret).value();
     if (sessionPtr == nullptr) {
       LOG(ERROR) << "Get session for sessionId: " << sessionId << " is nullptr";
       ctx->resp().errorCode = ErrorCode::E_SESSION_INVALID;
-      ctx->resp().errorMsg.reset(
-          new std::string(folly::stringPrintf("SessionId[%ld] does not exist", sessionId)));
+      ctx->resp().errorMsg = std::make_unique<std::string>(
+          folly::stringPrintf("SessionId[%ld] does not exist", sessionId));
       return ctx->finish();
     }
     stats::StatsManager::addValue(kNumQueries);
diff --git a/src/graph/service/GraphService.h b/src/graph/service/GraphService.h
index 85ed21a6ce4..a54c679dd6c 100644
--- a/src/graph/service/GraphService.h
+++ b/src/graph/service/GraphService.h
@@ -22,7 +22,7 @@ namespace graph {
 class GraphService final : public cpp2::GraphServiceSvIf {
  public:
   GraphService() = default;
-  ~GraphService() = default;
+  ~GraphService() override = default;
 
   Status NG_MUST_USE_RESULT init(std::shared_ptr<folly::IOThreadPoolExecutor> ioExecutor,
                                  const HostAddr& hostAddr);
diff --git a/src/graph/service/PermissionManager.cpp b/src/graph/service/PermissionManager.cpp
index 80ce3153a53..328463c2e75 100644
--- a/src/graph/service/PermissionManager.cpp
+++ b/src/graph/service/PermissionManager.cpp
@@ -10,8 +10,7 @@
 namespace nebula {
 namespace graph {
 
-// static
-Status PermissionManager::canReadSpace(ClientSession *session, GraphSpaceID spaceId) {
+/* static */ Status PermissionManager::canReadSpace(ClientSession *session, GraphSpaceID spaceId) {
   if (!FLAGS_enable_authorize) {
     return Status::OK();
   }
@@ -35,8 +34,8 @@ Status PermissionManager::canReadSpace(ClientSession *session, GraphSpaceID spac
   return Status::PermissionError("No permission to read space.");
 }
 
-// static
-Status PermissionManager::canReadSchemaOrData(ClientSession *session, ValidateContext *vctx) {
+/* static */ Status PermissionManager::canReadSchemaOrData(ClientSession *session,
+                                                           ValidateContext *vctx) {
   if (!FLAGS_enable_authorize) {
     return Status::OK();
   }
@@ -60,8 +59,7 @@ Status PermissionManager::canReadSchemaOrData(ClientSession *session, ValidateCo
   return Status::PermissionError("No permission to read schema/data.");
 }
 
-// static
-Status PermissionManager::canWriteSpace(ClientSession *session) {
+/* static */ Status PermissionManager::canWriteSpace(ClientSession *session) {
   if (!FLAGS_enable_authorize) {
     return Status::OK();
   }
@@ -71,8 +69,8 @@ Status PermissionManager::canWriteSpace(ClientSession *session) {
   return Status::PermissionError("No permission to write space.");
 }
 
-// static
-Status PermissionManager::canWriteSchema(ClientSession *session, ValidateContext *vctx) {
+/* static */ Status PermissionManager::canWriteSchema(ClientSession *session,
+                                                      ValidateContext *vctx) {
   if (!FLAGS_enable_authorize) {
     return Status::OK();
   }
@@ -97,8 +95,7 @@ Status PermissionManager::canWriteSchema(ClientSession *session, ValidateContext
   return Status::PermissionError("No permission to write schema.");
 }
 
-// static
-Status PermissionManager::canWriteUser(ClientSession *session) {
+/* static */ Status PermissionManager::canWriteUser(ClientSession *session) {
   if (!FLAGS_enable_authorize) {
     return Status::OK();
   }
@@ -113,8 +110,8 @@ Status PermissionManager::canWriteUser(ClientSession *session) {
   }
 }
 
-// static
-Status PermissionManager::canReadUser(ClientSession *session, const std::string &targetUser) {
+/* static */ Status PermissionManager::canReadUser(ClientSession *session,
+                                                   const std::string &targetUser) {
   if (!FLAGS_enable_authorize) {
     return Status::OK();
   }
@@ -177,8 +174,7 @@ Status PermissionManager::canWriteRole(ClientSession *session,
                                  targetUser.c_str());
 }
 
-// static
-Status PermissionManager::canWriteData(ClientSession *session, ValidateContext *vctx) {
+/* static */ Status PermissionManager::canWriteData(ClientSession *session, ValidateContext *vctx) {
   if (!FLAGS_enable_authorize) {
     return Status::OK();
   }
diff --git a/src/graph/service/PermissionManager.h b/src/graph/service/PermissionManager.h
index 655b8dd420f..06737c439e1 100644
--- a/src/graph/service/PermissionManager.h
+++ b/src/graph/service/PermissionManager.h
@@ -16,6 +16,7 @@
 namespace nebula {
 namespace graph {
 
+// This module is responsible for checking the permission of the user
 class PermissionManager final {
  public:
   PermissionManager() = delete;
diff --git a/src/graph/service/QueryEngine.cpp b/src/graph/service/QueryEngine.cpp
index d3f4f54e609..f3cd29113fa 100644
--- a/src/graph/service/QueryEngine.cpp
+++ b/src/graph/service/QueryEngine.cpp
@@ -24,6 +24,7 @@ DEFINE_int32(check_memory_interval_in_secs, 1, "Memory check interval in seconds
 namespace nebula {
 namespace graph {
 
+// register planners, init optimizer and set memory monitor
 Status QueryEngine::init(std::shared_ptr<folly::IOThreadPoolExecutor> ioExecutor,
                          meta::MetaClient* metaClient) {
   metaClient_ = metaClient;
@@ -54,6 +55,7 @@ void QueryEngine::execute(RequestContextPtr rctx) {
   instance->execute();
 }
 
+// setup memory monitor thread
 Status QueryEngine::setupMemoryMonitorThread() {
   memoryMonitorThread_ = std::make_unique<thread::GenericWorker>();
   if (!memoryMonitorThread_ || !memoryMonitorThread_->start("graph-memory-monitor")) {
diff --git a/src/graph/service/QueryInstance.cpp b/src/graph/service/QueryInstance.cpp
index aa56c9e58d5..5fca30c89c0 100644
--- a/src/graph/service/QueryInstance.cpp
+++ b/src/graph/service/QueryInstance.cpp
@@ -43,11 +43,14 @@ void QueryInstance::execute() {
     return;
   }
 
-  if (!explainOrContinue()) {
+  // sentence is explain query, finish
+  if (explainAndFinish()) {
     onFinish();
     return;
   }
 
+  // the execution engine converts the physical execution plan generated by the Planner into a
+  // series of Executors through the Scheduler to drive the execution of the Executors.
   scheduler_->schedule()
       .thenValue([this](Status s) {
         if (s.ok()) {
@@ -66,6 +69,7 @@ Status QueryInstance::validateAndOptimize() {
   auto *rctx = qctx()->rctx();
   auto &spaceName = rctx->session()->space().name;
   VLOG(1) << "Parsing query: " << rctx->query();
+  // result of parsing, get the parsing tree
   auto result = GQLParser(qctx()).parse(rctx->query());
   NG_RETURN_IF_ERROR(result);
   sentence_ = std::move(result).value();
@@ -84,7 +88,9 @@ Status QueryInstance::validateAndOptimize() {
     }
   }
 
+  // validate the query, if failed, return
   NG_RETURN_IF_ERROR(Validator::validate(sentence_.get(), qctx()));
+  // optimize the query, and get the execution plan
   NG_RETURN_IF_ERROR(findBestPlan());
   stats::StatsManager::addValue(kOptimizerLatencyUs, *(qctx_->plan()->optimizeTimeInUs()));
   if (FLAGS_enable_space_level_metrics && spaceName != "") {
@@ -95,9 +101,10 @@ Status QueryInstance::validateAndOptimize() {
   return Status::OK();
 }
 
-bool QueryInstance::explainOrContinue() {
+// sentence is explain query, return the description of plan , and then finish
+bool QueryInstance::explainAndFinish() {
   if (sentence_->kind() != Sentence::Kind::kExplain) {
-    return true;
+    return false;
   }
   auto &resp = qctx_->rctx()->resp();
   resp.planDesc = std::make_unique<PlanDescription>();
@@ -209,6 +216,7 @@ void QueryInstance::addSlowQueryStats(uint64_t latency, const std::string &space
   }
 }
 
+// get result from query context and fill the response
 void QueryInstance::fillRespData(ExecutionResponse *resp) {
   auto ectx = DCHECK_NOTNULL(qctx_->ectx());
   auto plan = DCHECK_NOTNULL(qctx_->plan());
@@ -229,6 +237,7 @@ void QueryInstance::fillRespData(ExecutionResponse *resp) {
   }
 }
 
+// the entry point of the optimizer
 Status QueryInstance::findBestPlan() {
   auto plan = qctx_->plan();
   SCOPED_TIMER(plan->optimizeTimeInUs());
diff --git a/src/graph/service/QueryInstance.h b/src/graph/service/QueryInstance.h
index a30168d56cc..b7823c56624 100644
--- a/src/graph/service/QueryInstance.h
+++ b/src/graph/service/QueryInstance.h
@@ -29,6 +29,7 @@ class QueryInstance final : public boost::noncopyable, public cpp::NonMovable {
   explicit QueryInstance(std::unique_ptr<QueryContext> qctx, opt::Optimizer* optimizer);
   ~QueryInstance() = default;
 
+  // entrance of the Validate, Optimize, Schedule, Execute process
   void execute();
 
   QueryContext* qctx() const {
@@ -52,7 +53,7 @@ class QueryInstance final : public boost::noncopyable, public cpp::NonMovable {
 
   Status validateAndOptimize();
   // return true if continue to execute
-  bool explainOrContinue();
+  bool explainAndFinish();
   void addSlowQueryStats(uint64_t latency, const std::string& spaceName) const;
   void fillRespData(ExecutionResponse* resp);
   Status findBestPlan();

From 8a42fc1d09d486b5a7ed76e2d0f0dd01bd4a6b3d Mon Sep 17 00:00:00 2001
From: jackwener <30525741+jackwener@users.noreply.github.com>
Date: Mon, 17 Jan 2022 14:22:58 +0800
Subject: [PATCH 3/6] Capital letters

---
 src/graph/service/Authenticator.h     |  2 +-
 src/graph/service/GraphFlags.h        |  2 +-
 src/graph/service/GraphServer.cpp     |  2 +-
 src/graph/service/GraphServer.h       |  2 +-
 src/graph/service/GraphService.cpp    |  4 ++--
 src/graph/service/PermissionCheck.cpp | 13 ++++++-------
 src/graph/service/QueryEngine.cpp     |  4 ++--
 src/graph/service/QueryInstance.cpp   | 18 +++++++++---------
 src/graph/service/QueryInstance.h     |  2 +-
 src/graph/service/RequestContext.h    |  4 ++--
 10 files changed, 26 insertions(+), 27 deletions(-)

diff --git a/src/graph/service/Authenticator.h b/src/graph/service/Authenticator.h
index 09a28b62831..ffddb8d17e4 100644
--- a/src/graph/service/Authenticator.h
+++ b/src/graph/service/Authenticator.h
@@ -12,7 +12,7 @@
 namespace nebula {
 namespace graph {
 
-// interface for authentication
+// Interface for authentication
 class Authenticator {
  public:
   virtual ~Authenticator() = default;
diff --git a/src/graph/service/GraphFlags.h b/src/graph/service/GraphFlags.h
index fe3ecaec3ba..22d8e433ddd 100644
--- a/src/graph/service/GraphFlags.h
+++ b/src/graph/service/GraphFlags.h
@@ -45,7 +45,7 @@ DECLARE_uint32(failed_login_attempts);
 // The deault value is 0. A value of 0 disables the option.
 DECLARE_uint32(password_lock_time_in_secs);
 
-// optimizer
+// Optimizer
 DECLARE_bool(enable_optimizer);
 
 DECLARE_int64(max_allowed_connections);
diff --git a/src/graph/service/GraphServer.cpp b/src/graph/service/GraphServer.cpp
index 541caa0a33b..51aa1039e68 100644
--- a/src/graph/service/GraphServer.cpp
+++ b/src/graph/service/GraphServer.cpp
@@ -40,7 +40,7 @@ bool GraphServer::start() {
     return false;
   }
 
-  // init worker id for snowflake generating unique id
+  // Init worker id for snowflake generating unique id
   nebula::Snowflake::initWorkerId(interface->metaClient_.get());
 
   graphThread_ = std::make_unique<std::thread>([&] {
diff --git a/src/graph/service/GraphServer.h b/src/graph/service/GraphServer.h
index 671c0720605..6c2b2696dde 100644
--- a/src/graph/service/GraphServer.h
+++ b/src/graph/service/GraphServer.h
@@ -24,7 +24,7 @@ class GraphServer {
 
   void stop();
 
-  // used for signal handler to set an internal stop flag
+  // Used for signal handler to set an internal stop flag
   void notifyStop();
 
   void waitUntilStop();
diff --git a/src/graph/service/GraphService.cpp b/src/graph/service/GraphService.cpp
index 02b13eb75e3..8be94813eb5 100644
--- a/src/graph/service/GraphService.cpp
+++ b/src/graph/service/GraphService.cpp
@@ -42,7 +42,7 @@ Status GraphService::init(std::shared_ptr<folly::IOThreadPoolExecutor> ioExecuto
 
   metaClient_ = std::make_unique<meta::MetaClient>(ioExecutor, std::move(addrs.value()), options);
 
-  // load data try 3 time
+  // Load data try 3 time
   bool loadDataOk = metaClient_->waitForMetadReady(3);
   if (!loadDataOk) {
     // Resort to retrying in the background
@@ -70,7 +70,7 @@ folly::Future<AuthResponse> GraphService::future_authenticate(const std::string&
 
   auto ctx = std::make_unique<RequestContext<AuthResponse>>();
   auto future = ctx->future();
-  // check username and password failed
+  // Check username and password failed
   auto authResult = auth(username, password);
   if (!authResult.ok()) {
     ctx->resp().errorCode = ErrorCode::E_BAD_USERNAME_PASSWORD;
diff --git a/src/graph/service/PermissionCheck.cpp b/src/graph/service/PermissionCheck.cpp
index 3acacf1846b..f0abcc00b5e 100644
--- a/src/graph/service/PermissionCheck.cpp
+++ b/src/graph/service/PermissionCheck.cpp
@@ -28,11 +28,10 @@ namespace graph {
  * Special operation : kShow, kChangePassword
  */
 
-// static
-Status PermissionCheck::permissionCheck(ClientSession *session,
-                                        Sentence *sentence,
-                                        ValidateContext *vctx,
-                                        GraphSpaceID targetSpace) {
+/* static */ Status PermissionCheck::permissionCheck(ClientSession *session,
+                                                     Sentence *sentence,
+                                                     ValidateContext *vctx,
+                                                     GraphSpaceID targetSpace) {
   if (!FLAGS_enable_authorize) {
     return Status::OK();
   }
@@ -165,7 +164,7 @@ Status PermissionCheck::permissionCheck(ClientSession *session,
     case Sentence::Kind::kShowMetaLeader:
     case Sentence::Kind::kShowHosts: {
       /**
-       * all roles can be show for above operations.
+       * All roles can be show for above operations.
        */
       return Status::OK();
     }
@@ -206,7 +205,7 @@ Status PermissionCheck::permissionCheck(ClientSession *session,
       return Status::OK();
     }
     case Sentence::Kind::kExplain:
-      // everyone could explain
+      // Everyone could explain
       return Status::OK();
     case Sentence::Kind::kSequential: {
       // No permission checking for sequential sentence.
diff --git a/src/graph/service/QueryEngine.cpp b/src/graph/service/QueryEngine.cpp
index f3cd29113fa..692d1dc4c72 100644
--- a/src/graph/service/QueryEngine.cpp
+++ b/src/graph/service/QueryEngine.cpp
@@ -24,7 +24,7 @@ DEFINE_int32(check_memory_interval_in_secs, 1, "Memory check interval in seconds
 namespace nebula {
 namespace graph {
 
-// register planners, init optimizer and set memory monitor
+// Register planners, init optimizer and set memory monitor
 Status QueryEngine::init(std::shared_ptr<folly::IOThreadPoolExecutor> ioExecutor,
                          meta::MetaClient* metaClient) {
   metaClient_ = metaClient;
@@ -55,7 +55,7 @@ void QueryEngine::execute(RequestContextPtr rctx) {
   instance->execute();
 }
 
-// setup memory monitor thread
+// Setup memory monitor thread
 Status QueryEngine::setupMemoryMonitorThread() {
   memoryMonitorThread_ = std::make_unique<thread::GenericWorker>();
   if (!memoryMonitorThread_ || !memoryMonitorThread_->start("graph-memory-monitor")) {
diff --git a/src/graph/service/QueryInstance.cpp b/src/graph/service/QueryInstance.cpp
index 5fca30c89c0..cf56376d88d 100644
--- a/src/graph/service/QueryInstance.cpp
+++ b/src/graph/service/QueryInstance.cpp
@@ -43,13 +43,13 @@ void QueryInstance::execute() {
     return;
   }
 
-  // sentence is explain query, finish
+  // Sentence is explain query, finish
   if (explainAndFinish()) {
     onFinish();
     return;
   }
 
-  // the execution engine converts the physical execution plan generated by the Planner into a
+  // The execution engine converts the physical execution plan generated by the Planner into a
   // series of Executors through the Scheduler to drive the execution of the Executors.
   scheduler_->schedule()
       .thenValue([this](Status s) {
@@ -69,7 +69,7 @@ Status QueryInstance::validateAndOptimize() {
   auto *rctx = qctx()->rctx();
   auto &spaceName = rctx->session()->space().name;
   VLOG(1) << "Parsing query: " << rctx->query();
-  // result of parsing, get the parsing tree
+  // Result of parsing, get the parsing tree
   auto result = GQLParser(qctx()).parse(rctx->query());
   NG_RETURN_IF_ERROR(result);
   sentence_ = std::move(result).value();
@@ -88,9 +88,9 @@ Status QueryInstance::validateAndOptimize() {
     }
   }
 
-  // validate the query, if failed, return
+  // Validate the query, if failed, return
   NG_RETURN_IF_ERROR(Validator::validate(sentence_.get(), qctx()));
-  // optimize the query, and get the execution plan
+  // Optimize the query, and get the execution plan
   NG_RETURN_IF_ERROR(findBestPlan());
   stats::StatsManager::addValue(kOptimizerLatencyUs, *(qctx_->plan()->optimizeTimeInUs()));
   if (FLAGS_enable_space_level_metrics && spaceName != "") {
@@ -101,7 +101,7 @@ Status QueryInstance::validateAndOptimize() {
   return Status::OK();
 }
 
-// sentence is explain query, return the description of plan , and then finish
+// Sentence is explain query, return the description of plan , and then finish
 bool QueryInstance::explainAndFinish() {
   if (sentence_->kind() != Sentence::Kind::kExplain) {
     return false;
@@ -216,7 +216,7 @@ void QueryInstance::addSlowQueryStats(uint64_t latency, const std::string &space
   }
 }
 
-// get result from query context and fill the response
+// Get result from query context and fill the response
 void QueryInstance::fillRespData(ExecutionResponse *resp) {
   auto ectx = DCHECK_NOTNULL(qctx_->ectx());
   auto plan = DCHECK_NOTNULL(qctx_->plan());
@@ -226,7 +226,7 @@ void QueryInstance::fillRespData(ExecutionResponse *resp) {
   auto &&value = ectx->moveValue(name);
   if (!value.isDataSet()) return;
 
-  // fill dataset
+  // Fill dataset
   auto result = value.moveDataSet();
   if (!result.colNames.empty()) {
     resp->data = std::make_unique<DataSet>(std::move(result));
@@ -237,7 +237,7 @@ void QueryInstance::fillRespData(ExecutionResponse *resp) {
   }
 }
 
-// the entry point of the optimizer
+// The entry point of the optimizer
 Status QueryInstance::findBestPlan() {
   auto plan = qctx_->plan();
   SCOPED_TIMER(plan->optimizeTimeInUs());
diff --git a/src/graph/service/QueryInstance.h b/src/graph/service/QueryInstance.h
index b7823c56624..ffffe30ed01 100644
--- a/src/graph/service/QueryInstance.h
+++ b/src/graph/service/QueryInstance.h
@@ -52,7 +52,7 @@ class QueryInstance final : public boost::noncopyable, public cpp::NonMovable {
   void onError(Status);
 
   Status validateAndOptimize();
-  // return true if continue to execute
+  // Return true if continue to execute
   bool explainAndFinish();
   void addSlowQueryStats(uint64_t latency, const std::string& spaceName) const;
   void fillRespData(ExecutionResponse* resp);
diff --git a/src/graph/service/RequestContext.h b/src/graph/service/RequestContext.h
index 6bf3cd4fc0d..80348480680 100644
--- a/src/graph/service/RequestContext.h
+++ b/src/graph/service/RequestContext.h
@@ -33,7 +33,7 @@ class RequestContext final : public boost::noncopyable, public cpp::NonMovable {
   RequestContext() = default;
   ~RequestContext() {
     if (session_ != nullptr) {
-      // keep the session active
+      // Keep the session active
       session_->charge();
     }
   }
@@ -57,7 +57,7 @@ class RequestContext final : public boost::noncopyable, public cpp::NonMovable {
   void setSession(std::shared_ptr<ClientSession> session) {
     session_ = std::move(session);
     if (session_ != nullptr) {
-      // keep the session active
+      // Keep the session active
       session_->charge();
     }
   }

From fa8c63a3dd39acdf228acb7e5a0f3131b96adede Mon Sep 17 00:00:00 2001
From: jackwener <30525741+jackwener@users.noreply.github.com>
Date: Wed, 9 Feb 2022 17:50:07 +0800
Subject: [PATCH 4/6] fix

---
 src/graph/service/GraphService.cpp | 16 ++++++++--------
 src/graph/service/QueryInstance.h  |  2 +-
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/graph/service/GraphService.cpp b/src/graph/service/GraphService.cpp
index 8be94813eb5..ff5dce6e96b 100644
--- a/src/graph/service/GraphService.cpp
+++ b/src/graph/service/GraphService.cpp
@@ -83,7 +83,7 @@ folly::Future<AuthResponse> GraphService::future_authenticate(const std::string&
 
   if (sessionManager_->isOutOfConnections()) {
     ctx->resp().errorCode = ErrorCode::E_TOO_MANY_CONNECTIONS;
-    ctx->resp().errorMsg = std::make_unique<std::string>("Too many connections in the cluster");
+    ctx->resp().errorMsg.reset(new std::string("Too many connections in the cluster"));
     ctx->finish();
     stats::StatsManager::addValue(kNumAuthFailedSessions);
     stats::StatsManager::addValue(kNumAuthFailedSessionsOutOfMaxAllowed);
@@ -97,24 +97,24 @@ folly::Future<AuthResponse> GraphService::future_authenticate(const std::string&
       LOG(ERROR) << "Create session for userName: " << user << ", ip: " << cIp
                  << " failed: " << ret.status();
       ctx->resp().errorCode = ErrorCode::E_SESSION_INVALID;
-      ctx->resp().errorMsg = std::make_unique<std::string>(ret.status().toString());
+      ctx->resp().errorMsg.reset(new std::string("Get session for sessionId is nullptr"));
       return ctx->finish();
     }
     auto sessionPtr = std::move(ret).value();
     if (sessionPtr == nullptr) {
       LOG(ERROR) << "Get session for sessionId is nullptr";
       ctx->resp().errorCode = ErrorCode::E_SESSION_INVALID;
-      ctx->resp().errorMsg = std::make_unique<std::string>("Get session for sessionId is nullptr");
+      ctx->resp().errorMsg.reset(new std::string("Get session for sessionId is nullptr"));
       return ctx->finish();
     }
     stats::StatsManager::addValue(kNumOpenedSessions);
     stats::StatsManager::addValue(kNumActiveSessions);
     ctx->setSession(sessionPtr);
-    ctx->resp().sessionId = std::make_unique<int64_t>(ctx->session()->id());
-    ctx->resp().timeZoneOffsetSeconds =
-        std::make_unique<int32_t>(time::Timezone::getGlobalTimezone().utcOffsetSecs());
-    ctx->resp().timeZoneName =
-        std::make_unique<std::string>(time::Timezone::getGlobalTimezone().stdZoneName());
+    ctx->resp().sessionId.reset(new int64_t(ctx->session()->id()));
+    ctx->resp().timeZoneOffsetSeconds.reset(
+        new int32_t(time::Timezone::getGlobalTimezone().utcOffsetSecs()));
+    ctx->resp().timeZoneName.reset(
+        new std::string(time::Timezone::getGlobalTimezone().stdZoneName()));
     return ctx->finish();
   };
 
diff --git a/src/graph/service/QueryInstance.h b/src/graph/service/QueryInstance.h
index ffffe30ed01..c146c3fd6b2 100644
--- a/src/graph/service/QueryInstance.h
+++ b/src/graph/service/QueryInstance.h
@@ -29,7 +29,7 @@ class QueryInstance final : public boost::noncopyable, public cpp::NonMovable {
   explicit QueryInstance(std::unique_ptr<QueryContext> qctx, opt::Optimizer* optimizer);
   ~QueryInstance() = default;
 
-  // entrance of the Validate, Optimize, Schedule, Execute process
+  // Entrance of the Validate, Optimize, Schedule, Execute process
   void execute();
 
   QueryContext* qctx() const {

From 48def49a0619be4d7c458205327f6f8ae9f6c042 Mon Sep 17 00:00:00 2001
From: jackwener <30525741+jackwener@users.noreply.github.com>
Date: Tue, 1 Mar 2022 17:34:23 +0800
Subject: [PATCH 5/6] fix

---
 src/graph/service/GraphService.cpp  | 4 ++--
 src/graph/service/QueryInstance.cpp | 1 +
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/graph/service/GraphService.cpp b/src/graph/service/GraphService.cpp
index ff5dce6e96b..099576a9b5f 100644
--- a/src/graph/service/GraphService.cpp
+++ b/src/graph/service/GraphService.cpp
@@ -74,7 +74,7 @@ folly::Future<AuthResponse> GraphService::future_authenticate(const std::string&
   auto authResult = auth(username, password);
   if (!authResult.ok()) {
     ctx->resp().errorCode = ErrorCode::E_BAD_USERNAME_PASSWORD;
-    ctx->resp().errorMsg = std::make_unique<std::string>(authResult.toString());
+    ctx->resp().errorMsg.reset(new std::string(authResult.toString()));
     ctx->finish();
     stats::StatsManager::addValue(kNumAuthFailedSessions);
     stats::StatsManager::addValue(kNumAuthFailedSessionsBadUserNamePassword);
@@ -97,7 +97,7 @@ folly::Future<AuthResponse> GraphService::future_authenticate(const std::string&
       LOG(ERROR) << "Create session for userName: " << user << ", ip: " << cIp
                  << " failed: " << ret.status();
       ctx->resp().errorCode = ErrorCode::E_SESSION_INVALID;
-      ctx->resp().errorMsg.reset(new std::string("Get session for sessionId is nullptr"));
+      ctx->resp().errorMsg.reset(new std::string(ret.status().toString()));
       return ctx->finish();
     }
     auto sessionPtr = std::move(ret).value();
diff --git a/src/graph/service/QueryInstance.cpp b/src/graph/service/QueryInstance.cpp
index cf56376d88d..01182f228b1 100644
--- a/src/graph/service/QueryInstance.cpp
+++ b/src/graph/service/QueryInstance.cpp
@@ -65,6 +65,7 @@ void QueryInstance::execute() {
                  [this](const std::exception &e) { onError(Status::Error("%s", e.what())); });
 }
 
+// Get query from GQLParser, validate and optimize the query
 Status QueryInstance::validateAndOptimize() {
   auto *rctx = qctx()->rctx();
   auto &spaceName = rctx->session()->space().name;

From d5839c0ab58ceb2fc9a91c22ff7f2b24482fc21d Mon Sep 17 00:00:00 2001
From: jackwener <30525741+jackwener@users.noreply.github.com>
Date: Tue, 8 Mar 2022 11:58:18 +0800
Subject: [PATCH 6/6] revert some code

---
 src/graph/service/GraphService.cpp | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/graph/service/GraphService.cpp b/src/graph/service/GraphService.cpp
index 099576a9b5f..bd71953838b 100644
--- a/src/graph/service/GraphService.cpp
+++ b/src/graph/service/GraphService.cpp
@@ -149,16 +149,16 @@ folly::Future<ExecutionResponse> GraphService::future_executeWithParameter(
     if (!ret.ok()) {
       LOG(ERROR) << "Get session for sessionId: " << sessionId << " failed: " << ret.status();
       ctx->resp().errorCode = ErrorCode::E_SESSION_INVALID;
-      ctx->resp().errorMsg = std::make_unique<std::string>(folly::stringPrintf(
-          "Get sessionId[%ld] failed: %s", sessionId, ret.status().toString().c_str()));
+      ctx->resp().errorMsg.reset(new std::string(folly::stringPrintf(
+          "Get sessionId[%ld] failed: %s", sessionId, ret.status().toString().c_str())));
       return ctx->finish();
     }
     auto sessionPtr = std::move(ret).value();
     if (sessionPtr == nullptr) {
       LOG(ERROR) << "Get session for sessionId: " << sessionId << " is nullptr";
       ctx->resp().errorCode = ErrorCode::E_SESSION_INVALID;
-      ctx->resp().errorMsg = std::make_unique<std::string>(
-          folly::stringPrintf("SessionId[%ld] does not exist", sessionId));
+      ctx->resp().errorMsg.reset(
+          new std::string(folly::stringPrintf("SessionId[%ld] does not exist", sessionId)));
       return ctx->finish();
     }
     stats::StatsManager::addValue(kNumQueries);