On Fri, Sep 15, 2017 at 3:03 PM, Andres Freund <andres@anarazel.de> wrote:
> On 2017-09-04 18:14:39 +1200, Thomas Munro wrote:
>> Thanks for the review and commits so far. Here's a rebased, debugged
>> and pgindented version of the remaining patches.
>
> I've pushed this with minor modifications:
Thank you!
> - added typedefs to typedefs.list
Should I do this manually with future patches?
> - re-pgindented, there were some missing reindents in headers
> - added a very brief intro into session.c, moved some content repeated
> in various places to the header - some of them were bound to become
> out-of-date due to future uses of the facility.
> - moved NULL setting in detach hook directly after the respective
> resource deallocation, for the not really probable case of it being
> reinvoked due to an error in a later dealloc function
>
> Two remarks:
> - I'm not sure I like the order in which things are added to the typemod
> hashes, I wonder if some more careful organization could get rid of
> the races. Doesn't seem critical, but would be a bit nicer.
I will have a think about whether I can improve that. In an earlier
version I did things in a different order and had different problems.
The main hazard to worry about here is that you can't let any typmod
number escape into shmem where it might be read by others (for example
a concurrent session that wants a typmod for a TupleDesc that happens
to match) until the typmod number is resolvable back to a TupleDesc
(meaning you can look it up in shared_typmod_table). Not
wasting/leaking memory in various failure cases is a secondary (but
obviously important) concern.
> - I'm not yet quite happy with the Session facility. I think it'd be
> nicer if we'd a cleaner split between the shared memory notion of a
> session and the local memory version of it. The shared memory version
> would live in a ~max_connections sized array, referenced from
> PGPROC. In a lot of cases it'd completely obsolete the need for a
> shm_toc, because you could just store handles etc in there. The local
> memory version then would just store local pointers etc into that.
>
> But I think we can get there incrementally.
+1 to all of the above. I fully expect this to get changed around quite a lot.
I'll keep an eye out for problem reports.
--
Thomas Munro
http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers