You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2020/03/27 03:08:59 UTC

[GitHub] [incubator-doris] vagetablechicken opened a new pull request #3211: use SleepFor() instead of usleep()

vagetablechicken opened a new pull request #3211: use SleepFor() instead of usleep()
URL: https://github.com/apache/incubator-doris/pull/3211
 
 
   https://github.com/apache/incubator-doris/issues/3204#issuecomment-604334778
   
   contains so many reformat diffs, please use commit filter, or I can revert the format diffs.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] imay commented on a change in pull request #3211: use SleepFor() instead of usleep()

Posted by GitBox <gi...@apache.org>.
imay commented on a change in pull request #3211: use SleepFor() instead of usleep()
URL: https://github.com/apache/incubator-doris/pull/3211#discussion_r399049053
 
 

 ##########
 File path: be/src/exprs/utility_functions.cpp
 ##########
 @@ -17,27 +17,25 @@
 
 #include "exprs/utility_functions.h"
 
-#include "exprs/expr.h"
 #include "exprs/anyval_util.h"
-#include "util/debug_util.h"
+#include "exprs/expr.h"
 #include "runtime/tuple_row.h"
+#include "util/debug_util.h"
 
 namespace doris {
 
-void UtilityFunctions::init() {
-}
+void UtilityFunctions::init() {}
 
 StringVal UtilityFunctions::version(FunctionContext* ctx) {
     return AnyValUtil::from_string_temp(ctx, "5.1.0");
 }
 
-BooleanVal UtilityFunctions::sleep(
-        FunctionContext* ctx, const IntVal& milliseconds) {
+BooleanVal UtilityFunctions::sleep(FunctionContext* ctx, const IntVal& milliseconds) {
     if (milliseconds.is_null) {
         return BooleanVal::null();
     }
-    usleep(milliseconds.val * 1000 * 1000);
+    usleep(milliseconds.val * 1000 * 1000); // TODO(hw): ms * 1000 * 1000 = ns
 
 Review comment:
   this function is used to support SQL `select sleep(N)`; N is for N second. so you just change it to `sleepFor(Second)` 

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] chaoyli commented on a change in pull request #3211: use SleepFor() instead of usleep()

Posted by GitBox <gi...@apache.org>.
chaoyli commented on a change in pull request #3211: use SleepFor() instead of usleep()
URL: https://github.com/apache/incubator-doris/pull/3211#discussion_r399057171
 
 

 ##########
 File path: be/src/agent/task_worker_pool.cpp
 ##########
 @@ -668,8 +658,8 @@ void* TaskWorkerPool::_push_worker_thread_callback(void* arg_this) {
             error_msgs.push_back("push request push_type invalid.");
             task_status.__set_status_code(TStatusCode::ANALYSIS_ERROR);
         } else {
-            OLAP_LOG_WARNING("push failed, error_code: %d, signature: %ld",
-                             status, agent_task_req.signature);
+            OLAP_LOG_WARNING("push failed, error_code: %d, signature: %ld", status,
 
 Review comment:
   use LOG(WARNING) instead

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] chaoyli commented on a change in pull request #3211: use SleepFor() instead of usleep()

Posted by GitBox <gi...@apache.org>.
chaoyli commented on a change in pull request #3211: use SleepFor() instead of usleep()
URL: https://github.com/apache/incubator-doris/pull/3211#discussion_r399057425
 
 

 ##########
 File path: be/src/agent/task_worker_pool.cpp
 ##########
 @@ -1016,20 +1005,17 @@ void* TaskWorkerPool::_check_consistency_worker_thread_callback(void* arg_this)
         TStatus task_status;
 
         uint32_t checksum = 0;
-        EngineChecksumTask engine_task(check_consistency_req.tablet_id,
-                check_consistency_req.schema_hash,
-                check_consistency_req.version,
-                check_consistency_req.version_hash,
-                &checksum);
+        EngineChecksumTask engine_task(
+                check_consistency_req.tablet_id, check_consistency_req.schema_hash,
+                check_consistency_req.version, check_consistency_req.version_hash, &checksum);
         OLAPStatus res = worker_pool_this->_env->storage_engine()->execute_task(&engine_task);
         if (res != OLAP_SUCCESS) {
-            OLAP_LOG_WARNING("check consistency failed. status: %d, signature: %ld",
-                             res, agent_task_req.signature);
+            OLAP_LOG_WARNING("check consistency failed. status: %d, signature: %ld", res,
 
 Review comment:
   use LOG(WARNING) instead

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] vagetablechicken commented on a change in pull request #3211: use SleepFor() instead of usleep()

Posted by GitBox <gi...@apache.org>.
vagetablechicken commented on a change in pull request #3211: use SleepFor() instead of usleep()
URL: https://github.com/apache/incubator-doris/pull/3211#discussion_r399033631
 
 

 ##########
 File path: be/src/exprs/utility_functions.cpp
 ##########
 @@ -17,27 +17,25 @@
 
 #include "exprs/utility_functions.h"
 
-#include "exprs/expr.h"
 #include "exprs/anyval_util.h"
-#include "util/debug_util.h"
+#include "exprs/expr.h"
 #include "runtime/tuple_row.h"
+#include "util/debug_util.h"
 
 namespace doris {
 
-void UtilityFunctions::init() {
-}
+void UtilityFunctions::init() {}
 
 StringVal UtilityFunctions::version(FunctionContext* ctx) {
     return AnyValUtil::from_string_temp(ctx, "5.1.0");
 }
 
-BooleanVal UtilityFunctions::sleep(
-        FunctionContext* ctx, const IntVal& milliseconds) {
+BooleanVal UtilityFunctions::sleep(FunctionContext* ctx, const IntVal& milliseconds) {
     if (milliseconds.is_null) {
         return BooleanVal::null();
     }
-    usleep(milliseconds.val * 1000 * 1000);
+    usleep(milliseconds.val * 1000 * 1000); // TODO(hw): ms * 1000 * 1000 = ns
 
 Review comment:
   I'm confused about this. This usleep() use ns as its argument. Is it a mistake?
   I should change it to
   `SleepFor(MonoDelta::FromMilliseconds(milliseconds.val)` ?

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] imay merged pull request #3211: use SleepFor() instead of usleep()

Posted by GitBox <gi...@apache.org>.
imay merged pull request #3211: use SleepFor() instead of usleep()
URL: https://github.com/apache/incubator-doris/pull/3211
 
 
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] chaoyli commented on a change in pull request #3211: use SleepFor() instead of usleep()

Posted by GitBox <gi...@apache.org>.
chaoyli commented on a change in pull request #3211: use SleepFor() instead of usleep()
URL: https://github.com/apache/incubator-doris/pull/3211#discussion_r399057491
 
 

 ##########
 File path: be/src/agent/task_worker_pool.cpp
 ##########
 @@ -1169,7 +1160,8 @@ void* TaskWorkerPool::_report_tablet_worker_thread_callback(void* arg_this) {
 
         request.__set_report_version(_s_report_version);
         OLAPStatus report_all_tablets_info_status =
-                StorageEngine::instance()->tablet_manager()->report_all_tablets_info(&request.tablets);
+                StorageEngine::instance()->tablet_manager()->report_all_tablets_info(
+                        &request.tablets);
         if (report_all_tablets_info_status != OLAP_SUCCESS) {
             OLAP_LOG_WARNING("report get all tablets info failed. status: %d",
 
 Review comment:
   use LOG(WARNING) instead

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] chaoyli commented on a change in pull request #3211: use SleepFor() instead of usleep()

Posted by GitBox <gi...@apache.org>.
chaoyli commented on a change in pull request #3211: use SleepFor() instead of usleep()
URL: https://github.com/apache/incubator-doris/pull/3211#discussion_r399057245
 
 

 ##########
 File path: be/src/agent/task_worker_pool.cpp
 ##########
 @@ -718,11 +708,12 @@ void* TaskWorkerPool::_publish_version_worker_thread_callback(void* arg_this) {
             if (res == OLAP_SUCCESS) {
                 break;
             } else {
-                OLAP_LOG_WARNING("publish version error, retry. "
-                                 "[transaction_id=%ld, error_tablets_size=%d]",
-                                 publish_version_req.transaction_id, error_tablet_ids.size());
+                OLAP_LOG_WARNING(
 
 Review comment:
   use LOG(WARNING) instead

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] imay commented on a change in pull request #3211: use SleepFor() instead of usleep()

Posted by GitBox <gi...@apache.org>.
imay commented on a change in pull request #3211: use SleepFor() instead of usleep()
URL: https://github.com/apache/incubator-doris/pull/3211#discussion_r399016712
 
 

 ##########
 File path: be/src/exprs/utility_functions.cpp
 ##########
 @@ -17,27 +17,25 @@
 
 #include "exprs/utility_functions.h"
 
-#include "exprs/expr.h"
 #include "exprs/anyval_util.h"
-#include "util/debug_util.h"
+#include "exprs/expr.h"
 #include "runtime/tuple_row.h"
+#include "util/debug_util.h"
 
 namespace doris {
 
-void UtilityFunctions::init() {
-}
+void UtilityFunctions::init() {}
 
 StringVal UtilityFunctions::version(FunctionContext* ctx) {
     return AnyValUtil::from_string_temp(ctx, "5.1.0");
 }
 
-BooleanVal UtilityFunctions::sleep(
-        FunctionContext* ctx, const IntVal& milliseconds) {
+BooleanVal UtilityFunctions::sleep(FunctionContext* ctx, const IntVal& milliseconds) {
     if (milliseconds.is_null) {
         return BooleanVal::null();
     }
-    usleep(milliseconds.val * 1000 * 1000);
+    usleep(milliseconds.val * 1000 * 1000); // TODO(hw): ms * 1000 * 1000 = ns
 
 Review comment:
   why not change this?

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] chaoyli commented on a change in pull request #3211: use SleepFor() instead of usleep()

Posted by GitBox <gi...@apache.org>.
chaoyli commented on a change in pull request #3211: use SleepFor() instead of usleep()
URL: https://github.com/apache/incubator-doris/pull/3211#discussion_r399057343
 
 

 ##########
 File path: be/src/agent/task_worker_pool.cpp
 ##########
 @@ -967,8 +956,8 @@ void* TaskWorkerPool::_storage_medium_migrate_worker_thread_callback(void* arg_t
         EngineStorageMigrationTask engine_task(storage_medium_migrate_req);
         OLAPStatus res = worker_pool_this->_env->storage_engine()->execute_task(&engine_task);
         if (res != OLAP_SUCCESS) {
-            OLAP_LOG_WARNING("storage media migrate failed. status: %d, signature: %ld",
-                             res, agent_task_req.signature);
+            OLAP_LOG_WARNING("storage media migrate failed. status: %d, signature: %ld", res,
 
 Review comment:
   use LOG(WARNING) instead

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org