Hacker News new | ask | show | jobs
by vermon 1030 days ago
pgAudit is pretty cool, but it's a bit of a hassle for us currently because it keeps logging SELECT FOR UPDATE's even though SELECT is not allowed for the auditor, so if you implement stuff like simple queues with continuous polling it fills up the audit log with junk pretty fast. I opened a PR though so hopefully it gets fixed at some point.
2 comments

Interesting. I did not know that. I have to dig into the source code then. Do you know any alternate logging tools that don't have this issue?
I think pgAudit it still the best and it's not a major issue. You can try my PR that fixes this issue https://github.com/pgaudit/pgaudit/pull/219 it should work and it should handle the other types of SELECT's that need update permissions but are not actually updating anything https://pglocks.org/?pglock=RowShareLock
Ok, looking into it.
FYI, be careful, see my other comments in reply but I think this is working correctly, and due to the fact that a `SELECT ... FOR UPDATE` is a write.
Yes, I have been reading the conversation. That was so much to learn. I did not know that `SELECT ... FOR UPDATE` is "sort of" a write. I googled and found out this SO link: https://stackoverflow.com/questions/18879584/postgres-select...

that corroborates what you have said.

This sounds like a desirable feature. SELECT FOR UPDATE results in a write where SELECT does not, and distinguishing between reads and writes is definitely something I'd want to be able to do here. Unless I've misunderstood the problem – I'm not sure I fully understand your PR.

Perhaps being able to exclude those as a separate category would be good?

What does a write in this context mean? I still need to execute the actual update clause to change the relevant data.

EDIT: Since you control object auditing by granting/revoking permissions to the relevant relations I don't think it's possible to have another category there since Postgres itself doesn't differentiate between UPDATE and SELECT FOR UPDATE on permission level

It means that the `FOR UPDATE` part in `SELECT ... FOR UPDATE` causes a disk write, because it locks that row. It bumps the MVCC metadata, the transaction ID on the tuple I think(?) in order to prevent other transactions from modifying it. While the data hasn't actually changed, the availability of that data has changed, so I think it's reasonable to consider that a write from the database perspective.

Although FWIW, I do completely understand your use-case of trying to quieten down logs for a queue!

Make sense. I guess it could also be a flag for the audit module where you can toggle if SELECT permission for auditing includes the SELECT clauses that need RowShareLock as well or not.
Yes! I don't think that should be a default because I think it's "safer" and more correct to consider locks writes, but having it available for pragmatic use-cases like this would be nice.