Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve ChunkAppend parallel worker assignment #1465

Merged
merged 1 commit into from Oct 7, 2019

Conversation

svenklemm
Copy link
Member

The initial iteration of assigning multiple workers per subplan
would always assign a fixed amount of workers per child. This
patch changes the logic to assign each child one worker and then
moving to the next subplan, iterating until all subplans are finished.
This will lead to a much better distribution of workers especially
when the children are not well balanced.

Copy link
Contributor

@cevian cevian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we just point out any remaining differences in the commit msg with core PG logic, or if there arent any point that out too.

@codecov
Copy link

codecov bot commented Oct 6, 2019

Codecov Report

Merging #1465 into master will decrease coverage by 8.82%.
The diff coverage is 91.17%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1465      +/-   ##
==========================================
- Coverage   90.22%   81.39%   -8.83%     
==========================================
  Files         117      117              
  Lines       17610    16896     -714     
==========================================
- Hits        15888    13753    -2135     
- Misses       1722     3143    +1421
Flag Coverage Δ
#cron 81.39% <91.17%> (-8.83%) ⬇️
#pr 81.39% <91.17%> (+0.01%) ⬆️
Impacted Files Coverage Δ
src/planner.c 86.11% <100%> (-5.52%) ⬇️
src/chunk_append/exec.c 95.84% <90.62%> (+0.05%) ⬆️
tsl/src/nodes/gapfill/gapfill.c 45.45% <0%> (-54.55%) ⬇️
src/chunk_insert_state.c 31.62% <0%> (-54.42%) ⬇️
src/chunk.h 0% <0%> (-50%) ⬇️
src/utils.h 0% <0%> (-50%) ⬇️
src/plan_add_hashagg.c 46.87% <0%> (-40.63%) ⬇️
src/plan_expand_hypertable.c 43.28% <0%> (-40.22%) ⬇️
src/copy.c 44.67% <0%> (-37.66%) ⬇️
src/scan_iterator.h 0% <0%> (-33.34%) ⬇️
... and 85 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ddb8f46...d58054c. Read the comment docs.

@svenklemm svenklemm added this to the 1.5.0 milestone Oct 6, 2019
@svenklemm svenklemm force-pushed the parallel_chunkappend3 branch 3 times, most recently from ddb12b9 to c701a8c Compare October 7, 2019 10:44
The initial implementation of assigning multiple workers per subplan
would always assign a fixed amount of workers per child. This
patch changes the logic to assign each child one worker and then
moving to the next subplan, iterating until all subplans are finished.
This will lead to a much better distribution of workers especially
when the children are not well balanced.
@svenklemm svenklemm force-pushed the parallel_chunkappend3 branch 2 times, most recently from 7e230c6 to d58054c Compare October 7, 2019 11:14
@svenklemm svenklemm merged commit 4cd645e into timescale:master Oct 7, 2019
@svenklemm svenklemm deleted the parallel_chunkappend3 branch October 10, 2019 13:45
@cevian cevian added the fixup label Oct 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants