Support deletion of database

[MathieuBordere]

SQLite doesn't support the "DROP database" SQL statement and says to delete the database file instead.
In dqlite's case it lives in memory. SQLite's xDelete VFS method to delete the in-memory file is implemented in dqlite, but can't be accessed through the go-dqlite client.

We should provide a way for a user to delete a database so that after reopening a deleted database, the database is pristine.

[MathieuBordere]

The problem with deleting a database is that every time dqlite restores its state, the deleted database will reappear unless the delete operation is part of the raft log.
Therefore, to support deletion of a database we would need a new dqlite command COMMAND_DELETE, similar to COMMAND_FRAMES or COMMAND_OPEN wrapped in a raft log entry. The wire protocol would then need a message to support deletion of a database.

[freeekanayaka]

The problem with deleting a database is that every time dqlite restores its state, the deleted database will reappear unless the delete operation is part of the raft log. Therefore, to support deletion of a database we would need a new dqlite command COMMAND_DELETE, similar to COMMAND_FRAMES or COMMAND_OPEN wrapped in a raft log entry. The wire protocol would then need a message to support deletion of a database.

I see the point about the new command, but do we absolutely need a change in the dqlite wire protocol too? It feels that the new command could be simply triggered when we execute the DROP database statement.

[MathieuBordere]

The problem with deleting a database is that every time dqlite restores its state, the deleted database will reappear unless the delete operation is part of the raft log. Therefore, to support deletion of a database we would need a new dqlite command COMMAND_DELETE, similar to COMMAND_FRAMES or COMMAND_OPEN wrapped in a raft log entry. The wire protocol would then need a message to support deletion of a database.

I see the point about the new command, but do we absolutely need a change in the dqlite wire protocol too? It feels that the new command could be simply triggered when we execute the DROP database statement.

Yes, that would make more sense and be simpler, thank you!

[cole-miller]

cc @manadart, who requested this.

Some questions:

• Suppose we try to drop/reset a database that's currently open on one or more connections -- what should happen? We could immediately return an error to the client, or we could set a flag on the DB object indicating that it should be dropped when the last connection closes (returning either immediately or once that drop completes).
• Should any connection be able to drop any database, or only connections on which the database is open?
• Assuming we introduce a new drop/reset command, what will happen when a node running an old version of dqlite receives an AppendEntries request containing such a log entry?

As for how this should be exposed to clients: I prefer @MathieuBordere's original proposal to add a new request type to the wire protocol, rather than parsing SQL strings to look for DROP database. I think we present a cleaner mental model and maintain a stronger separation of concerns if we're able to say that all SQL strings are passed to SQLite unmodified.

[MathieuBordere]

cc @manadart, who requested this.

Some questions:

* Suppose we try to drop/reset a database that's currently open on one or more connections -- what should happen? We could immediately return an error to the client, or we could set a flag on the DB object indicating that it should be dropped when the last connection closes (returning either immediately or once that drop completes).

* Should any connection be able to drop any database, or only connections on which the database is open?

* Assuming we introduce a new drop/reset command, what will happen when a node running an old version of dqlite receives an AppendEntries request containing such a log entry?

As for how this should be exposed to clients: I prefer @MathieuBordere's original proposal to add a new request type to the wire protocol, rather than parsing SQL strings to look for DROP database. I think we present a cleaner mental model and maintain a stronger separation of concerns if we're able to say that all SQL strings are passed to SQLite unmodified.

I think in this case I prefer @freeekanayaka's proposal as SQLite doesn't support DROP database so we don't pass it anyway to SQLite.

[freeekanayaka]

cc @manadart, who requested this.

Some questions:

• Suppose we try to drop/reset a database that's currently open on one or more connections -- what should happen? We could immediately return an error to the client, or we could set a flag on the DB object indicating that it should be dropped when the last connection closes (returning either immediately or once that drop completes).

I'd say we'd just return an error to the client. Basically I'd expect the LOOKUP_DB macro in gateway.c to return an error. It's probably not worth to add the extra complexity of delaying the deletion until the last connection closes.

• Should any connection be able to drop any database, or only connections on which the database is open?

If possible, any connection. But I'm not sure about the ramifications of that. In any case I believe at the moment we don't support multiple databases.

• Assuming we introduce a new drop/reset command, what will happen when a node running an old version of dqlite receives an AppendEntries request containing such a log entry?

I'd say the FSM should fail to apply it and the node should go down or enter some permanent failure state. I can see in fsm.c that fsm__apply does indeed return an error already, but I'm not sure how that will propagate and how the node will behave. It's worth testing.

As for how this should be exposed to clients: I prefer @MathieuBordere's original proposal to add a new request type to the wire protocol, rather than parsing SQL strings to look for DROP database. I think we present a cleaner mental model and maintain a stronger separation of concerns if we're able to say that all SQL strings are passed to SQLite unmodified.

I'm fine either way, I hadn't realize that SQLite does not support DROP database. I can see @cole-miller point.

[MathieuBordere]

I thought about it a bit too, a separate wire protocol command is probably handier. Otherwise we will have to for example support preparing a DROP database statement, while SQLite doesn't support it, leading to a different kind of prepared statement that needs to be tracked by dqlite.

[manadart]

* Suppose we try to drop/reset a database that's currently open on one or more connections -- what should happen? We could immediately return an error to the client, or we could set a flag on the DB object indicating that it should be dropped when the last connection closes (returning either immediately or once that drop completes).

Agree with @freeekanayaka. Return an error to the client.

* Should any connection be able to drop any database, or only connections on which the database is open?

Same again; any connection.

* Assuming we introduce a new drop/reset command, what will happen when a node running an old version of dqlite receives an AppendEntries request containing such a log entry?

Won't affect us in Juju; don't mind how this is handled.