Skip to content

use homebrew postgres driver. #7243

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 25, 2022
Merged

Conversation

fakeshadow
Copy link
Contributor

No description provided.

@NateBrady23
Copy link
Member

NateBrady23 commented Mar 24, 2022

@fakeshadow Before I take a dive into what you're doing with a homebrewed postgres driver, are you aware of the rule changes we just made here?

Specifically no. 7:

Except where noted, all database queries should be delivered to the database servers as-is and not coalesced or deduplicated at the database driver. In all cases where a database query is required, it is expected that the query will reach and execute on the database server. However, it is permissible to pipeline traffic between the application and database as a network-level optimization. That is, two or more separate queries may be delivered from the application to the database in a single transmission over the network, and likewise for query results from the database back to the application, as long as the queries themselves are executed separately on the database server and not combined into a single complex query or transaction. If any pipelining or network optimization is performed, then the execution behavior and semantics must match what they would be if the multiple SQL statements were executed without the optimization, i.e. in separate roundtrips. For example, it is forbidden to perform any optimization that could change the transactionality and/or error handling of the SQL statements. To avoid any doubt, such optimizations are forbidden even if they wouldn't produce the behavioral change with the specific SQL statements of the benchmarks, but would produce it with other statements a user might supply. If using PostgreSQL's extended query protocol, each query must be separated by a Sync message.

@michaelhixson
Copy link
Contributor

It looks good to me in manual verification.

  • Pull in the changes from this PR.
  • Pull in these other changes to enable the postgres query log.
  • Run tfb --mode debug --test xitca-web
  • Run tcpdump on the database:
    docker run --rm -it \
      --net container:tfb-database \
      nicolaka/netshoot \
      tcpdump -i eth0 port 5432 and inbound -Xvv
  • Run curl -i -s localhost:8080/queries?q=10
  • Verify the postgres log prints a distinct virtual transaction id for each query. (Batch queries would reuse the same virtual transaction id across multiple queries.)
  • Verify the tcpdump output shows 10 "B", "E", "S" (bind, execute, sync) messages. (Batch queries would have fewer than 10 "S" messages.)

@fakeshadow
Copy link
Contributor Author

fakeshadow commented Mar 25, 2022

@nbrady-techempower Thanks for informing the new rule and after a read I think my driver does not violate it.

Here is some of the source code of it if you don't mind:
https://github.com/HFQR/xitca-web/blob/e1debcd4630196669923f94c7693e5519f34db52/postgres/src/prepare.rs#L230
https://github.com/HFQR/xitca-web/blob/e1debcd4630196669923f94c7693e5519f34db52/postgres/src/query.rs#L44

Every query and prepared statement is encoded with sync .

@fakeshadow
Copy link
Contributor Author

@michaelhixson Thanks for the verify. That's a nice way to reinforce the new rule.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants