Yida Wu has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/23204 )

Change subject: IMPALA-14234: Fix a version mismatch DCHECK hit when admissiond 
cluster membership recovering
......................................................................


Patch Set 6:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/23204/5//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/23204/5//COMMIT_MSG@9
PS5, Line 9: This patch fixes a DCHECK failure
> Mention where the DCHEK is. Is it AdmissionController::ComputeGroupSchedule
Yes, the DCHECK is in AdmissionController::ComputeGroupScheduleState. Because 
it seems an old code, not sure the original purpose, but I guess it is 
something common sense that the version of cluster membership just keeps 
increasing, so the smaller current version compared to the old version is 
treated as an abnormal case. However in the release version, if the current 
membership version is lower, it's treated as being equal to the query's 
version, meaning no rescheduling is needed and no retry is needed in 
https://github.com/apache/impala/blob/16c350ce5aa5dbf1a9ea8b481c64780d445e9a2a/be/src/scheduling/admission-controller.cc#L2235.
 If the current cluster membership version is larger, it will try to find out 
the latest match from the cluster.
Updated the commit message.


http://gerrit.cloudera.org:8080/#/c/23204/2/tests/custom_cluster/test_admission_controller.py
File tests/custom_cluster/test_admission_controller.py:

http://gerrit.cloudera.org:8080/#/c/23204/2/tests/custom_cluster/test_admission_controller.py@2296
PS2, Line 2296:         continue
> Probably also update the test to actually change cluster membership snapsho
This is a good suggestion for a more comprehensive test. Since this test is 
narrowly focused on reproducing the DCHECK regression, I'd prefer to add more 
complex tests like that in a separate follow up patch. Does that sound 
reasonable?



--
To view, visit http://gerrit.cloudera.org:8080/23204
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iea77347bd4775abd7866817146e326c7c5042f5e
Gerrit-Change-Number: 23204
Gerrit-PatchSet: 6
Gerrit-Owner: Yida Wu <[email protected]>
Gerrit-Reviewer: Abhishek Rawat <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Jason Fehr <[email protected]>
Gerrit-Reviewer: Riza Suminto <[email protected]>
Gerrit-Reviewer: Wenzhe Zhou <[email protected]>
Gerrit-Reviewer: Yida Wu <[email protected]>
Gerrit-Comment-Date: Tue, 23 Sep 2025 22:47:14 +0000
Gerrit-HasComments: Yes

Reply via email to