4 ms·

> I'm curious why a library maintainer needs to assert responsibility over session variables at all times When pooling connections, you need to reset the conne

> I'm curious why a library maintainer needs to assert responsibility over session variables at all times When pooling connections, you need to reset the connection when putting it back in the pool. For example, if you grab a connection from the pool and do: SET SESSION statement_timeout to '1s'; SET timezone TO 'Europe/London'; …and then release the connection, you don't want the next connection to inherit these settings. It has nothing to do with the ORM (although the ORM itself may want to set some baseline settings based on config), and everything to do with sharing connections between different parts of a single application.

throwanem 2y ago
I mean I get that, what I don't get is why screwing around that way is not explicitly defined as "don't do this, here's why it isn't supported and here's what you should try to do instead; if you ignore all that and still break something, you get to keep both its pieces" in the documentation, rather than trying to work around it with some kind of global reset. Why should the engine burden itself with an undo stack (even if only one frame deep) for every client, on behalf of a minority so incoherently implemented that it can't keep straight what it's doing across the span of two (application and ORM library) codebases? A metaphor already exists in the interface for getting a fresh connection: you do this by creating a fresh connection. If you want some other way to accomplish the same task, it's fair to require justification for the effort of providing that second option, and no such justification is presented in the article under discussion. I don't like ORMs, I think it's long obvious. But to be excruciatingly clear, I grant them the same courtesy as any other library in that they absolutely may declare things explicitly out of scope. Seeing that not done, in a case where the consumer is pretty explicitly screwing around with internals of a database connection belonging to the library, is what's surprising, even in the context of ORMs as a category, where maximalism seems so constantly the order of the day.
- atombender 2y ago
  It has nothing to do with ORMs. It's the same issue if you don't use an ORM. It's about connection pooling, as I explained. Postgres has no native awareness of connection pooling, and so you need to issue certain reset commands to ensure that the connection is "clean" for the next piece of code that claims it. It should be possible to do things like "SET SESSION statement_timeout" without worrying about who the next user of the connection is.
  - throwanem 2y ago
    Right, again, I get that, and the database libraries that I use tend to warn in their documentation about what you should and shouldn't do with connections belonging to a pool, cf. node-postgres docs warning about pooled connections and transactions [1]. The unit of connection pooling is the pool not the connection, etc, that's all fine. What confuses me is that the Active Record maintainer should argue for modifications to several database engines, so this poor behavior - namely, treating pooled connections as if distinct - can be safe. Or, better said, I can see several justifications both technical and social for such an argument, none of which compels me. It's a philosophical difference, I suspect. Users who remove a cover labeled "no user-serviceable parts inside" and end up sorry they did so, in my view, learn a valuable lesson about what such covers and labels are for. Others take a kinder or less grandmotherly view. Fair enough; it's a big enough world for everyone, last I checked. (That said, it is worth noting that the only way any ORM could actually protect itself from this misuse would be by reimplementing enough of each of its supported databases' command parsers to recognize and reject session state mutations. Obviously no one would be mad enough to do so, but from a perspective of what we could call software design or architecture or craftsmanship or even taste, this kind of maximalism in necessary implication could and in my view should be taken to indict the entire paradigm: 'Why worry about the lesion? It's only pre-cancerous.') [1] https://node-postgres.com/apis/pool https://node-postgres.com/apis/pool
    - atombender 2y ago
      Cleaning up the connection removes a serious foot gun that you may not consider when writing code. For example, I work on an application that sets statement_timeout defensively to avoid accidentally holding onto locks for too long. This used SET on a pooled connection, causing a bug where later connections inherited the timeout. I have no particular opinions about ActiveRecord, which I don't use, but in my opinion all poolers ought to do this. The pooler should hand out clean connections so that you can treat them as distinct. It's about separation of concerns: A pooled connection should behave like a distinct connection, otherwise every single connection has to worry about getting an "unclean" connection. Sure, Postgres offers SET LOCAL, which reverts the mutated state on commit/rollback, but that requires a transaction, which is not always desirable. For example, let's say you are doing many UPDATEs in batches. You can do this: SET SESSION ... UPDATE ...; UPDATE ...; -- etc. As opposed to: BEGIN; SET LOCAL SESSION ...; UPDATE; COMMIT; BEGIN; SET LOCAL SESSION ...; UPDATE; COMMIT; BEGIN; SET LOCAL SESSION ...; UPDATE; COMMIT; -- etc. This saves three roundtrips per batch, which can significantly increased throughput. Of course you can save roundtrips by bundling several statements in a single statements string separated by semicolons, but that's painful to work with and doesn't work with prepared statements or SELECTs. And sure, you could yourself call RESET ALL before releasing a connection, but if the pool always does this for you, you've solved the problem everywhere and don't need to think about it ever again. (In the apps I work on, we install a global "after release" hook to automatically do RESET ALL.) Parsing statements isn't needed for this. You keep mentioning ORMs, but again, it has nothing to do with them.

throwanem 2y ago

I mean I get that, what I don't get is why screwing around that way is not explicitly defined as "don't do this, here's why it isn't supported and here's what you should try to do instead; if you ignore all that and still break something, you get to keep both its pieces" in the documentation, rather than trying to work around it with some kind of global reset. Why should the engine burden itself with an undo stack (even if only one frame deep) for every client, on behalf of a minority so incoherently implemented that it can't keep straight what it's doing across the span of two (application and ORM library) codebases? A metaphor already exists in the interface for getting a fresh connection: you do this by creating a fresh connection. If you want some other way to accomplish the same task, it's fair to require justification for the effort of providing that second option, and no such justification is presented in the article under discussion. I don't like ORMs, I think it's long obvious. But to be excruciatingly clear, I grant them the same courtesy as any other library in that they absolutely may declare things explicitly out of scope. Seeing that not done, in a case where the consumer is pretty explicitly screwing around with internals of a database connection belonging to the library, is what's surprising, even in the context of ORMs as a category, where maximalism seems so constantly the order of the day.

atombender 2y ago
It has nothing to do with ORMs. It's the same issue if you don't use an ORM. It's about connection pooling, as I explained. Postgres has no native awareness of connection pooling, and so you need to issue certain reset commands to ensure that the connection is "clean" for the next piece of code that claims it. It should be possible to do things like "SET SESSION statement_timeout" without worrying about who the next user of the connection is.
- throwanem 2y ago
  Right, again, I get that, and the database libraries that I use tend to warn in their documentation about what you should and shouldn't do with connections belonging to a pool, cf. node-postgres docs warning about pooled connections and transactions [1]. The unit of connection pooling is the pool not the connection, etc, that's all fine. What confuses me is that the Active Record maintainer should argue for modifications to several database engines, so this poor behavior - namely, treating pooled connections as if distinct - can be safe. Or, better said, I can see several justifications both technical and social for such an argument, none of which compels me. It's a philosophical difference, I suspect. Users who remove a cover labeled "no user-serviceable parts inside" and end up sorry they did so, in my view, learn a valuable lesson about what such covers and labels are for. Others take a kinder or less grandmotherly view. Fair enough; it's a big enough world for everyone, last I checked. (That said, it is worth noting that the only way any ORM could actually protect itself from this misuse would be by reimplementing enough of each of its supported databases' command parsers to recognize and reject session state mutations. Obviously no one would be mad enough to do so, but from a perspective of what we could call software design or architecture or craftsmanship or even taste, this kind of maximalism in necessary implication could and in my view should be taken to indict the entire paradigm: 'Why worry about the lesion? It's only pre-cancerous.') [1] https://node-postgres.com/apis/pool https://node-postgres.com/apis/pool
  - atombender 2y ago
    Cleaning up the connection removes a serious foot gun that you may not consider when writing code. For example, I work on an application that sets statement_timeout defensively to avoid accidentally holding onto locks for too long. This used SET on a pooled connection, causing a bug where later connections inherited the timeout. I have no particular opinions about ActiveRecord, which I don't use, but in my opinion all poolers ought to do this. The pooler should hand out clean connections so that you can treat them as distinct. It's about separation of concerns: A pooled connection should behave like a distinct connection, otherwise every single connection has to worry about getting an "unclean" connection. Sure, Postgres offers SET LOCAL, which reverts the mutated state on commit/rollback, but that requires a transaction, which is not always desirable. For example, let's say you are doing many UPDATEs in batches. You can do this: SET SESSION ... UPDATE ...; UPDATE ...; -- etc. As opposed to: BEGIN; SET LOCAL SESSION ...; UPDATE; COMMIT; BEGIN; SET LOCAL SESSION ...; UPDATE; COMMIT; BEGIN; SET LOCAL SESSION ...; UPDATE; COMMIT; -- etc. This saves three roundtrips per batch, which can significantly increased throughput. Of course you can save roundtrips by bundling several statements in a single statements string separated by semicolons, but that's painful to work with and doesn't work with prepared statements or SELECTs. And sure, you could yourself call RESET ALL before releasing a connection, but if the pool always does this for you, you've solved the problem everywhere and don't need to think about it ever again. (In the apps I work on, we install a global "after release" hook to automatically do RESET ALL.) Parsing statements isn't needed for this. You keep mentioning ORMs, but again, it has nothing to do with them.