Skip to content

Optimize PlanFragment Serialization#13451

Merged
wenleix merged 1 commit intoprestodb:masterfrom
NikhilCollooru:PlanFragment-optimization
Oct 28, 2019
Merged

Optimize PlanFragment Serialization#13451
wenleix merged 1 commit intoprestodb:masterfrom
NikhilCollooru:PlanFragment-optimization

Conversation

@NikhilCollooru
Copy link
Contributor

@NikhilCollooru NikhilCollooru commented Sep 25, 2019

Do not send PlanFragment's jsonRepresentation, statsAndCosts to workers as they don't need it.
They are only for the coordinator UI purpose.

== NO RELEASE NOTE ==

@NikhilCollooru NikhilCollooru changed the title [WIP] PlanFragment optimization [WIP] PlanFragment optimization - please do not review Sep 25, 2019
@NikhilCollooru NikhilCollooru force-pushed the PlanFragment-optimization branch from ebf06cd to c19c5e4 Compare October 16, 2019 22:26
@NikhilCollooru NikhilCollooru changed the title [WIP] PlanFragment optimization - please do not review PlanFragment optimization Oct 18, 2019
@NikhilCollooru NikhilCollooru changed the title PlanFragment optimization Optimize PlanFragment Oct 18, 2019
@NikhilCollooru NikhilCollooru force-pushed the PlanFragment-optimization branch from c19c5e4 to 18a592f Compare October 18, 2019 20:33
@wenleix
Copy link
Contributor

wenleix commented Oct 24, 2019

I did some performance profiling in production workload and it turns out serializing splits is dominating the TaskUpdateRequest serialization CPU. Serializing plan fragment is about 20%-35% of the time spent on serializing splits (i.e. TaskSource). Thus once #8964 get fixed, we no longer worry about time spent on serializing plan fragment :) .

This PR is still valuable since it reduce the bytes sent to worker from coordinator. Too large update has been demonstrated to cause troubles (see, e.g. #13360) and aligns with the Coordinator reliability efforts such as #13584 .

@NikhilCollooru NikhilCollooru force-pushed the PlanFragment-optimization branch from 18a592f to d775fb0 Compare October 25, 2019 01:02
@wenleix wenleix self-assigned this Oct 25, 2019
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this comment as well

@wenleix
Copy link
Contributor

wenleix commented Oct 25, 2019

For the commit subject, I would say "Optimize PlanFragment serialization".

"PlanFragment" is not an action, you cannot optimize "PlanFragment" :) .

@NikhilCollooru NikhilCollooru force-pushed the PlanFragment-optimization branch from d775fb0 to a3464d1 Compare October 25, 2019 22:46
Dont send PlanFragment's jsonRepresentation, statsAndCosts to workers
as they do not need it.
@NikhilCollooru NikhilCollooru force-pushed the PlanFragment-optimization branch from a3464d1 to 325c37d Compare October 25, 2019 22:48
@NikhilCollooru NikhilCollooru changed the title Optimize PlanFragment Optimize PlanFragment Serialization Oct 26, 2019
Copy link
Contributor

@wenleix wenleix left a comment

Choose a reason for hiding this comment

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

LGTM.

@wenleix wenleix merged commit 2be543e into prestodb:master Oct 28, 2019
@NikhilCollooru NikhilCollooru deleted the PlanFragment-optimization branch October 28, 2019 21:10
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.

4 participants