Yida Wu has posted comments on this change. ( http://gerrit.cloudera.org:8080/23094 )
Change subject: IMPALA-12057: Track removed coordinators to reject queued queries early ...................................................................... Patch Set 12: (4 comments) http://gerrit.cloudera.org:8080/#/c/23094/11/be/src/scheduling/cluster-membership-mgr.h File be/src/scheduling/cluster-membership-mgr.h: http://gerrit.cloudera.org:8080/#/c/23094/11/be/src/scheduling/cluster-membership-mgr.h@295 PS11, Line 295: members > nit: membership Done http://gerrit.cloudera.org:8080/#/c/23094/10/be/src/scheduling/cluster-membership-mgr.cc File be/src/scheduling/cluster-membership-mgr.cc: http://gerrit.cloudera.org:8080/#/c/23094/10/be/src/scheduling/cluster-membership-mgr.cc@205 PS10, Line 205: atic inline void _markCoordinatorAsRemoved( : const std::shared_ptr<ClusterMembershipMgr::Snapshot>& state, : const BackendDescriptorPB& be, const BackendDescriptorPB& desc) { : string backend_id_str = PrintId(be.backend_id()) > 1000 is probably too much for list of strings. Have you consider adding bac Changed to only store the address string in the value, which is the only needed info. Even 1000 entries should stay under 1MB. Added cluster_membership_retained_removed_coords to control the list size, capped by MAX_REMOVED_COORD_SIZE. http://gerrit.cloudera.org:8080/#/c/23094/10/be/src/scheduling/cluster-membership-mgr.cc@221 PS10, Line 221: > should we call this function only if this host is admissiond ? Yeah, it makes more sense to use this only in admissiond. Added logic to ensure it's only marked in admissiond. http://gerrit.cloudera.org:8080/#/c/23094/11/be/src/scheduling/cluster-membership-mgr.cc File be/src/scheduling/cluster-membership-mgr.cc: http://gerrit.cloudera.org:8080/#/c/23094/11/be/src/scheduling/cluster-membership-mgr.cc@200 PS11, Line 200: FLAGS_cluster_membership_retained_removed_coords > Can we just the flag and remove the constant? Can also add validator to ens The MAX_REMOVED_COORD_SIZE is to cap the maximum size to prevent any crazy large setting. I think line 198 ensure value to be higher than 0, or some other place to validate? -- To view, visit http://gerrit.cloudera.org:8080/23094 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1e0f270299f8c20975d7895c17f4e2791c3360e0 Gerrit-Change-Number: 23094 Gerrit-PatchSet: 12 Gerrit-Owner: Yida Wu <[email protected]> Gerrit-Reviewer: Abhishek Rawat <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Reviewer: Wenzhe Zhou <[email protected]> Gerrit-Reviewer: Yida Wu <[email protected]> Gerrit-Comment-Date: Fri, 11 Jul 2025 01:13:31 +0000 Gerrit-HasComments: Yes
