From: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
---|---|
To: | Peter Geoghegan <pg(at)bowt(dot)ie> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, deathlock13(at)gmail(dot)com, PostgreSQL mailing lists <pgsql-bugs(at)lists(dot)postgresql(dot)org>, Robert Haas <robertmhaas(at)gmail(dot)com> |
Subject: | Re: BUG #15309: ERROR: catalog is missing 1 attribute(s) for relid 760676 when max_parallel_maintenance_workers > 0 |
Date: | 2018-08-10 13:45:48 |
Message-ID: | CAA4eK1JHOg1a3UuZmRSko8+XcjEOCeqGx3N+MoGWSAL+GVh7JA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-bugs |
On Thu, Aug 9, 2018 at 11:28 PM, Peter Geoghegan <pg(at)bowt(dot)ie> wrote:
> On Thu, Aug 9, 2018 at 10:34 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> I took a quick look at these. The v11 patch seems OK as far as it goes,
>> but I wonder if you shouldn't also include the RelationMapUpdateMap
>> hunk from the master patch, ie adding rejection of map changes in parallel
>> mode.
>
> Good idea. I'll do that.
>
>> I don't have any objection to the master patch, but it'd be good to get
>> a +1 from someone who's spent more time with the parallelism
>> infrastructure than I have.
>
> That would be my preference too, but commit 29d58fd3 is very analogous
> to the proposed master branch fix, so I feel that it's reasonable to
> proceed without final approval from someone like Robert or Amit.
>
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?
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.
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
From | Date | Subject | |
---|---|---|---|
Next Message | Bruce Momjian | 2018-08-10 16:12:45 | Re: [PG_UPGRADE] 9.6 to 10.5 |
Previous Message | DEGLAVE Remi | 2018-08-10 12:51:29 | [PG_UPGRADE] 9.6 to 10.5 |