Skip to content

[server/db] resume periodic deleter on quorum re-election #17092

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

Merged
merged 1 commit into from
Mar 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions components/gitpod-db/src/periodic-deleter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,18 @@ export class PeriodicDbDeleter {
@inject(GitpodTableDescriptionProvider) protected readonly tableProvider: GitpodTableDescriptionProvider;
@inject(TypeORM) protected readonly typeORM: TypeORM;

start() {
start(shouldRunFn: () => Promise<boolean>) {
log.info("[PeriodicDbDeleter] Start ...");
this.sync().catch((err) => log.error("[PeriodicDbDeleter] sync failed", err));
this.sync(shouldRunFn).catch((err) => log.error("[PeriodicDbDeleter] sync failed", err));
}

protected async sync() {
protected async sync(shouldRunFn: () => Promise<boolean>) {
const doSync = async () => {
const shouldRun = await shouldRunFn();
if (!shouldRun) {
return;
}

const tickID = new Date().toISOString();
log.info("[PeriodicDbDeleter] Starting to collect deleted rows.", {
periodicDeleterTickId: tickID,
Expand Down
14 changes: 7 additions & 7 deletions components/server/src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -334,13 +334,13 @@ export class Server<C extends GitpodClient, S extends GitpodServer> {
if (!this.config.runDbDeleter) {
return;
}
const areWeLeader = await this.qorum.areWeLeader();
if (areWeLeader) {
log.info("[PeriodicDbDeleter] Current instance is leader, starting periodic deleter.");
this.periodicDbDeleter.start();
} else {
log.info("[PeriodicDbDeleter] Current instance is not the leader, periodic deleter will not run.");
}
this.periodicDbDeleter.start(async () => {
Copy link
Member Author

Choose a reason for hiding this comment

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

periodic deleter lives in gitpod-db, therefore I opted for the callback as signal of enablement.

Copy link
Member

Choose a reason for hiding this comment

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

Nice, the main difference is we always start, but then do not run if we're not the leader.

const areWeLeader = await this.qorum.areWeLeader();
log.info(
"[PeriodicDbDeleter]" + areWeLeader ? "Deleter should run." : "Current instance is not the leader",
);
return areWeLeader;
});
}

protected async registerRoutes(app: express.Application) {
Expand Down