Sailesh Mukil has posted comments on this change. Change subject: IMPALA-5198: Error messages are sometimes dropped before reaching client ......................................................................
Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/6627/1//COMMIT_MSG Commit Message: Line 7: IMPALA-5198: Error messages are sometimes dropped before reaching client > Maybe describe the symptoms/bug more generally, so it's easier to understan Done http://gerrit.cloudera.org:8080/#/c/6627/1/be/src/service/impala-beeswax-server.cc File be/src/service/impala-beeswax-server.cc: Line 288: if (exec_state->coord() != NULL) { > Is this addition related to the change? I don't understand why we're printi It's up for debate whether we want to add this or not. I added it to be consistent with the HS2 server. https://github.com/apache/incubator-impala/blob/6a9df540967e07b09524268d0cc52b7d10835676/be/src/service/impala-hs2-server.cc#L826 Technically GetLog()/get_log() is not just for retrieving just error logs, it can be used at any point in the query to get any sort of information available. However, our use of it in beeswax seems just to be for retrieving errors. If we think this is unnecessary for beeswax, I can go ahead and remove it. http://gerrit.cloudera.org:8080/#/c/6627/1/be/src/util/error-util.cc File be/src/util/error-util.cc: Line 123: const ArgType& arg4, const ArgType& arg5, const ArgType& arg6, const ArgType& arg7, > It seems like conceptually the problem is the Status(TStatus) constructor s I spent some time looking at the different places that TStatus is used, and although the best option would be to make TStatus identical in structure to Status (it currently isn't), doing so would require a lot of changes and cause inconsistencies between the different TStatus objects (thrift::TStatus, TCLIService::TStatus, impala::TStatus). Some of these are exposed to clients and would cause a breaking change. So I decided to just move this unwrapping to the Status level by introducing a function called Status::FromThrift(), which makes the wrapping and unrwrapping happen at the same level. -- To view, visit http://gerrit.cloudera.org:8080/6627 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5d9d63610eb0d2acae3a9303ce46e1410727ce87 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Sailesh Mukil <[email protected]> Gerrit-Reviewer: Sailesh Mukil <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
