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

SQS cleanup (tables and apps) #13689

Merged
merged 29 commits into from
May 17, 2024
Merged

SQS cleanup (tables and apps) #13689

merged 29 commits into from
May 17, 2024

Conversation

mike12345567
Copy link
Collaborator

Description

SQS automatically syncs data from CouchDB that comes via the changes stream, however when tables are deleted or when an app database is deleted it is not fully aware of this.

Keeping this information will not cause any search problems, but would use extra disk space that could be freed up. This solves the problem by making sure when a DB is removed that the on disk status is cleaned up, as well as making sure when a table is deleted it is removed from the SQLite design schema.

Sadly this is quite hard to test as SQS was already aware that rows/DB doesn't exist anymore, so won't respond to the search with them, the only way to confirm that this has worked as expected is by confirming the SQLite databases have been removed from the disk. I've checked/confirmed this and it does appear to be working as expected.

Addresses

…e sure the DB is cleared of any unused tables or rows.
@mike12345567 mike12345567 added the firestorm Data/Infra/Revenue Team label May 14, 2024
@mike12345567 mike12345567 self-assigned this May 14, 2024
@mike12345567 mike12345567 requested a review from a team as a code owner May 14, 2024 16:54
@mike12345567 mike12345567 requested review from adrinr and removed request for a team May 14, 2024 16:54
packages/backend-core/src/db/Replication.ts Outdated Show resolved Hide resolved
const db = context.getAppDB()
const tableId = table._id!
// remove table search index
if (!env.isTest() || env.COUCH_DB_URL) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be checking if there sqs is enabled, instead of env.COUCH_DB_URL?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the reason why this is excluded from tests?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

honestly, I'm not sure - I didn't add this/change it, the indexes stuff is very old (and unrelated to SQS) that probably needs removed, I just moved it all to a central location for internal table cleanup, rather than it being splattered throughout the table controller.

packages/backend-core/src/db/instrumentation.ts Outdated Show resolved Hide resolved
packages/backend-core/src/db/couch/DatabaseImpl.ts Outdated Show resolved Hide resolved
Copy link
Collaborator

@adrinr adrinr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀 SQS!

@mike12345567 mike12345567 merged commit 8547b9b into master May 17, 2024
10 checks passed
@mike12345567 mike12345567 deleted the feature/sqs-table-cleanup branch May 17, 2024 10:27
@github-actions github-actions bot locked and limited conversation to collaborators May 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
firestorm Data/Infra/Revenue Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants