Repository navigation
Add gang-parallel scan to pxf_fdw - #61
MisterRaindrop wants to merge 10 commits into
Conversation
|
@ostinru My approach to kernel parallel processing is still too simplistic. Maybe I will change or refactor later. |
|
All deployments are local.
When exploring parallelization, the good news is that parallelization does indeed improve efficiency. For small data volumes, the improvement is not obvious and may even be less efficient than non-parallel processing. Only when the data volume is large does it show a noticeable improvement. However, the current improvement still falls short of the expected level. Theoretically, the speedup factor should be almost equal to the number of workers. The reason it hasn’t reached the expected level may be due to bottlenecks in I/O or CPU. Further exploration will be conducted in the future. |
9f68aec to
71f0bed
Compare
| EXPLAIN SELECT count(*) FROM pxf_parallel_enabled; | ||
| QUERY PLAN | ||
| ---------------------------------------------------------------------------------------------- | ||
| Gather Motion 1:1 (slice1; segments: 1) (cost=XXX rows=NNN width=NN) |
There was a problem hiding this comment.
The plan seems not correct?
There was a problem hiding this comment.
You're right, it wasn't. That expected output was written by hand: a PostgreSQL Gather with Workers Planned under a one-segment Gather Motion. Cloudberry plans a parallel pxf_fdw scan as gang-parallel workers under the Gather Motion, so there is no Gather node and no Workers Planned, and CI runs on three segments.
3981ccb replaced it with the plan Cloudberry actually produces:
Finalize Aggregate
-> Gather Motion 12:1 (slice1; segments: 12)
-> Partial Aggregate
-> Parallel Foreign Scan on pxf_parallel_enabled
It uses EXPLAIN (COSTS OFF) and normalizes the Gather Motion size, so the test doesn't depend on the number of segments. ParallelScanTest.testParallelExplain passes in the features_fdw jobs of the latest CI run. Sorry for the slow reply.
There was a problem hiding this comment.
A correction to my reply above: testParallelExplain passing in CI didn't show anything. atmsort.pl sends statements that look like EXPLAIN ... SELECT to explain.pl, which isn't installed in the CI image, so the actual and expected output were both empty. 8757e6e returns the plan from a function as ordinary query rows, so it is really compared now. The expected plan is the same as before. I checked that a wrong expected plan fails, with and without explain.pl.
- Added PxfBridgeImportStartVirtual function to manage imports with virtual segment IDs. - Updated PxfFdwScanState structure to include fields for gang-parallel execution. - Enhanced foreign scan functions to support gang-parallel mode, ensuring unique fragment distribution among workers. - Implemented initialization and cleanup routines for gang-parallel state management.
71f0bed to
77ff52c
Compare
A parallel pxf_fdw scan splits fragments statically among all gang workers of the slice, so every worker must execute the scan. A Parallel Append runs each child in only some of the workers, and the fragments assigned to the other workers were never read. UNION ALL of parallel foreign tables and partitioned tables with foreign partitions returned fewer rows, and the count varied between runs. Offer the partial path only for a top-level base relation. Partitions and flattened UNION ALL members are append rel children; other set-operation branches and subqueries are planned at query_level > 1, and their partial paths can also end up under a Parallel Append. enable_parallel is valid at both table and server level, but PxfGetOptions lets the last value win and lists the server after the table, so a table could not turn off parallel enabled on its server. Keep the first value so the table overrides the server. Add tests for UNION ALL, partitioned tables and option precedence.
Cloudberry runs a parallel pxf_fdw scan as gang-parallel workers under a Gather Motion (e.g. "Gather Motion 12:1" for 3 segments x 4 workers); there is no PG Gather node and no "Workers Planned/Launched". The expected plans also hard-coded one segment while CI uses the 3-segment demo cluster, and the match substitutions were only in the .ans files. Use EXPLAIN (COSTS OFF), put the match substitutions in both .sql and .ans, and normalize the Gather Motion size and the EXPLAIN ANALYZE row counts so the tests pass with any number of segments.
ParallelScanTest.beforeClass failed in CI because runQuery treats any NOTICE as an error: DROP ... IF EXISTS raises one when the object does not exist, and a partitioned table without DISTRIBUTED BY raises the distribution key notice. Run the DROP statements with ignoreFail and add DISTRIBUTED BY to the partitioned table. Exclude the parallel SQL test files from the Apache Rat check, like the other sqlrepo tests.
@MisterRaindrop , is it ready for review? |
it is ready for review |
|
Review by |
The explain tests ran EXPLAIN directly. atmsort.pl replaces the output of an "EXPLAIN ... SELECT" statement with the output of explain.pl, and the CI image has no explain.pl, so the plan was dropped from both the actual and the expected output and any plan passed. An expected file with a made-up plan node passed as well. Return the plan lines from a plpgsql helper, pxf_parallel_plan(opts, q), which builds the EXPLAIN statement itself and masks the Gather Motion size and actual row counts. The sql files then contain no EXPLAIN statement, and the plan lines are compared as ordinary query results, with or without explain.pl. Run the explain tests only with pxf_fdw, like the append and option precedence tests: an external table is never scanned in parallel.
Changes from the review of the gang-parallel scan: - Do not offer the parallel path for a table with a reject_limit. Each gang worker counted rejected rows on its own, so a segment with W workers accepted up to W times the limit, and a percentage limit was checked against each worker's smaller row count. A query that failed without parallelism succeeded with it. - Estimate the parallel path from the rows the planner assumes for the non-parallel path. set_foreign_pathlist() multiplies the rows of a strewn ForeignPath by numsegments * 10, but only in the pathlist, so the partial path was estimated 30 times lower. With that estimate the planner redistributed and sorted all rows for GROUP BY instead of using a partial HashAggregate. Both paths now charge cpu_tuple_cost per estimated row, and the parallel path divides it by the number of workers. This raises the cost of every PXF scan by a constant. The non-parallel plans of 16 queries (joins, subqueries, set operations) did not change, with both the Postgres planner and ORCA. - Decide whether to split fragments from the plan's parallel_aware flag only, not from the enable_parallel option read again at execution. - Start the import in one place, StartScan, and override the segment headers in PxfBridgeImportStart, instead of the copies PxfBridgeImportStartVirtual and InitCopyStateVirtual. - A rescan closes the import and IterateForeignScan starts a new one. The planner never puts a partial path where it is rescanned, so the rescan test covers ordinary scans only; say so in a comment. - Keep PxfCopyFromErrorCallback, pxfEndForeignScan and the iterate loop as on main. Their rewrite was not needed for parallel scans. - Drop the unused worker_index field and utils/guc.h.
Describe where the option can be set, the session settings that a parallel plan needs, when a table is not scanned in parallel, and the extra load on the PXF server.
pxf_regress applies global_init_file before the matchsubs of a test, and it renames DETAIL to CONTEXT and Foreign table to External table. The test's patterns matched the original text, so they never applied, and the expected output kept the path of the data on the machine it was generated on. Match the renamed text, so the error context lines, which contain the data path and the record number, are ignored.
|
Thanks @tuhaihe, the review was useful and the repros made each item easy to check. Addressed in 8757e6e, f4a487c, ce0afaf and d722f96: 1. Reject limit per worker. No parallel path is offered for a table with a 2. Costing and row estimate. The 30x gap comes from What this changes for GROUP BY: the parallel path is no longer chosen. Cloudberry does not plan a parallel partial HashAggregate over randomly distributed input, even with the parallel costs set to 0 (a randomly distributed heap table behaves the same way). So the plan is now the non-parallel Streaming Partial HashAggregate: no slowdown on CSV, but also no speedup on Parquet. COUNT/SUM, filters and joins still use the parallel scan. I did not size the worker count or skip small tables, as PXF does not know the data size at plan time; the docs say a small table can be slower. 3. Rescan test. I couldn't put a partial path on a rescanned side either, so I simplified the rescan to close the import and let the next iterate start a new one, and said in a comment that a partial path is never rescanned. 4. Duplicated import setup. 5. PxfCopyFromErrorCallback. Restored to main, together with Nits. One more fix: the EXPLAIN tests compared nothing. Happy to have the benchmark re-run on d722f96. |
Add the header to the new test class, and move the Apache Rat excludes of the features/parallel SQL and expected files to the section for files introduced by Cloudberry.
|
Thanks for the update! Below are a re-review and a cluster benchmark of the new head (7f708c1), both prepared with Claude Code. The results look good: COUNT/SUM still get ~1.8x (CSV) / ~1.7x (Parquet) at 4 workers, the reject_limit issue is fixed, and CSV GROUP BY is no longer slower than main. The one trade-off worth noting is that Parquet GROUP BY no longer uses a parallel scan (1.70x → ~1.0x); details and a few minor suggestions are in the review below. Thanks for the great work! Re-review (review-comment-round2.md)Re-review of the new head (7f708c1)Thanks for the quick turnaround. I re-reviewed fa855b3..7f708c1 and re-ran the benchmark from the last round on the same EC2 setup, against both Summary: every item from the last review is addressed, and I found nothing blocking. COUNT/SUM speedups are unchanged, the reject-limit bug is fixed, and CSV GROUP BY is no longer slower than Last round's items
Good catch on the EXPLAIN tests comparing nothing; the plpgsql wrapper is a nice fix. Remaining points
From my side this is ready to merge once point 1 is agreed and points 4–5 are cleaned up. Benchmark report (test-report-round2.md)PR #61 benchmark, round 2:
|
| Dataset | count (old → new) | sum (old → new) | GROUP BY (old → new) |
|---|---|---|---|
| csv100m_24 | 1.33 → 1.33 | 1.34 → 1.35 | 1.03 → 1.00 |
| csv1g_24 | 1.71 → 1.68 | 1.56 → 1.60 | 1.00 → 0.96 |
| csv10g_24 | 1.81 → 1.83 | 1.85 → 1.85 | 1.01 → 0.98 |
| csv10g_3 | 1.81 → 1.82 | 1.84 → 1.86 | 1.01 → 0.98 |
| csv10g_12 | 1.81 → 1.83 | 1.83 → 1.86 | 1.01 → 0.98 |
| csv10g_48 | 1.81 → 1.81 | 1.82 → 1.85 | 1.00 → 0.98 |
| pq10g_24 | 1.69 → 1.71 | 1.67 → 1.67 | 1.70 → 0.98 |
At 2 and 8 workers:
| Query | p2 (old → new) | p8 (old → new) |
|---|---|---|
| csv10g_24 count | 1.56 → 1.62 | 1.79 → 1.79 |
| csv10g_24 GROUP BY | 0.92 → 0.98 | 1.07 → 0.98 |
| pq10g_24 GROUP BY | 1.48 → 0.99 | 1.79 → 0.99 |
Absolute times, median ms (new head)
| Dataset | Query | base | new, no parallel | new p2 | new p4 | new p8 | old p4 |
|---|---|---|---|---|---|---|---|
| csv10g_24 | count | 17,950 | 18,935 | 11,091 | 9,805 | 10,012 | 9,905 |
| csv10g_24 | sum | 19,013 | 19,365 | 11,930 | 10,260 | 10,369 | 10,270 |
| csv10g_24 | groupby | 20,965 | 21,475 | 21,288 | 21,291 | 21,308 | 20,692 |
| pq10g_24 | count | 39,946 | 40,470 | 28,194 | 23,408 | 23,226 | 23,586 |
| pq10g_24 | sum | 39,918 | 40,209 | 29,560 | 23,911 | 23,656 | 23,910 |
| pq10g_24 | groupby | 39,725 | 40,211 | 39,931 | 40,401 | 39,935 | 23,407 |
| csv1g_24 | count | 2,057 | 2,006 | 1,978 | 1,223 | 1,093 | 1,206 |
| csv100m_24 | count | 249 | 265 | 218 | 187 | 189 | 188 |
The new head looks about 3–5% slower than base without parallel on CSV, but that did not reproduce. An interleaved A/B run (base, new, base, new) on count(*) over csv10g_24 gave 17,973 / 17,935 ms on base and 18,011 / 17,917 ms on new. The gap in the table comes from when the runs happened, not from the PR.
GROUP BY plans (pq10g_24, p4)
- base: Gather 3:1 → Sort → Finalize HashAgg → Redistribute 3:3 → Streaming Partial HashAgg → Foreign Scan
- old head: Gather 12:1 → GroupAggregate → Sort → Redistribute 12:12 → Parallel Foreign Scan (rows=250)
- new head: same as base (Foreign Scan cost 50000..50300). GROUP BY no longer uses a parallel scan.
- new head, COUNT/SUM: Finalize Agg → Gather 12:1 → Partial Agg → Parallel Foreign Scan (rows=7500)
CPU (average %, out of 1600)
| Query | base | new p4 | old p4 |
|---|---|---|---|
| csv10g_24 sum | java 111 / postgres 287 | java 382 / postgres 986 | java 397 / postgres 991 |
| pq10g_24 count | java 298 / postgres 146 | java 923 / postgres 460 | java 917 / postgres 446 |
The CPU profile is the same as with the previous head: CSV is bound on the postgres side, Parquet on the Java side. 8 workers is no faster than 4.
Functional checks
| Check | Result |
|---|---|
| R1 reject_limit | Fixed. At p4/p8 the plan is a plain Foreign Scan. A rows limit of 10 and a percent limit of 2% both error like main (the previous head returned 23,952 and 2,328 rows instead). With LOG ERRORS and limit 100: 23,952 rows and 48 errors logged in every configuration. |
| R2 small tables | Still parallel, as documented. 100-row table: 25.0 ms parallel vs 21.8 ms not. 1 MB table: 29.8 vs 27.3 ms. |
| R3 non-parallel plans | No shape change. With optimizer=off, each PXF Foreign Scan's cost goes up by 300; nothing else changes. With ORCA, plans are byte-identical to main. |
| R4 end-of-scan cleanup | LIMIT 10: about +2.5 ms (59.3/59.0 → 61.4/61.9 ms). LIMIT 100000 and full scans: no change. Same PXF WARN count (126 "Remote connection closed by the client") on both builds. Cancels with statement_timeout (non-parallel, p4 CSV, p8 Parquet) were clean: no leftover backends, and the next query worked. No PANIC or FATAL. |
| R5 rescan | All four rescan queries match the non-parallel results. |
| R6 UNION ALL / partitions / hash join / skew | Results match non-parallel. Partitioned tables run their children as non-parallel scans under Parallel Append. |
#58
Change logs
pxf_fdw can now scan a foreign table with several gang workers per segment on Cloudberry. Parallel scan is off by default and is enabled with the
enable_paralleloption on the foreign server or the foreign table; a value on the table overrides the server.How it works:
enable_parallel, the session hasenable_parallel = onandmax_parallel_workers_per_gather > 0, and the Postgres planner is used. The plan is a Parallel Foreign Scan under a gang-parallel Gather Motion.s + w * Nout ofN * W(N segments, W workers) inX-GP-SEGMENT-ID/X-GP-SEGMENT-COUNT. The server's existing round-robin fragment split then gives each worker a disjoint part of what the segment would read. No PXF server change is needed.ParallelWorkerNumberOfSlice/TotalParallelWorkerNumberOfSlice, which are already on Cloudberry main.No parallel path is offered for:
reject_limit, because the limit is counted per process;UNION ALLmembers and subqueries, which may run under a Parallel Append that runs each child in only some of the workers.Rows and cost of the parallel path are derived from the non-parallel estimate divided by the number of workers. The non-parallel plans of 16 queries are unchanged with both the Postgres planner and ORCA.
Tests (
features/parallel, FDW mode): result correctness, single fragment, rescan, Parallel Append, option precedence, reject limit, and EXPLAIN. The option is documented infdw/PXF_FDW.md.Known limitations:
max_parallel_workers_per_gather, because PXF does not know the data size at plan time.Contributor's checklist
Here are some reminders before you submit your pull request:
🤖 Generated with Claude Code