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/04/07 11:20:31 UTC
[GitHub] [incubator-doris] yiguolei opened a new pull request, #8900: [Refactor-step1] Add OLAPInternalError to status
yiguolei opened a new pull request, #8900:
URL: https://github.com/apache/incubator-doris/pull/8900
# Proposed changes
Issue Number: close #xxx
## Problem Summary:
This job is part of replace OLAPStatus with common/Status. I add some method to Status, including:
1. constructor for OLAPInternalError
2. add stacktrace during construct error status, it is useful for unknown errors or cores
3. override << operator for log message
## Checklist(Required)
1. Does it affect the original behavior: (Yes/No/I Don't know)
4. Has unit tests been added: (Yes/No/No Need)
5. Has document been added or modified: (Yes/No/No Need)
6. Does it need to update dependencies: (Yes/No)
7. 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] xinyiZzz commented on a diff in pull request #8900: [Refactor-step1] Add OLAPInternalError to status
Posted by GitBox <gi...@apache.org>.
xinyiZzz commented on code in PR #8900:
URL: https://github.com/apache/incubator-doris/pull/8900#discussion_r845682870
##########
be/src/common/status.h:
##########
@@ -151,8 +155,18 @@ class Status {
return Status(TStatusCode::DATA_QUALITY_ERROR, msg, precise_code, msg2);
}
+ // A wrapper for OLAPStatus
+ // Precise code is for OLAPStatus's enum value
+ // All Status Error is treated as Internal Error
+ static Status OLAPInternalError(int16_t precise_code, const Slice& msg = Slice()) {
+ #ifdef PRINT_ALL_ERR_STATUS_STACKTRACE
+ LOG(WARNING) << "Error occurred, error code = " << precise_code << ", with message: " << msg
+ << "\n" << boost::stacktrace::stacktrace();
Review Comment:
This is an optimization of HotSpot VM called fast throw, I'm not sure if C++ has a similar optimization. Or can we do it ourselves.
Ref: https://stackoverflow.com/questions/42115284/what-are-jvm-pre-allocated-exceptions
https://www.jianshu.com/p/e87d166380eb
##########
be/src/common/status.h:
##########
@@ -151,8 +155,18 @@ class Status {
return Status(TStatusCode::DATA_QUALITY_ERROR, msg, precise_code, msg2);
}
+ // A wrapper for OLAPStatus
+ // Precise code is for OLAPStatus's enum value
+ // All Status Error is treated as Internal Error
+ static Status OLAPInternalError(int16_t precise_code, const Slice& msg = Slice()) {
+ #ifdef PRINT_ALL_ERR_STATUS_STACKTRACE
+ LOG(WARNING) << "Error occurred, error code = " << precise_code << ", with message: " << msg
+ << "\n" << boost::stacktrace::stacktrace();
Review Comment:
This is an optimization of HotSpot VM called `fast throw`, I'm not sure if C++ has a similar optimization. Or can we do it ourselves.
Ref: https://stackoverflow.com/questions/42115284/what-are-jvm-pre-allocated-exceptions
https://www.jianshu.com/p/e87d166380eb
--
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 #8900: [Refactor-step1] Add OLAPInternalError to status
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #8900:
URL: https://github.com/apache/incubator-doris/pull/8900#issuecomment-1092435237
PR approved by anyone and no changes requested.
--
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] wangbo commented on a diff in pull request #8900: [Refactor-step1] Add OLAPInternalError to status
Posted by GitBox <gi...@apache.org>.
wangbo commented on code in PR #8900:
URL: https://github.com/apache/incubator-doris/pull/8900#discussion_r845077790
##########
be/src/common/status.h:
##########
@@ -151,8 +155,18 @@ class Status {
return Status(TStatusCode::DATA_QUALITY_ERROR, msg, precise_code, msg2);
}
+ // A wrapper for OLAPStatus
+ // Precise code is for OLAPStatus's enum value
+ // All Status Error is treated as Internal Error
+ static Status OLAPInternalError(int16_t precise_code, const Slice& msg = Slice()) {
+ #ifdef PRINT_ALL_ERR_STATUS_STACKTRACE
+ LOG(WARNING) << "Error occurred, error code = " << precise_code << ", with message: " << msg
+ << "\n" << boost::stacktrace::stacktrace();
+ #endif
+ return Status(TStatusCode::INTERNAL_ERROR, Slice(), precise_code, msg);
+ }
+
bool ok() const { return _length == 0; }
Review Comment:
Because ```_length = size + HEADER_LEN;```,
So ```ok()``` should mean ```_length == HEADER_LEN``` ?
--
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 diff in pull request #8900: [Refactor-step1] Add OLAPInternalError to status
Posted by GitBox <gi...@apache.org>.
yiguolei commented on code in PR #8900:
URL: https://github.com/apache/incubator-doris/pull/8900#discussion_r845757488
##########
be/src/common/status.h:
##########
@@ -151,8 +155,18 @@ class Status {
return Status(TStatusCode::DATA_QUALITY_ERROR, msg, precise_code, msg2);
}
+ // A wrapper for OLAPStatus
+ // Precise code is for OLAPStatus's enum value
+ // All Status Error is treated as Internal Error
+ static Status OLAPInternalError(int16_t precise_code, const Slice& msg = Slice()) {
+ #ifdef PRINT_ALL_ERR_STATUS_STACKTRACE
+ LOG(WARNING) << "Error occurred, error code = " << precise_code << ", with message: " << msg
+ << "\n" << boost::stacktrace::stacktrace();
Review Comment:
@xinyiZzz Currently, I won't implement this feature in Doris, because it depends on unify the error code work.
I will do following task after this PR is closed:
1. Replace OLAPStatus with Status
2. Unify ErrorCode, for example TStatusCode, OLAPStatus, PStatus, I will unify them to only one error code
3. Do some statistics on ErrorCodes, for example, we could find how many IO Error happens.
4. After all these 3 tasks are finished, I think maybe we could implement OmitStackTraceInFastThrow. But I am not sure because it will ignore some errors.
--
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 diff in pull request #8900: [Refactor-step1] Add OLAPInternalError to status
Posted by GitBox <gi...@apache.org>.
yiguolei commented on code in PR #8900:
URL: https://github.com/apache/incubator-doris/pull/8900#discussion_r845670569
##########
be/src/common/status.h:
##########
@@ -151,8 +155,18 @@ class Status {
return Status(TStatusCode::DATA_QUALITY_ERROR, msg, precise_code, msg2);
}
+ // A wrapper for OLAPStatus
+ // Precise code is for OLAPStatus's enum value
+ // All Status Error is treated as Internal Error
+ static Status OLAPInternalError(int16_t precise_code, const Slice& msg = Slice()) {
+ #ifdef PRINT_ALL_ERR_STATUS_STACKTRACE
+ LOG(WARNING) << "Error occurred, error code = " << precise_code << ", with message: " << msg
+ << "\n" << boost::stacktrace::stacktrace();
Review Comment:
Does java have this feature? Could you show me some demo code in JAVA?
--
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] xinyiZzz commented on a diff in pull request #8900: [Refactor-step1] Add OLAPInternalError to status
Posted by GitBox <gi...@apache.org>.
xinyiZzz commented on code in PR #8900:
URL: https://github.com/apache/incubator-doris/pull/8900#discussion_r845035829
##########
be/src/common/status.h:
##########
@@ -151,8 +155,18 @@ class Status {
return Status(TStatusCode::DATA_QUALITY_ERROR, msg, precise_code, msg2);
}
+ // A wrapper for OLAPStatus
+ // Precise code is for OLAPStatus's enum value
+ // All Status Error is treated as Internal Error
+ static Status OLAPInternalError(int16_t precise_code, const Slice& msg = Slice()) {
+ #ifdef PRINT_ALL_ERR_STATUS_STACKTRACE
+ LOG(WARNING) << "Error occurred, error code = " << precise_code << ", with message: " << msg
+ << "\n" << boost::stacktrace::stacktrace();
Review Comment:
Is it possible to print stacktrace for all errors, this is useful in many times like mem limit. But the same stacktrace will not be printed after printing several times, like JAVA, until the next restart of BE.
--
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 diff in pull request #8900: [Refactor-step1] Add OLAPInternalError to status
Posted by GitBox <gi...@apache.org>.
morningman commented on code in PR #8900:
URL: https://github.com/apache/incubator-doris/pull/8900#discussion_r845681873
##########
be/src/common/status.h:
##########
@@ -151,8 +155,18 @@ class Status {
return Status(TStatusCode::DATA_QUALITY_ERROR, msg, precise_code, msg2);
}
+ // A wrapper for OLAPStatus
+ // Precise code is for OLAPStatus's enum value
+ // All Status Error is treated as Internal Error
+ static Status OLAPInternalError(int16_t precise_code, const Slice& msg = Slice()) {
+ #ifdef PRINT_ALL_ERR_STATUS_STACKTRACE
+ LOG(WARNING) << "Error occurred, error code = " << precise_code << ", with message: " << msg
+ << "\n" << boost::stacktrace::stacktrace();
Review Comment:
Search jvm option `OmitStackTraceInFastThrow`
--
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 #8900: [Refactor-step1] Add OLAPInternalError to status
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #8900:
URL: https://github.com/apache/incubator-doris/pull/8900#issuecomment-1092435219
PR approved by at least one committer and no changes requested.
--
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] wangbo commented on a diff in pull request #8900: [Refactor-step1] Add OLAPInternalError to status
Posted by GitBox <gi...@apache.org>.
wangbo commented on code in PR #8900:
URL: https://github.com/apache/incubator-doris/pull/8900#discussion_r845077790
##########
be/src/common/status.h:
##########
@@ -151,8 +155,18 @@ class Status {
return Status(TStatusCode::DATA_QUALITY_ERROR, msg, precise_code, msg2);
}
+ // A wrapper for OLAPStatus
+ // Precise code is for OLAPStatus's enum value
+ // All Status Error is treated as Internal Error
+ static Status OLAPInternalError(int16_t precise_code, const Slice& msg = Slice()) {
+ #ifdef PRINT_ALL_ERR_STATUS_STACKTRACE
+ LOG(WARNING) << "Error occurred, error code = " << precise_code << ", with message: " << msg
+ << "\n" << boost::stacktrace::stacktrace();
+ #endif
+ return Status(TStatusCode::INTERNAL_ERROR, Slice(), precise_code, msg);
+ }
+
bool ok() const { return _length == 0; }
Review Comment:
Because ```_length = size + HEADER_LEN;```,
So ```ok()``` should mean ```_length == HEADER_LEN``` ?
--
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 merged pull request #8900: [Refactor-step1] Add OLAPInternalError to status
Posted by GitBox <gi...@apache.org>.
yiguolei merged PR #8900:
URL: https://github.com/apache/incubator-doris/pull/8900
--
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] xinyiZzz commented on a diff in pull request #8900: [Refactor-step1] Add OLAPInternalError to status
Posted by GitBox <gi...@apache.org>.
xinyiZzz commented on code in PR #8900:
URL: https://github.com/apache/incubator-doris/pull/8900#discussion_r845035829
##########
be/src/common/status.h:
##########
@@ -151,8 +155,18 @@ class Status {
return Status(TStatusCode::DATA_QUALITY_ERROR, msg, precise_code, msg2);
}
+ // A wrapper for OLAPStatus
+ // Precise code is for OLAPStatus's enum value
+ // All Status Error is treated as Internal Error
+ static Status OLAPInternalError(int16_t precise_code, const Slice& msg = Slice()) {
+ #ifdef PRINT_ALL_ERR_STATUS_STACKTRACE
+ LOG(WARNING) << "Error occurred, error code = " << precise_code << ", with message: " << msg
+ << "\n" << boost::stacktrace::stacktrace();
Review Comment:
Can the stacktrace be printed for all errors, but the same stacktrace will not be printed after printing several times, like JAVA, until the next restart of BE.
--
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] xinyiZzz commented on a diff in pull request #8900: [Refactor-step1] Add OLAPInternalError to status
Posted by GitBox <gi...@apache.org>.
xinyiZzz commented on code in PR #8900:
URL: https://github.com/apache/incubator-doris/pull/8900#discussion_r845769740
##########
be/src/common/status.h:
##########
@@ -151,8 +155,18 @@ class Status {
return Status(TStatusCode::DATA_QUALITY_ERROR, msg, precise_code, msg2);
}
+ // A wrapper for OLAPStatus
+ // Precise code is for OLAPStatus's enum value
+ // All Status Error is treated as Internal Error
+ static Status OLAPInternalError(int16_t precise_code, const Slice& msg = Slice()) {
+ #ifdef PRINT_ALL_ERR_STATUS_STACKTRACE
+ LOG(WARNING) << "Error occurred, error code = " << precise_code << ", with message: " << msg
+ << "\n" << boost::stacktrace::stacktrace();
Review Comment:
Great, looking forward.
--
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