On Mon, Apr 06, 2020 at 05:47:48PM -0400, James Coleman wrote:
On Mon, Apr 6, 2020 at 5:40 PM Tomas Vondra
<tomas.von...@2ndquadrant.com> wrote:
On Mon, Apr 06, 2020 at 11:12:32PM +0200, Tomas Vondra wrote:
>On Mon, Apr 06, 2020 at 04:54:38PM -0400, Alvaro Herrera wrote:
>>On 2020-Apr-06, Tom Lane wrote:
>>
>>>Locally, things pass without force_parallel_mode, but turning it on
>>>produces failures that look similar to rhinoceros's (didn't examine
>>>other BF members).
>>
>>FWIW I looked at the eight failures there were about fifteen minutes ago
>>and they were all identical. I can confirm that, in my laptop, the
>>tests work without that GUC, and fail in exactly that way with it.
>>
>
>Yes, there's a thinko in show_incremental_sort_info() and it returns too
>soon. I'll push a fix in a minute.
>
OK, I've pushed a fix - this should make the buildfarm happy again.
It however seems to me a bit more needs to be done. The fix makes
show_incremental_sort_info closer to show_sort_info, but not entirely
because IncrementalSortState does not have sort_Done flag so it still
depends on (fullsortGroupInfo->groupCount > 0). I haven't noticed that
before, but not having that flag seems a bit weird to me.
It also seems possibly incorrect - we may end up with
fullsortGroupInfo->groupCount == 0
prefixsortGroupInfo->groupCount > 0
but we won't print anything.
This shouldn't ever be possible, because the only way we get any
prefix groups at all is if we've already sorted a full sort group
during the mode transition.
James, any opinion on this? I'd say we should restore the sort_Done flag
and make it work as in plain Sort. Or some comment explaining why
depending on the counts is OK (assuming it is).
There's previous email traffic on this thread about that (I can look
it up later this evening), but the short of it is that I believe that
relying on the group count is actually more correct than a sort_Done
flag in the case of incremental sort (in contrast to regular sort).
OK. Maybe we should add a comment to explain.c saying it's OK.
I've pushed a fix for failures due to different planned workers (in the
test I added to show changes due to add_partial_path tweaks).
It seems we're not out of the woods yet, though. rhinoceros and
sidewinder failed with something like this:
Sort Method: quicksort Memory: NNkB
+ Sort Method: unknown Disk: NNkB
Would you mind investigating at it?
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
