On 1/6/15, 10:33 AM, Robert Haas wrote:
>> >Entrypints?
> Already noted by Andres; fixed in the attached version.
Perhaps we only parallelize while drinking beer... ;)
CreateParallelContext(): Does it actually make sense to have nworkers=0? ISTM that's a bogus case. Also, since the
numberof workers will normally be determined dynamically by the planner, should that check be a regular conditional
insteadof just an Assert?
In LaunchParallelWorkers() the "Start Workers" comment states that we give up registering more workers if one fails to
register,but there's nothing in the if condition to do that, and I don't see RegisterDynamicBackgroundWorker() doing it
either.Is the comment just incorrect?
SerializeTransactionState(): instead of looping through the transaction stack to calculate nxids, couldn't we just set
itto maxsize - sizeof(TransactionId) * 3? If we're looping a second time for safety a comment explaining that would be
useful...
sequence.c: Is it safe to read a sequence value in a parallel worker if the cache_value is > 1?
This may be a dumb question, but for functions do we know that all pl's besides C and SQL use SPI? If not I think they
couldend up writing in a worker.
@@ -2968,7 +2969,8 @@ ProcessInterrupts(void) errmsg("canceling statement due to user request")));
} }
- /* If we get here, do nothing (probably, QueryCancelPending was reset) */
+ if (ParallelMessagePending)
+ HandleParallelMessageInterrupt(false);
ISTM it'd be good to leave that comment in place (after the if).
RestoreComboCIDState(): Assert(!comboCids || !comboHash): Shouldn't that be &&? AIUI both should always be either set
or0.
Typo: + elog(ERROR, "cannot update SecondarySnapshpt during a parallel operation");
The comment for RestoreSnapshot refers to set_transaction_snapshot, but that doesn't actually exist (or isn't
referenced).
--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com
