Skip to content

Commit

Permalink
Move towards sync deletion (away from PeriodicDeleter) - step II/II (#…
Browse files Browse the repository at this point in the history
…18866)

* [db] DBTeam: drop deleted column (unused)

* [db] DBProject: drop deleted column (unused)

* [db] Drop table d_b_user_storage_resource

* [db] DBTokenEntry: drop deleted column (unused)

* [db] DBAuthProviderEntry: drop deleted column (unused)

* [db] DBGitpodToken: drop deleted column (unused)

* [db] DBTeamMembership: drop deleted column (unused)

* [db] DBProjectUsage: drop deleted column (unused)

* [db] DBUserSshPublicKey: drop deleted column (unused)

* [server] Fix flaky test

* [db] Make backwards-compatible to mysql 5.7
  • Loading branch information
geropl authored Oct 6, 2023
1 parent 0f926eb commit 5bf82cd
Show file tree
Hide file tree
Showing 29 changed files with 262 additions and 149 deletions.
4 changes: 0 additions & 4 deletions components/gitpod-db/go/organization.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,6 @@ type Organization struct {
LastModified time.Time `gorm:"column:_lastModified;type:timestamp;default:CURRENT_TIMESTAMP(6);" json:"_lastModified"`

MarkedDeleted bool `gorm:"column:markedDeleted;type:tinyint;default:0;" json:"marked_deleted"`

// deleted is reserved for use by periodic deleter
_ bool `gorm:"column:deleted;type:tinyint;default:0;" json:"deleted"`
}

// TableName sets the insert table name for this struct type
Expand Down Expand Up @@ -85,7 +82,6 @@ func GetSingleOrganizationWithActiveSSO(ctx context.Context, conn *gorm.DB) (Org
WithContext(ctx).
Table(fmt.Sprintf("%s as team", (&Organization{}).TableName())).
Joins(fmt.Sprintf("JOIN %s AS config ON team.id = config.organizationId", (&OIDCClientConfig{}).TableName())).
Where("team.deleted = ?", 0).
Where("config.deleted = ?", 0).
Where("config.active = ?", 1).
Find(&orgs)
Expand Down
7 changes: 1 addition & 6 deletions components/gitpod-db/go/organization_membership.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,6 @@ type OrganizationMembership struct {
CreationTime VarcharTime `gorm:"column:creationTime;type:varchar;size:255;" json:"creationTime"`
// Read-only (-> property).
LastModified time.Time `gorm:"->:column:_lastModified;type:timestamp;default:CURRENT_TIMESTAMP(6);" json:"_lastModified"`

// deleted column is reserved for use by periodic deleter
_ bool `gorm:"column:deleted;type:tinyint;default:0;" json:"deleted"`
}

// TableName sets the insert table name for this struct type
Expand Down Expand Up @@ -54,7 +51,6 @@ func GetOrganizationMembership(ctx context.Context, conn *gorm.DB, userID, orgID
tx := conn.WithContext(ctx).
Where("userId = ?", userID.String()).
Where("teamId = ?", orgID.String()).
Where("deleted = ?", false).
First(&membership)
if tx.Error != nil {
if errors.Is(tx.Error, gorm.ErrRecordNotFound) {
Expand All @@ -79,8 +75,7 @@ func DeleteOrganizationMembership(ctx context.Context, conn *gorm.DB, userID uui
Model(&OrganizationMembership{}).
Where("userId = ?", userID.String()).
Where("teamId = ?", orgID.String()).
Where("deleted = ?", 0).
Update("deleted", 1)
Delete(&OrganizationMembership{})
if tx.Error != nil {
return fmt.Errorf("failed to retrieve organization membership for user %s, organization %s: %w", userID.String(), orgID.String(), tx.Error)
}
Expand Down
3 changes: 0 additions & 3 deletions components/gitpod-db/go/project.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,6 @@ type Project struct {
UserID sql.NullString `gorm:"column:userId;type:char;size:36;" json:"userId"`

MarkedDeleted bool `gorm:"column:markedDeleted;type:tinyint;default:0;" json:"markedDeleted"`

// deleted is reserved for use by periodic deleter
_ bool `gorm:"column:deleted;type:tinyint;default:0;" json:"deleted"`
}

// TableName sets the insert table name for this struct type
Expand Down
3 changes: 1 addition & 2 deletions components/gitpod-db/go/project_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ var projectJSON = map[string]interface{}{
"teamId": "0e433063-1358-4892-9ed2-68e273d17d07",
"appInstallationId": "20446411",
"creationTime": "2021-11-01T19:36:07.532Z",
"deleted": 0,
"_lastModified": "2021-11-02 10:49:12.473658",
"name": "gptest1-repo1-private",
"markedDeleted": 1,
Expand All @@ -49,7 +48,7 @@ func TestProject_ReadExistingRecords(t *testing.T) {

func insertRawProject(t *testing.T, conn *gorm.DB, obj map[string]interface{}) uuid.UUID {
columns := []string{
"id", "cloneUrl", "teamId", "appInstallationId", "creationTime", "deleted", "_lastModified", "name", "markedDeleted", "userId", "slug", "settings",
"id", "cloneUrl", "teamId", "appInstallationId", "creationTime", "_lastModified", "name", "markedDeleted", "userId", "slug", "settings",
}
statement := fmt.Sprintf(`INSERT INTO d_b_project (%s) VALUES ?;`, strings.Join(columns, ", "))
id := uuid.MustParse(obj["id"].(string))
Expand Down
3 changes: 1 addition & 2 deletions components/gitpod-db/src/auth-provider-entry.spec.db.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ describe("AuthProviderEntryDBSpec", async () => {
status: "verified",
type: "GitHub",
oauthRevision: undefined,
deleted: false,
...ap,
oauth: {
callBackUrl: "example.org/some/callback",
Expand Down Expand Up @@ -74,7 +73,7 @@ describe("AuthProviderEntryDBSpec", async () => {
await db.storeAuthProvider(ap2, false);

const all = await db.findAllHosts();
expect(all, "findAllHosts([])").to.deep.equal(["foo", "bar"]);
expect(all.sort(), "findAllHosts([])").to.deep.equal(["foo", "bar"].sort());
});

it("should oauthRevision", async () => {
Expand Down
49 changes: 0 additions & 49 deletions components/gitpod-db/src/tables.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,55 +62,18 @@ export class GitpodTableDescriptionProvider implements TableDescriptionProvider
primaryKeys: ["id"],
timeColumn: "_lastModified",
},
{
name: "d_b_token_entry",
primaryKeys: ["uid"],
deletionColumn: "deleted",
timeColumn: "_lastModified",
},
{
name: "d_b_gitpod_token",
primaryKeys: ["tokenHash"],
deletionColumn: "deleted",
timeColumn: "_lastModified",
dependencies: ["d_b_user"],
},
{
name: "d_b_one_time_secret",
primaryKeys: ["id"],
deletionColumn: "deleted",
timeColumn: "_lastModified",
},
{
name: "d_b_auth_provider_entry",
primaryKeys: ["id"],
deletionColumn: "deleted",
timeColumn: "_lastModified",
},
{
name: "d_b_team",
primaryKeys: ["id"],
deletionColumn: "deleted",
timeColumn: "_lastModified",
},
{
name: "d_b_team_membership",
primaryKeys: ["id"],
deletionColumn: "deleted",
timeColumn: "_lastModified",
},
{
name: "d_b_team_membership_invite",
primaryKeys: ["id"],
deletionColumn: "deleted",
timeColumn: "_lastModified",
},
{
name: "d_b_project",
primaryKeys: ["id"],
deletionColumn: "deleted",
timeColumn: "_lastModified",
},
{
name: "d_b_project_env_var",
primaryKeys: ["id", "projectId"],
Expand All @@ -123,18 +86,6 @@ export class GitpodTableDescriptionProvider implements TableDescriptionProvider
deletionColumn: "deleted",
timeColumn: "_lastModified",
},
{
name: "d_b_project_usage",
primaryKeys: ["projectId"],
deletionColumn: "deleted",
timeColumn: "_lastModified",
},
{
name: "d_b_user_ssh_public_key",
primaryKeys: ["id"],
deletionColumn: "deleted",
timeColumn: "_lastModified",
},
{
name: "d_b_stripe_customer",
primaryKeys: ["stripeCustomerId"],
Expand Down
17 changes: 6 additions & 11 deletions components/gitpod-db/src/typeorm/auth-provider-entry-db-impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ export class AuthProviderEntryDBImpl implements AuthProviderEntryDB {
[id],
);

// 2. then mark as deleted
// 2. then delete
const repo = await this.getAuthProviderRepo();
await repo.delete({ id });
}
Expand All @@ -55,7 +55,7 @@ export class AuthProviderEntryDBImpl implements AuthProviderEntryDB {
exceptOAuthRevisions = exceptOAuthRevisions.filter((r) => r !== ""); // never filter out '' which means "undefined" in the DB

const repo = await this.getAuthProviderRepo();
let query = repo.createQueryBuilder("auth_provider").where("auth_provider.deleted != true");
let query = repo.createQueryBuilder("auth_provider");
if (exceptOAuthRevisions.length > 0) {
query = query.andWhere("auth_provider.oauthRevision NOT IN (:...exceptOAuthRevisions)", {
exceptOAuthRevisions,
Expand All @@ -68,18 +68,15 @@ export class AuthProviderEntryDBImpl implements AuthProviderEntryDB {
const hostField: keyof DBAuthProviderEntry = "host";

const repo = await this.getAuthProviderRepo();
const query = repo.createQueryBuilder("auth_provider").select(hostField).where("auth_provider.deleted != true");
const query = repo.createQueryBuilder("auth_provider").select(hostField);
const result = (await query.execute()) as Pick<DBAuthProviderEntry, "host">[];
// HINT: host is expected to be lower case
return result.map((r) => r.host?.toLowerCase()).filter((h) => !!h);
}

async findByHost(host: string): Promise<AuthProviderEntry | undefined> {
const repo = await this.getAuthProviderRepo();
const query = repo
.createQueryBuilder("auth_provider")
.where(`auth_provider.host = :host`, { host })
.andWhere("auth_provider.deleted != true");
const query = repo.createQueryBuilder("auth_provider").where(`auth_provider.host = :host`, { host });
return query.getOne();
}

Expand All @@ -93,17 +90,15 @@ export class AuthProviderEntryDBImpl implements AuthProviderEntryDB {
const query = repo
.createQueryBuilder("auth_provider")
.where(`auth_provider.ownerId = :ownerId`, { ownerId })
.andWhere("(auth_provider.organizationId IS NULL OR auth_provider.organizationId = '')")
.andWhere("auth_provider.deleted != true");
.andWhere("(auth_provider.organizationId IS NULL OR auth_provider.organizationId = '')");
return query.getMany();
}

async findByOrgId(organizationId: string): Promise<AuthProviderEntry[]> {
const repo = await this.getAuthProviderRepo();
const query = repo
.createQueryBuilder("auth_provider")
.where(`auth_provider.organizationId = :organizationId`, { organizationId })
.andWhere("auth_provider.deleted != true");
.where(`auth_provider.organizationId = :organizationId`, { organizationId });
return query.getMany();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,4 @@ export class DBAuthProviderEntry implements AuthProviderEntry {
transformer: Transformer.MAP_EMPTY_STR_TO_UNDEFINED,
})
oauthRevision?: string;

@Column()
deleted?: boolean;
}
3 changes: 0 additions & 3 deletions components/gitpod-db/src/typeorm/entity/db-gitpod-token.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,4 @@ export class DBGitpodToken implements GitpodToken {

@Column()
created: string;

@Column()
deleted?: boolean;
}
4 changes: 0 additions & 4 deletions components/gitpod-db/src/typeorm/entity/db-project-usage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,4 @@ export class DBProjectUsage {

@Column("varchar")
lastWorkspaceStart: string;

// This column triggers the periodic deleter deletion mechanism. It's not intended for public consumption.
@Column()
deleted: boolean;
}
8 changes: 2 additions & 6 deletions components/gitpod-db/src/typeorm/entity/db-project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@

import { Entity, Column, PrimaryColumn, Index } from "typeorm";
import { TypeORM } from "../typeorm";
import { ProjectSettings } from "@gitpod/gitpod-protocol";
import { Project, ProjectSettings } from "@gitpod/gitpod-protocol";
import { Transformer } from "../transformer";

@Entity()
// on DB but not Typeorm: @Index("ind_lastModified", ["_lastModified"]) // DBSync
export class DBProject {
export class DBProject implements Project {
@PrimaryColumn(TypeORM.UUID_COLUMN_TYPE)
id: string;

Expand All @@ -38,10 +38,6 @@ export class DBProject {
@Column("varchar")
creationTime: string;

// This column triggers the periodic deleter deletion mechanism. It's not intended for public consumption.
@Column()
deleted: boolean;

@Column()
markedDeleted: boolean;
}
4 changes: 0 additions & 4 deletions components/gitpod-db/src/typeorm/entity/db-team-membership.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,4 @@ export class DBTeamMembership {

@Column("varchar")
creationTime: string;

// This column triggers the periodic deleter deletion mechanism. It's not intended for public consumption.
@Column()
deleted: boolean;
}
4 changes: 0 additions & 4 deletions components/gitpod-db/src/typeorm/entity/db-team.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,4 @@ export class DBTeam implements Team {

@Column()
markedDeleted?: boolean;

// This column triggers the periodic deleter deletion mechanism. It's not intended for public consumption.
@Column()
deleted: boolean;
}
3 changes: 0 additions & 3 deletions components/gitpod-db/src/typeorm/entity/db-token-entry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,4 @@ export class DBTokenEntry implements TokenEntry {
),
})
token: Token;

@Column()
deleted?: boolean;
}
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,4 @@ export class DBUserSshPublicKey implements UserSSHPublicKey {
transformer: Transformer.MAP_EMPTY_STR_TO_UNDEFINED,
})
lastUsedTime?: string;

// This column triggers the periodic deleter deletion mechanism. It's not intended for public consumption.
@Column()
deleted: boolean;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/**
* Copyright (c) 2023 Gitpod GmbH. All rights reserved.
* Licensed under the GNU Affero General Public License (AGPL).
* See License.AGPL.txt in the project root for license information.
*/

import { MigrationInterface, QueryRunner } from "typeorm";
import { columnExists } from "./helper/helper";

const TABLE_NAME = "d_b_team";
const COLUMN_NAME = "deleted";

export class TeamDropDeleted1695810605786 implements MigrationInterface {
public async up(queryRunner: QueryRunner): Promise<void> {
if (await columnExists(queryRunner, TABLE_NAME, COLUMN_NAME)) {
await queryRunner.query(`ALTER TABLE \`${TABLE_NAME}\` DROP COLUMN \`${COLUMN_NAME}\``);
}
}

public async down(queryRunner: QueryRunner): Promise<void> {
if (!(await columnExists(queryRunner, TABLE_NAME, COLUMN_NAME))) {
await queryRunner.query(
`ALTER TABLE \`${TABLE_NAME}\` ADD COLUMN \`${COLUMN_NAME}\` tinyint(4) NOT NULL DEFAULT '0'`,
);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/**
* Copyright (c) 2023 Gitpod GmbH. All rights reserved.
* Licensed under the GNU Affero General Public License (AGPL).
* See License.AGPL.txt in the project root for license information.
*/

import { MigrationInterface, QueryRunner } from "typeorm";
import { columnExists } from "./helper/helper";

const TABLE_NAME = "d_b_project";
const COLUMN_NAME = "deleted";

export class ProjectDropDeleted1695817445588 implements MigrationInterface {
public async up(queryRunner: QueryRunner): Promise<void> {
if (await columnExists(queryRunner, TABLE_NAME, COLUMN_NAME)) {
await queryRunner.query(`ALTER TABLE \`${TABLE_NAME}\` DROP COLUMN \`${COLUMN_NAME}\``);
}
}

public async down(queryRunner: QueryRunner): Promise<void> {
if (!(await columnExists(queryRunner, TABLE_NAME, COLUMN_NAME))) {
await queryRunner.query(
`ALTER TABLE \`${TABLE_NAME}\` ADD COLUMN \`${COLUMN_NAME}\` tinyint(4) NOT NULL DEFAULT '0'`,
);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/**
* Copyright (c) 2023 Gitpod GmbH. All rights reserved.
* Licensed under the GNU Affero General Public License (AGPL).
* See License.AGPL.txt in the project root for license information.
*/

import { MigrationInterface, QueryRunner } from "typeorm";

export class DropTableUserStorageResource1695819413747 implements MigrationInterface {
public async up(queryRunner: QueryRunner): Promise<void> {
await queryRunner.query("DROP TABLE IF EXISTS `d_b_user_storage_resource`");
}

public async down(queryRunner: QueryRunner): Promise<void> {}
}
Loading

0 comments on commit 5bf82cd

Please sign in to comment.