Thread: dsm_registry: Add detach and destroy features
Hi,
I had use cases for manually detaching and destroying dsm segment while developing my extension (emulating Oracle's DBMS_PIPE package). In this extension user can call user defined procedures that destroy the resources in the shared memory. There were mentions about adding detach features in the original discussion (introduce dynamic shared memory registry), but it seems like it didn't expand from there.
In the attached patch, dsm_registry not provides two more interfaces: DetachNamedDSMSegment, and DestroyNamedDSMSegment. Detach function simply detaches the corresponding dsm_segment. Destroy function calls dsm_unpin_segment and deletes the dshash entry from the dsm_registry_table. Destroy function may not "destroy" the dsm segment immediately if there are other processes attached to the dsm segment, but it will eventually cause the dsm segment to be destroyed when there are no more processes attached to it. Because Destroy function deletes the entry from the dsm_registry_table, it will block new attachment. It will create a new dsm segment with the same name.
Detach and Destroy functions allows on_detach_callback, which will be passed on to the dsm segment by calling on_dsm_detach. Because on_dsm_detach requires the callback function to have dsm_segment as input, we wrap the library user callback with a function matching to the required signature.
I also made some fix in GetNamedDSMSegment function where it throws an exception if the found entry's size field does not match the user input. It looks like dshash_release_lock needs to be called and MemoryContext needs to be switched back to the old one.
It's my first time submitting a patch so please let me know if I missed on something.
Best regards,
Sungwoo Chang
Attachment
On Fri, Mar 14, 2025 at 10:46:58AM +0900, Sungwoo Chang wrote: > I also made some fix in GetNamedDSMSegment function where it throws an > exception if the found entry's size field does not match the user input. It > looks like dshash_release_lock needs to be called and MemoryContext needs > to be switched back to the old one. My expectation is that the error handling takes care of these things. Are there cases where it does not? In any case, I would expect these errors to be extremely rare to the point of virtually never happening in practice. > It's my first time submitting a patch so please let me know if I missed on > something. I'm delighted that folks want to expand on this feature, and I'm very interested in helping. Other things folks have asked for are a view into the DSM registry [0] and a simple way to allocate a DSA or dshash table with the DSM registry, all of which seem like good improvements to me. That being said, we are at the very end of the v18 development cycle, and most folks are probably busy trying to finish their ongoing projects. So it may be a while before anyone can give this a meaningful review. Please be sure to add a commitfest entry [1] so it doesn't get lost. [0] https://postgr.es/m/4D445D3E-81C5-4135-95BB-D414204A0AB4%40gmail.com [1] https://commitfest.postgresql.org/ -- nathan
2025년 3월 14일 (금) 오후 11:36, Nathan Bossart <nathandbossart@gmail.com>님이 작성: > My expectation is that the error handling takes care of these things. Are > there cases where it does not? In any case, I would expect these errors to > be extremely rare to the point of virtually never happening in practice. I don't see any logic in ereport where it switches back the original memory context. I guess not switching back to the original memory context is not so problematic because we set it to TopMemoryContext and throw will cause the user to restart a query. Not releasing the dshash lock could be problematic, so it's better to keep that. You are right about these errors to be rare, especially the one where we can't find a dsm_segment given the dsm handle. However, user faults can cause the other error, where the input name and size for dshash lookup don't match. > I'm delighted that folks want to expand on this feature, and I'm very > interested in helping. Other things folks have asked for are a view into > the DSM registry [0] and a simple way to allocate a DSA or dshash table > with the DSM registry, all of which seem like good improvements to me. I didn't know about the discussions on the view for DSM registry. I'm happy to work on that. I think what I can also add along with the view is the user command for detaching/destroying the dsm segment (like pg_backend_terminate function). > That being said, we are at the very end of the v18 development cycle, and > most folks are probably busy trying to finish their ongoing projects. So > it may be a while before anyone can give this a meaningful review. Please > be sure to add a commitfest entry [1] so it doesn't get lost. > > [0] https://postgr.es/m/4D445D3E-81C5-4135-95BB-D414204A0AB4%40gmail.com > [1] https://commitfest.postgresql.org/ Thanks for the heads up. I will add an entry to the commitfest as you advised. Best regards, Sungwoo Chang
There was a minor typo in the test module. I built and ran the test_dsm_registry extension before submitting the patch but perhaps it got mixed up with an older version.
Attachment
Regression test failed because I didn't add an extra new line in the expected file. 2025년 3월 17일 (월) 오전 9:55, Sungwoo Chang <swchangdev@gmail.com>님이 작성: > > There was a minor typo in the test module. I built and ran the > test_dsm_registry extension before submitting the patch but perhaps it > got mixed up with an older version.
Attachment
On Tue, Mar 18, 2025 at 10:05:07AM +0900, Sungwoo Chang wrote: > +/* > + * Attempt to destroy a named DSM segment > + * > + * This routine attempts to destroy the DSM segment. We unpin the dsm_segment > + * and delete the entry from dsm_registry_table. This may not destroy the > + * dsm_segment instantly, but it would die out once all the other processes > + * attached to this dsm_segment either exit or manually detach from the > + * dsm_segment. > + * > + * Because we deleted the key from dsm_registry_table, calling > + * GetNamedDSMSegment with the same key would result into creating a new > + * dsm_segment instead of retrieving the old unpinned dsm_segment. > + */ One of the reasons I avoided adding detach/destroy functionality originally is because this seems difficult to do correctly. How would an extension ensure that it doesn't end up with one set of backends attached to a new segment and another attached to an old one that is pending deletion? -- nathan
---------- Forwarded message --------- 보낸사람: Sungwoo Chang <swchangdev@gmail.com> Date: 2025년 6월 13일 (금) 오전 8:03 Subject: Re: dsm_registry: Add detach and destroy features To: Nathan Bossart <nathandbossart@gmail.com> > One of the reasons I avoided adding detach/destroy functionality originally > is because this seems difficult to do correctly. How would an extension > ensure that it doesn't end up with one set of backends attached to a new > segment and another attached to an old one that is pending deletion? Sorry for the late response. I used this patch for my extension in a way that you should always detach after you are done using the shmem segment. So the situation you described would happen in a brief moment, but once the extension finishes its task, the shmem segment will be destroyed naturally as all processes detach from it. Would this not be applicable in other extensions?
On Fri, Jun 13, 2025 at 08:03:00AM +0900, Sungwoo Chang wrote: >> One of the reasons I avoided adding detach/destroy functionality originally >> is because this seems difficult to do correctly. How would an extension >> ensure that it doesn't end up with one set of backends attached to a new >> segment and another attached to an old one that is pending deletion? > > I used this patch for my extension in a way that you should always detach > after you are done using the shmem segment. So the situation you described > would happen in a brief moment, but once the extension finishes its task, > the shmem segment will be destroyed naturally as all processes detach from > it. > > Would this not be applicable in other extensions? I suspect detaching/destroying segments would be applicable elsewhere, I'm just not sure about providing something with the aforementioned behavior. Could your use-case be handled with a DSA? On the other thread [0], we're talking about adding a GetNamedDSA() function, which returns a DSA that you can use to allocate and free shared memory as needed. In theory you could even detach the DSA if you no longer needed it in a backend, although that's probably unnecessary. [0] https://postgr.es/m/aEyX-9k5vlK2lxjz%40nathan -- nathan