On Thu, Aug 29, 2013 at 8:12 PM, Jim Nasby wrote:
On 8/13/13 8:09 PM, Robert Haas wrote:
is removed, the segment automatically goes away (we could allow for
server-lifespan segments as well with only trivial changes, but I'm
not sure whether there are compelling use cases for that).
To clarify... you're talking something that would intentionally survive
postmaster restart? I don't see use for that either...
No, I meant something that would live as long as the postmaster and
die when it dies.
Ignorant question... is ResourceOwner related to memory contexts? If not,
would memory contexts be a better way to handle memory segment cleanup?
Nope. :-)
There are quite a few problems that this patch does not solve. First,
It also doesn't provide any mechanism for notifying backends of a new
segment. Arguably that's beyond the scope of dsm.c, but ISTM that it'd be
useful to have a standard method or three of doing that; perhaps just some
convenience functions wrapping the methods mentioned in comments.
I don't see that as being generally useful. Backends need to know
more than "there's a new segment", and in fact most backends won't
care about most new segments. A background worker needs to know about
the new segment *that it should attach*, but we have bgw_main_arg. If
we end up using this facility for any system-wide purposes, I imagine
we'll do that by storing the segment ID in the main shared memory
segment someplace.
Thanks to you and the rest of the folks at EnterpriseDB... dynamic shared
memory is something we've needed forever! :) Thanks.
Other comments...

+ * If the state file is empty or the contents are garbled, it probably
means
+ * that the operating system rebooted before the data written by the
previous
+ * postmaster made it to disk. In that case, we can just ignore it; any
shared
+ * memory from before the reboot should be gone anyway.

I'm a bit concerned about this; I know it was possible in older versions for
the global shared memory context to be left behind after a crash and needing
to clean it up by hand. Dynamic shared mem potentially multiplies that by
100 or more. I think it'd be worth changing dsm_write_state_file so it
always writes a new file and then does an atomic mv (or something similar).
I agree that the possibilities for leftover shared memory segments are
multiplied with this new facility, and I've done my best to address
that. However, I don't agree that writing the state file in a
different way would improve anything.
+ * If some other backend exited uncleanly, it might have corrupted
the
+ * control segment while it was dying. In that case, we warn and
ignore
+ * the contents of the control segment. This may end up leaving
behind
+ * stray shared memory segments, but there's not much we can do
about
+ * that if the metadata is gone.

Similar concern... in this case, would it be possible to always write
updates to an un-used slot and then atomically update a pointer? This would
be more work than what I suggested above, so maybe just a TODO for now...

Though... is there anything a dying backend could do that would corrupt the
control segment to the point that it would screw up segments allocated by
other backends and not related to the dead backend? Like marking a slot as
not used when it is still in use and isn't associated with the dead backend?
Sure. A messed-up backend can clobber the control segment just as it
can clobber anything else in shared memory. There's really no way
around that problem. If the control segment has been overwritten by a
memory stomp, we can't use it to clean up. There's no way around that
problem except to not the control segment, which wouldn't be better.
Should this (in dsm_attach)

+ * If you're hitting this error, you probably want to use attempt to

be

+ * If you're hitting this error, you probably want to attempt to

?
Good point.
Should dsm_impl_op sanity check the arguments after op? I didn't notice
checks in the type-specific code but I also didn't read all of it... are we
just depending on the OS to sanity-check?
Sanity-check for what?
Also, does the GUC code enforce that the GUC must always be something that's
supported? If not then the error in dsm_impl_op should be more
user-friendly. Yes.
I basically stopped reading after dsm_impl_op... the rest of the stuff was
rather over my head.
:-)

Thanks for your interest!

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Search Discussions

Discussion Posts

Previous

Follow ups

Related Discussions

Discussion Navigation
viewthread | post
posts ‹ prev | 7 of 8 | next ›
Discussion Overview
grouppgsql-hackers @
categoriespostgresql
postedAug 14, '13 at 1:09a
activeAug 31, '13 at 12:27p
posts8
users4
websitepostgresql.org...
irc#postgresql

People

Translate

site design / logo © 2017 Grokbase