search

actix / actix-web

Actix Web is a powerful, pragmatic, and extremely fast web framework for Rust.
https://actix.rs
Apache License 2.0
21.76k stars 1.68k forks source link

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

Open jinnatar opened 1 year ago

jinnatar commented 1 year ago

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

jinnatar commented 1 year ago

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 :shrug: