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 2022/03/01 02:32:56 UTC
[GitHub] [incubator-doris] yiguolei opened a new pull request #8273: [Refactor] Remove agent status
yiguolei opened a new pull request #8273:
URL: https://github.com/apache/incubator-doris/pull/8273
# Proposed changes
Issue Number: close #xxx
## Problem Summary:
There are 3 error code types in BE: OLAPStatus AgentStatus Status. It is very confused and sometimes conflict during write code. I will try to unify them to Status.
## Checklist(Required)
1. Does it affect the original behavior: (Yes/No/I Don't know)
2. Has unit tests been added: (Yes/No/No Need)
3. Has document been added or modified: (Yes/No/No Need)
4. Does it need to update dependencies: (Yes/No)
5. Are there any changes that cannot be rolled back: (Yes/No)
## Further comments
If this is a relatively large or complex change, kick off the discussion at [dev@doris.apache.org](mailto:dev@doris.apache.org) by explaining why you chose the solution you did and what alternatives you considered, etc...
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] yiguolei commented on a change in pull request #8273: [Refactor] Remove agent status
Posted by GitBox <gi...@apache.org>.
yiguolei commented on a change in pull request #8273:
URL: https://github.com/apache/incubator-doris/pull/8273#discussion_r817258851
##########
File path: be/src/agent/pusher.cpp
##########
@@ -73,7 +72,7 @@ AgentStatus Pusher::init() {
// Set download param
string tmp_file_dir;
status = _get_tmp_file_dir(olap_table->storage_root_path_name(), &tmp_file_dir);
- if (status != DORIS_SUCCESS) {
+ if (status != Status::OK()) {
LOG(WARNING) << "get local path failed. tmp file dir: " << tmp_file_dir;
Review comment:
I have already removed pusher.cpp file in next pr.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] morningman commented on pull request #8273: [Refactor] Remove agent status
Posted by GitBox <gi...@apache.org>.
morningman commented on pull request #8273:
URL: https://github.com/apache/incubator-doris/pull/8273#issuecomment-1055533379
> Details
ut has been fixed. you need to rebase the master.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] yiguolei commented on a change in pull request #8273: [Refactor] Remove agent status
Posted by GitBox <gi...@apache.org>.
yiguolei commented on a change in pull request #8273:
URL: https://github.com/apache/incubator-doris/pull/8273#discussion_r817258663
##########
File path: be/src/agent/pusher.cpp
##########
@@ -47,16 +47,15 @@ Pusher::Pusher(OLAPEngine* engine, const TPushReq& push_req)
Pusher::~Pusher() {}
-AgentStatus Pusher::init() {
- AgentStatus status = DORIS_SUCCESS;
+Status Pusher::init() {
+ Status status = Status::OK();
Review comment:
I already removed pusher.cpp file in next pr.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] github-actions[bot] commented on pull request #8273: [Refactor] Remove agent status
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #8273:
URL: https://github.com/apache/incubator-doris/pull/8273#issuecomment-1061744389
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] jackwener commented on pull request #8273: [Refactor] Remove agent status
Posted by GitBox <gi...@apache.org>.
jackwener commented on pull request #8273:
URL: https://github.com/apache/incubator-doris/pull/8273#issuecomment-1055237191
UT didn't pass. I can't ensure the correctness, It needs to build local and run all unit tests;
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] morningman commented on a change in pull request #8273: [Refactor] Remove agent status
Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #8273:
URL: https://github.com/apache/incubator-doris/pull/8273#discussion_r816850436
##########
File path: be/src/agent/pusher.cpp
##########
@@ -47,16 +47,15 @@ Pusher::Pusher(OLAPEngine* engine, const TPushReq& push_req)
Pusher::~Pusher() {}
-AgentStatus Pusher::init() {
- AgentStatus status = DORIS_SUCCESS;
+Status Pusher::init() {
+ Status status = Status::OK();
Review comment:
Remove this line and define it at line 74
##########
File path: be/src/agent/pusher.cpp
##########
@@ -73,7 +72,7 @@ AgentStatus Pusher::init() {
// Set download param
string tmp_file_dir;
status = _get_tmp_file_dir(olap_table->storage_root_path_name(), &tmp_file_dir);
- if (status != DORIS_SUCCESS) {
+ if (status != Status::OK()) {
LOG(WARNING) << "get local path failed. tmp file dir: " << tmp_file_dir;
Review comment:
print `st.get_error_msg()`
##########
File path: be/src/agent/task_worker_pool.cpp
##########
@@ -1541,11 +1523,11 @@ void TaskWorkerPool::_move_dir_thread_callback() {
TStatus task_status;
// TODO: move dir
- AgentStatus status =
+ Status status =
_move_dir(move_dir_req.tablet_id, move_dir_req.schema_hash, move_dir_req.src,
move_dir_req.job_id, true /* TODO */, &error_msgs);
- if (status != DORIS_SUCCESS) {
+ if (status != Status::OK()) {
Review comment:
use `status.ok()`.
Better to modify all other places.
##########
File path: be/src/agent/task_worker_pool.cpp
##########
@@ -1581,7 +1563,7 @@ AgentStatus TaskWorkerPool::_move_dir(const TTabletId tablet_id, const TSchemaHa
LOG(INFO) << "failed to get tablet. tablet_id:" << tablet_id
<< ", schema hash:" << schema_hash;
error_msgs->push_back("failed to get tablet");
- return DORIS_TASK_REQUEST_ERROR;
+ return Status::InvalidArgument("Could not find tablet");
Review comment:
If using Status, we don't need `error_msgs` anymore.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] morningman merged pull request #8273: [Refactor] Remove agent status
Posted by GitBox <gi...@apache.org>.
morningman merged pull request #8273:
URL: https://github.com/apache/incubator-doris/pull/8273
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org