On Fri, Aug 10, 2018 at 6:45 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
> I haven't studied the complete problem, but the way you are
> propagating the information to parallel workers looks correct to me.
> Few minor comments:
>
> 1.
> +void
> +RestoreRelationMap(char *startAddress)
> +{
> + SerializedActiveRelMaps *relmaps;
> +
> + if (active_shared_updates.num_mappings != 0 ||
> + active_local_updates.num_mappings != 0 ||
> + pending_shared_updates.num_mappings != 0 ||
> + pending_local_updates.num_mappings != 0)
> + elog(ERROR, "parallel worker has existing mappings");
> ..
>
> Shouldn't above be Assert?
This was based on AtPrepare_RelationMap().
> 2.
> +void
> +SerializeRelationMap(Size maxSize, char *startAddress)
> +{
> + SerializedActiveRelMaps *relmaps;
> +
> + relmaps = (SerializedActiveRelMaps *) startAddress;
> + relmaps->active_shared_updates = active_shared_updates;
> + relmaps->active_local_updates = active_local_updates;
> ..
> }
>
> Some of the other serialize functions use maxSize for Asserts. See
> SerializeComboCIDState. I think we can do without that as well, but
> it makes code consistent.
I'll put an assert in there.
Thanks
--
Peter Geoghegan