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

Add drop database #25523

Open
pauldix opened this issue Nov 8, 2024 · 6 comments · May be fixed by #25549
Open

Add drop database #25523

pauldix opened this issue Nov 8, 2024 · 6 comments · May be fixed by #25549
Assignees
Labels

Comments

@pauldix
Copy link
Member

pauldix commented Nov 8, 2024

We need a way to drop a database by name. This should be an HTTP API in the same area where we set options for last value cache, etc. We'll also want to add support to issue the command via the CLI. By default, the CLI should prompt the user to confirm they want to do the operation. There should be a command line option to auto-confirm.

Under the covers, the server should look up the name and log into the WAL a catalog operation to drop the DB by its id and the time the command was issued. When this is applied to the catalog, the name of the DB should be updated to {db_name} - {time of drop} and a marker should be applied to the DB that states that it is soft deleted.

The write buffer and last value caches should be cleared of any data they have for that dbid.

Actually deleting the underlying files and removing the old db from the Catalog will be a separate operation that we can create in the future.

We'll also want to have an undelete option, but that can be handled as separate follow up work.

@praveen-influx
Copy link
Contributor

@pauldix - high level queries on this feature,

  • should SHOW DATABASES now filter out databases with soft delete marker/flag set (or) is it ok to return the deleted one in query result?

  • when we talk of cli, I assume you're referring to query variant of influxdb3 command. How do you want to enable confirmation? Maybe make the user type the database name in cli? Or is it just yes/no?

  • drop database {name}, if name does not exist do we return an error? Do we need to support if exists sort of idea (possibly in the future)?

@hiltontj
Copy link
Contributor

I think in this case, we want a separate CLI from the query command to execute the drop.

The CLI command could follow how we delete last caches, which I think is like:

influxdb3 last_cache delete <params>

i.e.,

influxdb3 database delete <params>

Perhaps talking about the feature as DROP DATABASE is confusing the matter, since that is SQL/DDL and would be used in the context of a query, but I believe we want to keep write operations out of the query path.

@praveen-influx
Copy link
Contributor

Perhaps talking about the feature as DROP DATABASE is confusing the matter, since that is SQL/DDL and would be used in the context of a query, but I believe we want to keep write operations out of the query path.

Yes, I was thinking of DROP DATABASE as DDL/SQL. Thanks for clarifying 👍

@pauldix
Copy link
Member Author

pauldix commented Nov 12, 2024

I believe we want to keep write operations out of the query path.

This is correct. For now we'd like any modification/configuration/write operations outside of the query path. We may want to change this at some point. Most other DBs, even ones with custom configuration usually expose their config in the query language.

We may want to do this? I'm not sure yet. Mostly I want to optimize for usability while also keeping people from shooting themselves in the foot.

@praveen-influx
Copy link
Contributor

@pauldix in that case can I use the following syntax for now? That will keep it completely out of the query path. Maybe we can revisit the DDLs in SQL queries as a separate feature as I guess it'll be far more intrusive?

influxdb3 database delete <params>

should SHOW DATABASES now filter out databases with soft delete marker/flag set (or) is it ok to return the deleted one in query result?

I think this is still valid, if we soft delete should they appear in SHOW DATABASES query response marked as soft deleted? Or we just filter it out?

@pauldix
Copy link
Member Author

pauldix commented Nov 13, 2024

For now show them as soft deleted

praveen-influx added a commit that referenced this issue Nov 14, 2024
This commit allows to soft delete database using `influxdb3 database
delete <db_name>`. The write buffer and last value cache are cleared as
well.

closes: #25523
@praveen-influx praveen-influx linked a pull request Nov 14, 2024 that will close this issue
praveen-influx added a commit that referenced this issue Nov 15, 2024
This commit allows soft deletion of database using `influxdb3 database
delete <db_name>` command. The write buffer and last value cache are
cleared as well.

closes: #25523
praveen-influx added a commit that referenced this issue Nov 15, 2024
This commit allows soft deletion of database using `influxdb3 database
delete <db_name>` command. The write buffer and last value cache are
cleared as well.

closes: #25523
praveen-influx added a commit that referenced this issue Nov 15, 2024
This commit allows soft deletion of database using `influxdb3 database
delete <db_name>` command. The write buffer and last value cache are
cleared as well.

closes: #25523
praveen-influx added a commit that referenced this issue Nov 18, 2024
- In previous commit, the deletion of database immediately triggered
  clearing last cache and query buffer. But on restarts same logic had
  to be repeated to allow deleting database when starting up. This
  commit removes immediate deletion by explicitly calling necessary
  methods and moves the logic to `apply_catalog_batch` which already
  applies `CatalogOp` and also clearing cache and buffer in
  `buffer_ops` method which has hooks to call other places.

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

Successfully merging a pull request may close this issue.

3 participants