Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/12129 )
Change subject: IMPALA-6664: Tag log statements with fragment or query ids. ...................................................................... Patch Set 2: (7 comments) http://gerrit.cloudera.org:8080/#/c/12129/2/be/src/common/logging.cc File be/src/common/logging.cc: http://gerrit.cloudera.org:8080/#/c/12129/2/be/src/common/logging.cc@55 PS2, Line 55: kZeroUniqueId > ZERO_UNIQUE_ID Done http://gerrit.cloudera.org:8080/#/c/12129/2/be/src/common/logging.cc@63 PS2, Line 63: if (id == kZeroUniqueId) { > nit: one line Done http://gerrit.cloudera.org:8080/#/c/12129/2/be/src/common/logging.cc@67 PS2, Line 67: if (changed != nullptr) { > nit: one line Done http://gerrit.cloudera.org:8080/#/c/12129/2/be/src/common/thread-debug-info.h File be/src/common/thread-debug-info.h: http://gerrit.cloudera.org:8080/#/c/12129/2/be/src/common/thread-debug-info.h@a116 PS2, Line 116: > I think there was some reason this was a human-readable string - maybe to m Yep, I'll wait for Zoltan's input. http://gerrit.cloudera.org:8080/#/c/12129/2/be/src/common/thread-debug-info.h@118 PS2, Line 118: TUniqueId query_id_; > Can you document that these are 0 if this thread isn't part of a query or i Done http://gerrit.cloudera.org:8080/#/c/12129/2/be/src/runtime/io/request-context.h File be/src/runtime/io/request-context.h: http://gerrit.cloudera.org:8080/#/c/12129/2/be/src/runtime/io/request-context.h@403 PS2, Line 403: TUniqueId instance_id_; > Can you document that these are 0 if we're not working on behalf of a query I didn't do this because I don't think a request-context could be working not on behalf of a query. Happy to comment anyway. Do you think i should set this in RegisterContext() rather than around it? http://gerrit.cloudera.org:8080/#/c/12129/2/tests/common/impala_test_suite.py File tests/common/impala_test_suite.py: http://gerrit.cloudera.org:8080/#/c/12129/2/tests/common/impala_test_suite.py@843 PS2, Line 843: def assert_impalad_log_contains(self, level, line_regex, expected_count=1): > I don't see anything using this. d'oh; forgot to add the test file. -- To view, visit http://gerrit.cloudera.org:8080/12129 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6634ef9d1a7346339f24f2d40a7a3aa36a535da8 Gerrit-Change-Number: 12129 Gerrit-PatchSet: 2 Gerrit-Owner: Philip Zeyliger <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Philip Zeyliger <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Reviewer: Zoram Thanga <[email protected]> Gerrit-Comment-Date: Tue, 08 Jan 2019 19:49:54 +0000 Gerrit-HasComments: Yes
