Re: Parallel Append implementation

On 12 March 2017 at 08:50, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>> However, Ashutosh's response made me think of something: one thing is
>> that we probably do want to group all of the non-partial plans at the
>> beginning of the Append so that they get workers first, and put the
>> partial plans afterward. That's because the partial plans can always
>> be accelerated by adding more workers as they become available, but
>> the non-partial plans are just going to take as long as they take - so
>> we want to start them as soon as possible. In fact, what we might
>> want to do is actually sort the non-partial paths in order of
>> decreasing cost, putting the most expensive one first and the others
>> in decreasing order after that - and then similarly afterward with the
>> partial paths. If we did that, we wouldn't need to store a bitmapset
>> OR two separate lists. We could just store the index of the first
>> partial plan in the list. Then you can test whether a path is partial
>> by checking whether this_index >= first_partial_index.

Attached is an updated patch v7, which does the above. Now,
AppendState->subplans has all non-partial subplans followed by all
partial subplans, with the non-partial subplans in the order of
descending total cost. Also, for convenience, the AppendPath also now
has similar ordering in its AppendPath->subpaths. So there is a new
field both in Append and AppendPath : first_partial_path/plan, which
has value 0 if there are no non-partial subpaths.

Also the backend now scans reverse, so that it does not take up the
most expensive path.

There are also some changes in the costing done. Now that we know that
the very first path is the costliest non-partial path, we can use its
total cost as the total cost of Append in case all the partial path
costs are lesser.

Modified/enhanced an existing test scenario in
src/test/regress/select_parallel.sql so that Parallel Append is
covered.

As suggested by Robert, since pa_info->pa_finished was the only field
in pa_info, removed the ParallelAppendDescData.pa_info structure, and
instead brought pa_info->pa_finished into ParallelAppendDescData.

>>> +static inline void
>>> +exec_append_scan_first(AppendState *appendstate)
>>> +{
>>> + appendstate->as_whichplan = 0;
>>> +}
>>>
>>> I don't think this is buying you anything, and suggest backing it out.
>>
>> This is required for sequential Append, so that we can start executing
>> from the first subplan.
>
> My point is that there's really no point in defining a static inline
> function containing one line of code. You could just put that line of
> code in whatever places need it, which would probably be more clear.

Did the same.