Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/8100 )
Change subject: IMPALA-5940: Avoid log spew by using Status::Expected. ...................................................................... Patch Set 3: (2 comments) http://gerrit.cloudera.org:8080/#/c/8100/3/be/src/runtime/client-cache.h File be/src/runtime/client-cache.h: http://gerrit.cloudera.org:8080/#/c/8100/3/be/src/runtime/client-cache.h@246 PS3, Line 246: return Status::Expected(ErrorMsg(TErrorCode::RPC_RECV_TIMEOUT, err)); > Yea, the backtrace is not all that useful here. Forgot to add that. I was a I agree that removing the backtrace here is the right direction as 90% of the cases logged are from DataStreamSender and they are considered "harmless" in this case. The caller of this function from DataStreamSender is from DataStreamSender::Channel::DoTransmitDataRpc() which will react to a timeout error by retrying the recv part of the RPC repeatedly until success or a non-timeout related error is found. So, in that sense, the RECV_TIMEOUT error shouldn't be logged in GetSendStatus() until it hits other errors. I did a quick audit of all callers of DoRpc() and it appears that some callers didn't really log the error. So, it may still be a bit inconvenient in those cases but this can be made up by adding more logging in those cases instead. (e.g. CatalogOpExecutor::Exec) http://gerrit.cloudera.org:8080/#/c/8100/3/be/src/runtime/client-cache.h@281 PS3, Line 281: return Status::Expected(ErrorMsg(TErrorCode::RPC_RECV_TIMEOUT, err)); > same set of comment/question This one is definitely fine to not log the backtrace as we know this is coming from DataStreamSender for sure. -- To view, visit http://gerrit.cloudera.org:8080/8100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I38088482377a1c3e794a9c8178ef83f29957a330 Gerrit-Change-Number: 8100 Gerrit-PatchSet: 3 Gerrit-Owner: Philip Zeyliger <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Mostafa Mokhtar <[email protected]> Gerrit-Reviewer: Philip Zeyliger <[email protected]> Gerrit-Reviewer: Sailesh Mukil <[email protected]> Gerrit-Comment-Date: Tue, 03 Oct 2017 18:29:16 +0000 Gerrit-HasComments: Yes
