Skip to content
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

Inserted data is assumed to be immediately selectable without a transaction, which is not a safe assumption #3188

Open
jinnatar opened this issue Nov 14, 2023 · 1 comment

Comments

@jinnatar
Copy link

Expected Behavior

INSERT statements succeed insert confirmation even if the RDBMS uses replication and serves standalone SELECT queries from a secondary replica.

Current Behavior

Presumably a separate SELECT query is made after an insert which results in:

[2023-11-14T11:06:23Z DEBUG actix_web::middleware::logger] Error in response: Database(RecordNotFound("Failed to find inserted item"))

The string Failed to find inserted item is not present in the downstream repo, which lead me to suspect this is a feature of your project, not something they're doing wrong. If I'm mistaken and this is not a thing, please close as WAI and I'll bug the downstream project (TurtIeSocks/Koji) to not do insert validation wrong themselves. :-)

Debug log:

[2023-11-14T11:06:23Z DEBUG actix_web::extract] Error for Option<T> extractor: 401 Unauthorized
[2023-11-14T11:06:23Z DEBUG sqlx_mysql::connection::tls] not performing TLS upgrade: unsupported by server
[2023-11-14T11:06:23Z DEBUG sqlx::query] summary="SET sql_mode=(SELECT CONCAT(@@sql_mode, ',PIPES_AS_CONCAT,NO_ENGINE_SUBSTITUTION')),time_zone='+00:00',NAMES …" db.statement="\n\nSET\n  sql_mode =(\n    SELECT\n      CONCAT(\n        @ @sql_mode,\n        ',PIPES_AS_CONCAT,NO_ENGINE_SUBSTITUTION'\n      )\n  ),\n  time_zone = '+00:00',\n  NAMES utf8mb4 COLLATE utf8mb4_unicode_ci;\n" rows_affected=0 rows_returned=0 elapsed=1.938596ms
[2023-11-14T11:06:23Z DEBUG sqlx::query] summary="SELECT `project`.`id`, `project`.`name`, `project`.`api_endpoint`, …" db.statement="\n\nSELECT\n  `project`.`id`,\n  `project`.`name`,\n  `project`.`api_endpoint`,\n  `project`.`api_key`,\n  `project`.`scanner`,\n  `project`.`created_at`,\n  `project`.`updated_at`,\n  `project`.`description`\nFROM\n  `project`\nWHERE\n  `project`.`id` = ?\nLIMIT\n  ?\n" rows_affected=0 rows_returned=0 elapsed=2.141039ms
[2023-11-14T11:06:23Z DEBUG sqlx_mysql::connection::tls] not performing TLS upgrade: unsupported by server
[2023-11-14T11:06:23Z DEBUG sqlx::query] summary="SET sql_mode=(SELECT CONCAT(@@sql_mode, ',PIPES_AS_CONCAT,NO_ENGINE_SUBSTITUTION')),time_zone='+00:00',NAMES …" db.statement="\n\nSET\n  sql_mode =(\n    SELECT\n      CONCAT(\n        @ @sql_mode,\n        ',PIPES_AS_CONCAT,NO_ENGINE_SUBSTITUTION'\n      )\n  ),\n  time_zone = '+00:00',\n  NAMES utf8mb4 COLLATE utf8mb4_unicode_ci;\n" rows_affected=0 rows_returned=0 elapsed=1.344763ms
[2023-11-14T11:06:23Z DEBUG sqlx::query] summary="INSERT INTO `project` (`name`, …" db.statement="\n\nINSERT INTO\n  `project` (\n    `name`,\n    `api_endpoint`,\n    `api_key`,\n    `scanner`,\n    `description`\n  )\nVALUES\n  (?, ?, ?, ?, ?)\n" rows_affected=1 rows_returned=0 elapsed=2.960539ms
[2023-11-14T11:06:23Z DEBUG sqlx::query] summary="SELECT `project`.`id`, `project`.`name`, `project`.`api_endpoint`, …" db.statement="\n\nSELECT\n  `project`.`id`,\n  `project`.`name`,\n  `project`.`api_endpoint`,\n  `project`.`api_key`,\n  `project`.`scanner`,\n  `project`.`created_at`,\n  `project`.`updated_at`,\n  `project`.`description`\nFROM\n  `project`\nWHERE\n  `project`.`id` = ?\nLIMIT\n  ?\n" rows_affected=0 rows_returned=0 elapsed=901.866µs
[2023-11-14T11:06:23Z DEBUG actix_web::middleware::logger] Error in response: Database(RecordNotFound("Failed to find inserted item"))
[2023-11-14T11:06:23Z INFO  actix_web::middleware::logger] 500 | POST /internal/admin/project/ HTTP/1.1 - 66 bytes in 19.910021 ms (10.0.10.5)

In reality the insert succeeded, but has not yet reached the read-only replica to which the read-only separate SELECT query is distributed to, hence why the SELECT does not find the record. The insert is not rolled back, so even if I receive a 500 internal error, the data was left inserted.

Possible Solution

Perform insert validation within a transaction to keep the query on the same instance where the write was just executed. It's not a safe assumption that replication is instant to read-only replicas that handle standalone SELECT queries.

Steps to Reproduce (for bugs)

  1. Have a primary-secondary replicated DB and proxy that separates read & write queries, with read being sent to the readonly replica(s). My specific case is MariaDB + MaxScale, but setting up one of those or anything similar is far more steps than I can write here. :-)
  2. INSERT data with validation, no idea how that's done with your library, I'm sadly two steps removed from it.

Context

I am fully aware running MaxScale and replicas is not "common" and people without replication will never see the issue. As far as I understand it though, a separate SELECT after an INSERT is a logical fallacy outside of a transaction, and becomes a showstopper for anything even slightly enterprisey where replication, HA and loadbalancing is concerned. This may or may not be a concern for you. :-)

Your Environment

  • Rust Version (I.e, output of rustc -V): rust:1.71 Docker image.
  • Actix Web Version: 4.4.0
@jinnatar
Copy link
Author

Derp, I just realized this could be instead a feature of sqlx or even deeper in the stack, but not finding the error message from either project so 🤷

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

No branches or pull requests

1 participant