You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudberry.apache.org by "avamingli (via GitHub)" <gi...@apache.org> on 2025/01/24 07:54:07 UTC

[PR] New Approach to investigate Parallel Execution of SELECT Plan in REFRESH Command. [cloudberry]

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

    I can't count how many times I've been wrong in this situation in parallel refresh materialized views:
    
   ![image](https://github.com/user-attachments/assets/22ef5e6d-bce5-4c27-94a5-d60b85ded5d2)
   and all we can do is retry.
   
   Previously, I developed the `refresh_compare` function in `cbdb_parallel.sql`, which relied on actual execution times, with the expectation that parallel execution would consistently outperform non-parallel execution. It worked well for a certain period of time.
   However, since our CI transitioned to Apache, resource limitations have led to frequent failures in parallel refresh tests, making it challenging to obtain reliable results.
   
   This commit introduces a new approach to determine if the SELECT part of the REFRESH command is executed in parallel within Cloudberry. Since REFRESH is a utility command, it cannot be analyzed using EXPLAIN. 
   The new method focuses on assessing whether the REFRESH plan is parallel, specifically for AO/AOCS materialized views. I have retained the original test cases to allow other databases built on Cloudberry to evaluate actual execution times, as the new approach only identifies parallel execution without measuring performance.
   
   Authored-by: Zhang Mingli avamingli@gmail.com
   
   <!-- Thank you for your contribution to Apache Cloudberry (Incubating)! -->
   
   Fixes #ISSUE_Number
   
   ### What does this PR do?
   <!-- Brief overview of the changes, including any major features or fixes -->
   
   ### Type of Change
   - [ ] 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
   - [ ] Integration tests added/updated
   - [ ] 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] New Approach to investigate Parallel Execution of SELECT Plan in REFRESH Command. [cloudberry]

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


##########
src/backend/commands/matview.c:
##########
@@ -826,6 +827,14 @@ refresh_matview_datafill(DestReceiver *dest, Query *query,
 
 	plan = pg_plan_query(query, queryString, CURSOR_OPT_PARALLEL_OK, NULL);
 
+	if (!refreshClause->skipData &&
+		enable_parallel &&
+		check_refresh_plan_parallel &&
+		plan_has_parallel(plan))
+	{
+		ereport(NOTICE, errmsg("Plan of REFRESH is parallel"));

Review Comment:
   if (client_min_messages == WARNING) then ereport ?



-- 
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] New Approach to investigate Parallel Execution of SELECT Plan in REFRESH Command. [cloudberry]

Posted by "avamingli (via GitHub)" <gi...@apache.org>.
avamingli commented on PR #892:
URL: https://github.com/apache/cloudberry/pull/892#issuecomment-2614209982

   After discussing with @yjhjstz , he suggested using debug_print_plan = on to view the plan tree for a REFRESH command. No code changes are required for this.
   
   However, since the REFRESH command does not support plan printing well (and there is no guarantee that utility commands will print plans), we encountered some buggy output: "could not dump unrecognized node type: 1001."
   
   Fixing this issue would divert us from our main objective, and it's uncertain how many such cases exist. Additionally, I believe there is no need to export the output to a file outside the database; a function to filter the output would suffice. 
   
   This is just a suggestion for anyone interested.


-- 
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] New Approach to investigate Parallel Execution of SELECT Plan in REFRESH Command. [cloudberry]

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


##########
src/backend/commands/matview.c:
##########
@@ -826,6 +827,14 @@ refresh_matview_datafill(DestReceiver *dest, Query *query,
 
 	plan = pg_plan_query(query, queryString, CURSOR_OPT_PARALLEL_OK, NULL);
 
+	if (!refreshClause->skipData &&
+		enable_parallel &&
+		check_refresh_plan_parallel &&
+		plan_has_parallel(plan))
+	{
+		ereport(NOTICE, errmsg("Plan of REFRESH is parallel"));

Review Comment:
   <img width="1902" alt="image" src="https://github.com/user-attachments/assets/92e41a67-e9d3-4348-86a9-6ab061e9f389" />
   
   
   
   



-- 
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] New Approach to investigate Parallel Execution of SELECT Plan in REFRESH Command. [cloudberry]

Posted by "avamingli (via GitHub)" <gi...@apache.org>.
avamingli closed pull request #892: New Approach to investigate Parallel Execution of SELECT Plan in REFRESH Command.
URL: https://github.com/apache/cloudberry/pull/892


-- 
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] New Approach to investigate Parallel Execution of SELECT Plan in REFRESH Command. [cloudberry]

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


##########
src/backend/commands/matview.c:
##########
@@ -826,6 +827,14 @@ refresh_matview_datafill(DestReceiver *dest, Query *query,
 
 	plan = pg_plan_query(query, queryString, CURSOR_OPT_PARALLEL_OK, NULL);
 
+	if (!refreshClause->skipData &&
+		enable_parallel &&
+		check_refresh_plan_parallel &&
+		plan_has_parallel(plan))
+	{
+		ereport(NOTICE, errmsg("Plan of REFRESH is parallel"));

Review Comment:
   It's better then, but what should the log level be? 
   We don’t want to send this to the server logs, and displaying a message like 'Plan of REFRESH is parallel' to users by default is generally unnecessary and could disrupt their experience. 
   Additionally, it complicates the adjustment of many test cases.
   



-- 
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] New Approach to investigate Parallel Execution of SELECT Plan in REFRESH Command. [cloudberry]

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


##########
src/backend/commands/matview.c:
##########
@@ -826,6 +827,14 @@ refresh_matview_datafill(DestReceiver *dest, Query *query,
 
 	plan = pg_plan_query(query, queryString, CURSOR_OPT_PARALLEL_OK, NULL);
 
+	if (!refreshClause->skipData &&
+		enable_parallel &&
+		check_refresh_plan_parallel &&
+		plan_has_parallel(plan))
+	{
+		ereport(NOTICE, errmsg("Plan of REFRESH is parallel"));

Review Comment:
   like this?
   ```c
   		if(client_min_messages == WARNING)
   			ereport(NOTICE, errmsg("Plan of REFRESH is parallel"));
   ```
   very wired, and `client_min_messages` should be used only for **client** side, isn't it?
   



-- 
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] New Approach to investigate Parallel Execution of SELECT Plan in REFRESH Command. [cloudberry]

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


##########
src/backend/commands/matview.c:
##########
@@ -826,6 +827,14 @@ refresh_matview_datafill(DestReceiver *dest, Query *query,
 
 	plan = pg_plan_query(query, queryString, CURSOR_OPT_PARALLEL_OK, NULL);
 
+	if (!refreshClause->skipData &&
+		enable_parallel &&
+		check_refresh_plan_parallel &&
+		plan_has_parallel(plan))
+	{
+		ereport(NOTICE, errmsg("Plan of REFRESH is parallel"));

Review Comment:
   yeah, I found another way:
   set debug_print_plan to on;
   enable_parallel=on;
   ```c++
   :numSlices 3 
      :slices[i].sliceIndex 0 
      :slices[i].parentIndex -1 
      :slices[i].gangType 4 
      :slices[i].numsegments 3 
      :slices[i].parallel_workers 0 
      :slices[i].segindex 0 
      :slices[i].directDispatch.isDirectDispatch false 
      :slices[i].directDispatch.contentIds <> 
      :slices[i].sliceIndex 1 
      :slices[i].parentIndex 0 
      :slices[i].gangType 2 
      :slices[i].numsegments 1 
      :slices[i].parallel_workers 0 
      :slices[i].segindex 2 
      :slices[i].directDispatch.isDirectDispatch false 
      :slices[i].directDispatch.contentIds <> 
      :slices[i].sliceIndex 2 
      :slices[i].parentIndex 1 
      :slices[i].gangType 3 
      :slices[i].numsegments 3 
      :slices[i].parallel_workers 2 
      :slices[i].segindex 0 
      :slices[i].directDispatch.isDirectDispatch false 
      :slices[i].directDispatch.contentIds <> 
      :rewindPlanIDs (b)
   ```
   :slices[i].parallel_workers = 2, that's diff from parallel close case `:slices[i].parallel_workers 0`.
   
   use udf parse output ,  no need to change code, no overhead.



-- 
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] New Approach to investigate Parallel Execution of SELECT Plan in REFRESH Command. [cloudberry]

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


##########
src/backend/commands/matview.c:
##########
@@ -826,6 +827,14 @@ refresh_matview_datafill(DestReceiver *dest, Query *query,
 
 	plan = pg_plan_query(query, queryString, CURSOR_OPT_PARALLEL_OK, NULL);
 
+	if (!refreshClause->skipData &&
+		enable_parallel &&
+		check_refresh_plan_parallel &&
+		plan_has_parallel(plan))
+	{
+		ereport(NOTICE, errmsg("Plan of REFRESH is parallel"));

Review Comment:
   I didn't get it, could you provide codes to introduce your idea?
   and:
   > How to print plan in REFRESH command,



-- 
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] New Approach to investigate Parallel Execution of SELECT Plan in REFRESH Command. [cloudberry]

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


##########
src/backend/commands/matview.c:
##########
@@ -826,6 +827,14 @@ refresh_matview_datafill(DestReceiver *dest, Query *query,
 
 	plan = pg_plan_query(query, queryString, CURSOR_OPT_PARALLEL_OK, NULL);
 
+	if (!refreshClause->skipData &&
+		enable_parallel &&
+		check_refresh_plan_parallel &&
+		plan_has_parallel(plan))
+	{
+		ereport(NOTICE, errmsg("Plan of REFRESH is parallel"));

Review Comment:
   ok, 
   step 1:
   ```sql
   postgres=# create table t_p(c1 int, c2 int) with(parallel_workers=8) distributed by(c1);
   CREATE TABLE
   postgres=#   insert into t_p select i, i+1 from generate_series(1, 10000000)i;
   INSERT 0 10000000
   postgres=#     create materialized view matv using ao_row as select sum(a.c2) as c2, avg(b.c1) as c1 from t_p a join t_p b on a.c1 = b.c1 with no data distributed by(c2);
   CREATE MATERIALIZED VIEW
   ```
   step2:
   ```sql
   bash -c 'psql -X postgres -c "set optimizer=off; set enable_parallel=on;set client_min_messages to log; set debug_print_plan=on; refresh materialized view matv;" &> /tmp/refresh_plan.out'
   ```
   
   
   step3:
   cat /tmp/refresh_plan.out, found `:slices[i].parallel_workers 2`;
   ```sql
   select wait_until_query_then_parse('/tmp/refresh_plan.out'); // TODO wait_until_query_then_parse
   ```



-- 
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] New Approach to investigate Parallel Execution of SELECT Plan in REFRESH Command. [cloudberry]

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


##########
src/backend/commands/matview.c:
##########
@@ -826,6 +827,14 @@ refresh_matview_datafill(DestReceiver *dest, Query *query,
 
 	plan = pg_plan_query(query, queryString, CURSOR_OPT_PARALLEL_OK, NULL);
 
+	if (!refreshClause->skipData &&
+		enable_parallel &&
+		check_refresh_plan_parallel &&
+		plan_has_parallel(plan))
+	{
+		ereport(NOTICE, errmsg("Plan of REFRESH is parallel"));

Review Comment:
   pls refer to `select wait_until_query_output_to_file('/tmp/bfv_cte.out');`



-- 
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] New Approach to investigate Parallel Execution of SELECT Plan in REFRESH Command. [cloudberry]

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


##########
src/backend/commands/matview.c:
##########
@@ -826,6 +827,14 @@ refresh_matview_datafill(DestReceiver *dest, Query *query,
 
 	plan = pg_plan_query(query, queryString, CURSOR_OPT_PARALLEL_OK, NULL);
 
+	if (!refreshClause->skipData &&
+		enable_parallel &&
+		check_refresh_plan_parallel &&
+		plan_has_parallel(plan))
+	{
+		ereport(NOTICE, errmsg("Plan of REFRESH is parallel"));

Review Comment:
   if (client_min_messages == WARNING) then ereport ?  then set once for this case.



-- 
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] New Approach to investigate Parallel Execution of SELECT Plan in REFRESH Command. [cloudberry]

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


##########
src/backend/commands/matview.c:
##########
@@ -826,6 +827,14 @@ refresh_matview_datafill(DestReceiver *dest, Query *query,
 
 	plan = pg_plan_query(query, queryString, CURSOR_OPT_PARALLEL_OK, NULL);
 
+	if (!refreshClause->skipData &&
+		enable_parallel &&
+		check_refresh_plan_parallel &&
+		plan_has_parallel(plan))
+	{
+		ereport(NOTICE, errmsg("Plan of REFRESH is parallel"));

Review Comment:
   > How to print plan in REFRESH command
   
   @yjhjstz  Could you show your steps? 
   Even with your  settings like set debug_print_plan to on;, I couldn't see plan tree when exec 'refresh materialized view mv;'



-- 
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