Commit c7654f6a authored by Tom Lane's avatar Tom Lane

Fix representation of SORT_TYPE_STILL_IN_PROGRESS.

It turns out that the code did indeed rely on a zeroed
TuplesortInstrumentation.sortMethod field to indicate
"this worker never did anything", although it seems the
issue only comes up during certain race-condition-y cases.

Hence, rearrange the TuplesortMethod enum to restore
SORT_TYPE_STILL_IN_PROGRESS to having the value zero,
and add some comments reinforcing that that isn't optional.

Also future-proof a loop over the possible values of the enum.
sizeof(bits32) happened to be the correct limit value,
but only by purest coincidence.

Per buildfarm and local investigation.

Discussion: https://postgr.es/m/12222.1586223974@sss.pgh.pa.us
parent 4c04be9b
...@@ -2762,14 +2762,14 @@ show_incremental_sort_group_info(IncrementalSortGroupInfo *groupInfo, ...@@ -2762,14 +2762,14 @@ show_incremental_sort_group_info(IncrementalSortGroupInfo *groupInfo,
List *methodNames = NIL; List *methodNames = NIL;
/* Generate a list of sort methods used across all groups. */ /* Generate a list of sort methods used across all groups. */
for (int bit = 0; bit < sizeof(bits32); ++bit) for (int bit = 0; bit < NUM_TUPLESORTMETHODS; bit++)
{ {
if (groupInfo->sortMethods & (1 << bit)) TuplesortMethod sortMethod = (1 << bit);
if (groupInfo->sortMethods & sortMethod)
{ {
TuplesortMethod sortMethod = (1 << bit); const char *methodName = tuplesort_method_name(sortMethod);
const char *methodName;
methodName = tuplesort_method_name(sortMethod);
methodNames = lappend(methodNames, unconstify(char *, methodName)); methodNames = lappend(methodNames, unconstify(char *, methodName));
} }
} }
......
...@@ -62,18 +62,24 @@ typedef struct SortCoordinateData *SortCoordinate; ...@@ -62,18 +62,24 @@ typedef struct SortCoordinateData *SortCoordinate;
* TuplesortInstrumentation can't contain any pointers because we * TuplesortInstrumentation can't contain any pointers because we
* sometimes put it in shared memory. * sometimes put it in shared memory.
* *
* TuplesortMethod is used in a bitmask in Increment Sort's shared memory * The parallel-sort infrastructure relies on having a zero TuplesortMethod
* instrumentation so needs to have each value be a separate bit. * indicate that a worker never did anything, so we assign zero to
* SORT_TYPE_STILL_IN_PROGRESS. The other values of this enum can be
* OR'ed together to represent a situation where different workers used
* different methods, so we need a separate bit for each one. Keep the
* NUM_TUPLESORTMETHODS constant in sync with the number of bits!
*/ */
typedef enum typedef enum
{ {
SORT_TYPE_STILL_IN_PROGRESS = 1 << 0, SORT_TYPE_STILL_IN_PROGRESS = 0,
SORT_TYPE_TOP_N_HEAPSORT = 1 << 1, SORT_TYPE_TOP_N_HEAPSORT = 1 << 0,
SORT_TYPE_QUICKSORT = 1 << 2, SORT_TYPE_QUICKSORT = 1 << 1,
SORT_TYPE_EXTERNAL_SORT = 1 << 3, SORT_TYPE_EXTERNAL_SORT = 1 << 2,
SORT_TYPE_EXTERNAL_MERGE = 1 << 4 SORT_TYPE_EXTERNAL_MERGE = 1 << 3
} TuplesortMethod; } TuplesortMethod;
#define NUM_TUPLESORTMETHODS 4
typedef enum typedef enum
{ {
SORT_SPACE_TYPE_DISK, SORT_SPACE_TYPE_DISK,
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment