Harden database commands against failures #7369
Comments
onurctirtir
added a commit
that referenced
this issue
Feb 23, 2024
In preprocess phase, we save the original database name, replace dbname field of CreatedbStmt with a temporary name (to let Postgres to create the database with the temporary name locally) and then we insert a cleanup record for the temporary database name on all nodes **(\*\*)**. And in postprocess phase, we first rename the temporary database back to its original name for local node and then return a list of distributed DDL jobs i) to create the database with the temporary name and then ii) to rename it back to its original name on other nodes. That way, if CREATE DATABASE fails on any of the nodes, the temporary database will be cleaned up by the cleanup records that we inserted in preprocess phase and in case of a failure, we won't leak any databases called as the name that user intended to use for the database. Solves the problem documented in #7369 for CREATE DATABASE commands. **(\*\*):** To ensure that we insert cleanup records on all nodes, with this PR we also start requiring having the coordinator in the metadata because otherwise we would skip inserting a cleanup record for the coordinator.
emelsimsek
pushed a commit
that referenced
this issue
Mar 11, 2024
In preprocess phase, we save the original database name, replace dbname field of CreatedbStmt with a temporary name (to let Postgres to create the database with the temporary name locally) and then we insert a cleanup record for the temporary database name on all nodes **(\*\*)**. And in postprocess phase, we first rename the temporary database back to its original name for local node and then return a list of distributed DDL jobs i) to create the database with the temporary name and then ii) to rename it back to its original name on other nodes. That way, if CREATE DATABASE fails on any of the nodes, the temporary database will be cleaned up by the cleanup records that we inserted in preprocess phase and in case of a failure, we won't leak any databases called as the name that user intended to use for the database. Solves the problem documented in #7369 for CREATE DATABASE commands. **(\*\*):** To ensure that we insert cleanup records on all nodes, with this PR we also start requiring having the coordinator in the metadata because otherwise we would skip inserting a cleanup record for the coordinator.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Today, for database commands that cannot be executed in a transaction block (**), we don't do much next time when the command is invoked, if the prior invocation failed in the midway, e.g., if CREATE DATABASE succeeded on some nodes but not on all. We should do something better than this. For example, @thanodnl shared this as an idea on how we could handle failure scenarios for CREATE DATABASE better:
The text was updated successfully, but these errors were encountered: