You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudberry.apache.org by "fairyfar (via GitHub)" <gi...@apache.org> on 2026/03/29 01:23:12 UTC

[PR] Fix the issue of duplicate counting of num_executed in gp_toolkit.gp_resgroup_status [cloudberry]

fairyfar opened a new pull request, #1646:
URL: https://github.com/apache/cloudberry/pull/1646

   <!-- Thank you for your contribution to Apache Cloudberry (Incubating)! -->
   
   ### What does this PR do?
   
   Fix the issue of duplicate counting of num_executed in gp_toolkit.gp_resgroup_status
   This is a defect of the original GPDB.
   When enabling resource group management and the transaction switches from "Assigned" to "Bypass" state, the "num_executed" counter is repeatedly counted.
   
   **Reproducing test cases:**
   
   1. Enable resource group: set gp_resource_manager to "group".
   
   2. Log in with the default superuser.
   
   3. Execute gp_toolkit.gp_resgroup_status twice.
   
   ```sql
   SELECT groupname, num_executed FROM gp_toolkit.gp_resgroup_status WHERE groupname='admin_group';
    groupname   | num_executed 
   -------------+--------------
    admin_group | 31           
   (1 row)
   SELECT groupname, num_executed FROM gp_toolkit.gp_resgroup_status WHERE groupname='admin_group';
    groupname   | num_executed 
   -------------+--------------
    admin_group | 33           
   (1 row)
   ```
   
   The test case has been added to:
   
   ‎src/test/isolation2/sql/resgroup/resgroup_bypass.sql
   
   ### Type of Change
   - [x] Bug fix (non-breaking change)
   - [ ] New feature (non-breaking change)
   - [ ] Breaking change (fix or feature with breaking changes)
   - [ ] Documentation update
   
   ### Breaking Changes
   <!-- Remove if not applicable. If yes, explain impact and migration path -->
   
   ### Test Plan
   <!-- How did you test these changes? -->
   - [ ] Unit tests added/updated
   - [x] Integration tests added/updated
   - [x] Passed `make installcheck`
   - [ ] Passed `make -C src/test installcheck-cbdb-parallel`
   
   ### Impact
   <!-- Remove sections that don't apply -->
   **Performance:**
   <!-- Any performance implications? -->
   
   **User-facing changes:**
   <!-- Any changes visible to users? -->
   
   **Dependencies:**
   <!-- New dependencies or version changes? -->
   
   ### Checklist
   - [ ] Followed [contribution guide](https://cloudberry.apache.org/contribute/code)
   - [ ] Added/updated documentation
   - [ ] Reviewed code for security implications
   - [ ] Requested review from [cloudberry committers](https://github.com/orgs/apache/teams/cloudberry-committers)
   
   ### Additional Context
   <!-- Any other information that would help reviewers? Remove if none -->
   
   ### CI Skip Instructions
   <!--
   To skip CI builds, add the appropriate CI skip identifier to your PR title.
   The identifier must:
   - Be in square brackets []
   - Include the word "ci" and either "skip" or "no"
   - Only use for documentation-only changes or when absolutely necessary
   -->
   
   ---
   <!-- Join our community:
   - Mailing list: [dev@cloudberry.apache.org](https://lists.apache.org/list.html?dev@cloudberry.apache.org) (subscribe: dev-subscribe@cloudberry.apache.org)
   - Discussions: https://github.com/apache/cloudberry/discussions -->
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org


Re: [PR] Fix the issue of duplicate counting of num_executed in gp_toolkit.gp_resgroup_status [cloudberry]

Posted by "leborchuk (via GitHub)" <gi...@apache.org>.
leborchuk commented on code in PR #1646:
URL: https://github.com/apache/cloudberry/pull/1646#discussion_r3014407239


##########
src/backend/utils/resgroup/resgroup.c:
##########
@@ -3700,7 +3700,6 @@ check_and_unassign_from_resgroup(PlannedStmt* stmt)
 	} while (!groupIncBypassedRef(&groupInfo));
 
 	bypassedGroup = groupInfo.group;
-	bypassedGroup->totalExecuted++;

Review Comment:
   Quite cumbersome logic with a totalExecuted counter. We have 5 items where we increase the counter. But I checked the source and saw that check_and_unassign_from_resgroup means that no active transactions are going to be executed. So there is no need to increase the counter here.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org


Re: [PR] Fix the issue of duplicate counting of num_executed in gp_toolkit.gp_resgroup_status [cloudberry]

Posted by "tuhaihe (via GitHub)" <gi...@apache.org>.
tuhaihe merged PR #1646:
URL: https://github.com/apache/cloudberry/pull/1646


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org