Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/10793 )
Change subject: WIP IMPALA-7178: Add the possibility to reduce logging for common data errors ...................................................................... Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/10793/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/10793/1//COMMIT_MSG@12 PS1, Line 12: the logs : will contain a new line for every occurrence > I was hesitant to go that way, because messages with the same error code bu But isn't that the point of the change (as you state below). I guess it would help if you could summarize which cases you feel the logging is useful to preserve (even when the errors are aggregated) and which you feel the logging should be eliminated. http://gerrit.cloudera.org:8080/#/c/10793/1//COMMIT_MSG@22 PS1, Line 22: fragments > I agree that this logic would be mainly useful for scanners. KuduSink has As the class is written today, it's not really tied to RuntimeState. You could put it anywhere, right? But, it's still not clear to me what the rule of thumb is for when developers should use this new class vs. just calling SetError() directly going forward (if, as you say, sometimes the individual logging is useful). If the primary motivation is to deal with the logging, why not just do something simpler, rather than introduce a whole class, like pass a boolean to SetError() (or add a parallel SetError() interface) that only logs the first time that error code is encountered (but doesn't introduce extra state)? -- To view, visit http://gerrit.cloudera.org:8080/10793 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie3b7c1fd020a7ba5e0d9c619e1b67236dce198aa Gerrit-Change-Number: 10793 Gerrit-PatchSet: 1 Gerrit-Owner: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Tue, 26 Jun 2018 15:32:29 +0000 Gerrit-HasComments: Yes
