Re: [HACKERS] tick buildfarm failure
Stephen Frost sfr...@snowman.net writes: To be a bit more clear- why is it safe to change the contents if the equal() function returns 'false'? I'm guessing the answer is locking- whereby such a change would imply a lock was acquired on the relation by another backend, which shouldn't be possible if we're currently working with it, but wanted to double-check that. Right. If that were to fail, it would be the fault of something else not having gotten sufficient lock for whatever it had been doing: either lack of exclusive lock for an RLS update, or lack of an access lock to examine the cached data. The reason we want to make the equal() check rather than just summarily replace the data is that code that *does* hold AccessShare might reasonably expect the data to not move while it's looking at it. I also wonder if we might be better off with a way to identify that nothing has actually been changed due to the locks we hold and avoid rebuilding the relation cache entry entirely.. I am entirely disinclined to reinvent relcache as part of the RLS patch. You've got enough problems. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] tick buildfarm failure
* Tom Lane (t...@sss.pgh.pa.us) wrote: Stephen Frost sfr...@snowman.net writes: To be a bit more clear- why is it safe to change the contents if the equal() function returns 'false'? I'm guessing the answer is locking- whereby such a change would imply a lock was acquired on the relation by another backend, which shouldn't be possible if we're currently working with it, but wanted to double-check that. Right. If that were to fail, it would be the fault of something else not having gotten sufficient lock for whatever it had been doing: either lack of exclusive lock for an RLS update, or lack of an access lock to examine the cached data. The reason we want to make the equal() check rather than just summarily replace the data is that code that *does* hold AccessShare might reasonably expect the data to not move while it's looking at it. Ok, great, thanks for the confirmation. Yes- the RLS code wasn't anticipating a change happening underneath it such as this (as other places didn't appear worried about same, I didn't expect to see it be an issue). No problem, I'll definitely address it. I also wonder if we might be better off with a way to identify that nothing has actually been changed due to the locks we hold and avoid rebuilding the relation cache entry entirely.. I am entirely disinclined to reinvent relcache as part of the RLS patch. Apologies- I hadn't intend to even suggest that but rather to speculate about this possibility for future improvements. Thanks again! Stephen signature.asc Description: Digital signature
[HACKERS] tick buildfarm failure
All, I've been keeping an eye on tick as it failed a day-or-so ago and it looks to be related to RLS. Using a local CLFAGS=-DCLOBBER_CACHE_ALWAYS -DRANDOMIZE_ALLOCATED_MEMORY build, I was able to see the regression tests failing in check_role_for_policy() due to a pretty clear reset of the memory used for the policies. Looking through relcache.c, which I have to admit to not being as familiar with as some, the issue becomes rather apparent (I believe)- RelationClearRelation() hasn't been set up to handle the RLS policies specially, as it does for rules and tupledesc. Fair enough, it's reasonably straight-forward to add an equalPolicies() and handle the policies the same way- but I'm left wondering if that's actually *safe* to do? To be a bit more clear- why is it safe to change the contents if the equal() function returns 'false'? I'm guessing the answer is locking- whereby such a change would imply a lock was acquired on the relation by another backend, which shouldn't be possible if we're currently working with it, but wanted to double-check that. I also wonder if we might be better off with a way to identify that nothing has actually been changed due to the locks we hold and avoid rebuilding the relation cache entry entirely.. Thanks! Stephen signature.asc Description: Digital signature
