Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/16949 )
Change subject: IMPALA-9224 (part 1): Blacklist nodes with faulty disks ...................................................................... Patch Set 6: (3 comments) As discussed, it would be good to add a startup flag for this - probably default to having it off for now until we've done some of the planned follow up tasks, and then when we eventually switch it to always on it still allows users who run into problems with it to disable it. http://gerrit.cloudera.org:8080/#/c/16949/6/be/src/runtime/query-state.cc File be/src/runtime/query-state.cc: http://gerrit.cloudera.org:8080/#/c/16949/6/be/src/runtime/query-state.cc@347 PS6, Line 347: #ifndef NDEBUG I think this should be removed - its doesn't really help much from a performance perspective, since InitBufferPoolState is only called once per backend per query and checking to see if the string is empty is fast, and it will mean that the tests you wrote will fail in release build jobs, which is not what we want. It does at least protect against users accidentally setting a --debug_action in production environments, but we just rely on users not making that mistake. http://gerrit.cloudera.org:8080/#/c/16949/6/common/protobuf/control_service.proto File common/protobuf/control_service.proto: http://gerrit.cloudera.org:8080/#/c/16949/6/common/protobuf/control_service.proto@263 PS6, Line 263: runs nit: 'ran' http://gerrit.cloudera.org:8080/#/c/16949/6/common/protobuf/control_service.proto@265 PS6, Line 265: spilling_local_faulty_disk I think this is unnecessarily verbose, how about just 'local_disk_faulty' or 'spilling_disk_faulty'? (and of course, make the other places like function names match). -- To view, visit http://gerrit.cloudera.org:8080/16949 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I04bfcb7f2e0b1ef24a5b4350f270feecd8c47437 Gerrit-Change-Number: 16949 Gerrit-PatchSet: 6 Gerrit-Owner: Wenzhe Zhou <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]> Gerrit-Comment-Date: Mon, 25 Jan 2021 22:09:10 +0000 Gerrit-HasComments: Yes
