Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Show HN: I built this Postgres logger

rocketgraph.io

Post reply on HN
Print view View original

11–20 of 49 posts

Re: Show HN: I built this Postgres logger

#11

Post by danpalmer »

vermon wrote: post #2

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.

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?

Re: Show HN: I built this Postgres logger

#12

Post by kvaranasi_ »

palijer wrote: post #10
rsyring wrote: post #4

> In the future we can connect these logs to slack so you can get slack alerts when a developer accidentally DROPs a table. What can you do about getting me a slack notice BEFORE a developer accidentally drops a table? ;)

Haha, exactly, but if you have developers who can drop production tables without the org knowing in advance and reviewing the change, you already have a big problem. All our db operations like this go through our regular code review process for the db maintainers team, and instead of dropping tables, we just rename them with a prefix "to_be_dropped" as a scream test, then actually drop them a month later or so. Recov…

This is a nice idea to check if any dependencies are there on the table.

Re: Show HN: I built this Postgres logger

#13

Post by kvaranasi_ »

vermon wrote: post #8
kvaranasi_ wrote: post #3

Earlier quoted context omitted.

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.

Re: Show HN: I built this Postgres logger

#14

Post by vermon »

danpalmer wrote: post #11
vermon wrote: post #2

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.

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

Re: Show HN: I built this Postgres logger

#15

Post by danpalmer »

vermon wrote: post #14
danpalmer wrote: post #11

Earlier quoted context omitted.

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!

Re: Show HN: I built this Postgres logger

#16

Post by danpalmer »

kvaranasi_ wrote: post #3
vermon wrote: post #2

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.

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?

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.

Re: Show HN: I built this Postgres logger

#17

Post by vermon »

danpalmer wrote: post #15
vermon wrote: post #14

Earlier quoted context omitted.

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,…

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.

Re: Show HN: I built this Postgres logger

#18

Post by danpalmer »

vermon wrote: post #17
danpalmer wrote: post #15

Earlier quoted context omitted.

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,…

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.

Re: Show HN: I built this Postgres logger

#19

Post by kvaranasi_ »

danpalmer wrote: post #16
kvaranasi_ wrote: post #3

Earlier quoted context omitted.

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?

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.

Re: Show HN: I built this Postgres logger

#20

Post by dewey »

kvaranasi_ wrote: post #7
rsyring wrote: post #4

> In the future we can connect these logs to slack so you can get slack alerts when a developer accidentally DROPs a table. What can you do about getting me a slack notice BEFORE a developer accidentally drops a table? ;)

Hey wait, you have a point though. We can put our custom locks onto tables that doesn't let anyone delete a particular table. Let me think over it. Saving this comment for later. Thanks for the idea man.

Uhm, why would you not use PG permissions for that? That's what they are for.
Post reply on HN