Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.
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
2 changes: 1 addition & 1 deletion cmd/frontend/internal/cli/serve_cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ func InitDB() (*sql.DB, error) {
return dbconn.Global, nil
}

if err := dbconn.MigrateDB(dbconn.Global, "frontend"); err != nil {
if err := dbconn.MigrateDB(dbconn.Global, dbconn.Frontend); err != nil {
return nil, err
}

Expand Down
2 changes: 1 addition & 1 deletion enterprise/cmd/frontend/internal/codeintel/services.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ func mustInitializeCodeIntelDB() *sql.DB {
log.Fatalf("Failed to connect to codeintel database: %s", err)
}

if err := dbconn.MigrateDB(db, "codeintel"); err != nil {
if err := dbconn.MigrateDB(db, dbconn.CodeIntel); err != nil {
log.Fatalf("Failed to perform codeintel database migration: %s", err)
}

Expand Down
2 changes: 1 addition & 1 deletion enterprise/cmd/precise-code-intel-worker/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ func mustInitializeCodeIntelDB() *sql.DB {
log.Fatalf("Failed to connect to codeintel database: %s", err)
}

if err := dbconn.MigrateDB(db, "codeintel"); err != nil {
if err := dbconn.MigrateDB(db, dbconn.CodeIntel); err != nil {
log.Fatalf("Failed to perform codeintel database migration: %s", err)
}

Expand Down
79 changes: 35 additions & 44 deletions internal/database/dbconn/migration.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,49 +17,45 @@ import (
frontendMigrations "github.com/sourcegraph/sourcegraph/migrations/frontend"
)

// databases configures the migrations we want based on a database name. This
// configuration includes the name of the migration version table as well as
// the raw migration assets to run to migrate the target schema to a new version.
var databases = map[string]struct {
// Database describes one of our Postgres (or Postgres-like) databases.
type Database struct {
// Name is the name of the database.
Name string

// MigrationsTable is the migrations SQL table name.
MigrationsTable string
Resource *bindata.AssetSource
}{
"frontend": {

// Resource describes the raw migration assets to run to migrate the target schema to a new
// version.
Resource *bindata.AssetSource

// TargetsTimescaleDB indicates if the database targets TimescaleDB. Otherwise, Postgres.
TargetsTimescaleDB bool
}

var (
Frontend = &Database{
Name: "frontend",
MigrationsTable: "schema_migrations",
Resource: bindata.Resource(frontendMigrations.AssetNames(), frontendMigrations.Asset),
},
"codeintel": {
}

CodeIntel = &Database{
Name: "codeintel",
MigrationsTable: "codeintel_schema_migrations",
Resource: bindata.Resource(codeintelMigrations.AssetNames(), codeintelMigrations.Asset),
},
"codeinsights": {
MigrationsTable: "codeinsights_schema_migrations",
Resource: bindata.Resource(codeinsightsMigrations.AssetNames(), codeinsightsMigrations.Asset),
},
}

// DatabaseNames returns the list of database names (configured via `dbutil.databases`)..
var DatabaseNames = func() []string {
var names []string
for databaseName := range databases {
names = append(names, databaseName)
}

return names
}()

// MigrationTables returns the list of migration table names (configured via `dbutil.databases`).
var MigrationTables = func() []string {
var migrationTables []string
for _, db := range databases {
migrationTables = append(migrationTables, db.MigrationsTable)
CodeInsights = &Database{
Name: "codeinsights",
TargetsTimescaleDB: true,
MigrationsTable: "codeinsights_schema_migrations",
Resource: bindata.Resource(codeinsightsMigrations.AssetNames(), codeinsightsMigrations.Asset),
}
)

return migrationTables
}()

func MigrateDB(db *sql.DB, databaseName string) error {
m, err := NewMigrate(db, databaseName)
func MigrateDB(db *sql.DB, database *Database) error {
m, err := NewMigrate(db, database)
if err != nil {
return err
}
Expand All @@ -69,23 +65,18 @@ func MigrateDB(db *sql.DB, databaseName string) error {
return nil
}

// NewMigrate returns a new configured migration object for the given database name. This database
// name must be present in the `dbconn.databases` map. This migration can be subsequently run by
// invoking `dbconn.DoMigrate`.
func NewMigrate(db *sql.DB, databaseName string) (*migrate.Migrate, error) {
schemaData, ok := databases[databaseName]
if !ok {
return nil, fmt.Errorf("unknown database '%s'", databaseName)
}
// NewMigrate returns a new configured migration object for the given database. The migration can
// be subsequently run by invoking `dbconn.DoMigrate`.
func NewMigrate(db *sql.DB, database *Database) (*migrate.Migrate, error) {

driver, err := postgres.WithInstance(db, &postgres.Config{
MigrationsTable: schemaData.MigrationsTable,
MigrationsTable: database.MigrationsTable,
})
if err != nil {
return nil, err
}

d, err := bindata.WithInstance(schemaData.Resource)
d, err := bindata.WithInstance(database.Resource)
if err != nil {
return nil, err
}
Expand Down
18 changes: 11 additions & 7 deletions internal/database/dbstore_db_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,23 +17,27 @@ func TestMigrations(t *testing.T) {
db := dbtesting.GetDB(t)

migrate := func() {
for _, databaseName := range dbconn.DatabaseNames {
if err := dbconn.MigrateDB(db, databaseName); err != nil {
t.Errorf("error running initial migrations: %s", err)
}
if err := dbconn.MigrateDB(db, dbconn.Frontend); err != nil {
t.Errorf("error running initial migrations: %s", err)
}
if err := dbconn.MigrateDB(db, dbconn.CodeIntel); err != nil {
t.Errorf("error running initial migrations: %s", err)
}
}

for _, databaseName := range dbconn.DatabaseNames {
t.Run(databaseName, func(t *testing.T) {
for _, database := range []*dbconn.Database{
dbconn.Frontend,
dbconn.CodeIntel,
} {
Copy link
Member Author

Choose a reason for hiding this comment

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

Note: Still using a few lists like this in some places to use "the same DB for frontend and codeintel" but I will start fixing that in subsequent PRs where I aim to add code-insights-equivilents to these locations.

For now, it at least makes this "we use multiple DBs here which is kinda clunky" clear & explicit

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I think this is worlds better than tacking a third one on and doing retroactive edge case fixes.

t.Run(database.Name, func(t *testing.T) {
// Dropping a squash schema _all_ the way down just drops the entire public
// schema. Because we have a "combined" database that runs migrations for
// multiple disjoint schemas in development environments, migrating all the
// way down will drop all tables from all schemas. This loop runs such down
// migrations, so we prep our tests by re-migrating up on each iteration.
migrate()

m, err := dbconn.NewMigrate(db, databaseName)
m, err := dbconn.NewMigrate(db, database)
if err != nil {
t.Errorf("error constructing migrations: %s", err)
}
Expand Down
7 changes: 5 additions & 2 deletions internal/database/dbtest/dbtest.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,11 @@ func NewDB(t testing.TB, dsn string) *sql.DB {
config.Path = "/" + dbname
testDB := dbConn(t, config)

for _, databaseName := range dbconn.DatabaseNames {
m, err := dbconn.NewMigrate(testDB, databaseName)
for _, database := range []*dbconn.Database{
dbconn.Frontend,
dbconn.CodeIntel,
} {
m, err := dbconn.NewMigrate(testDB, database)
if err != nil {
t.Fatalf("failed to construct migrations: %s", err)
}
Expand Down
12 changes: 7 additions & 5 deletions internal/database/dbtesting/dbtesting.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,8 @@ func emptyDBPreserveSchema(t testing.TB, d *sql.DB) {
}

var conds []string
for _, migrationTable := range dbconn.MigrationTables {
conds = append(conds, fmt.Sprintf("table_name != '%s'", migrationTable))
}
conds = append(conds, fmt.Sprintf("table_name != '%s'", dbconn.Frontend.MigrationsTable))
Copy link
Contributor

Choose a reason for hiding this comment

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

lol but what if code intel wants a legit table named codeinsights_schema_migrations????

conds = append(conds, fmt.Sprintf("table_name != '%s'", dbconn.CodeIntel.MigrationsTable))

rows, err := d.Query("SELECT table_name FROM information_schema.tables WHERE table_schema='public' AND table_type='BASE TABLE' AND " + strings.Join(conds, " AND "))
if err != nil {
Expand Down Expand Up @@ -159,8 +158,11 @@ func initTest(nameSuffix string) error {
return err
}

for _, databaseName := range dbconn.DatabaseNames {
if err := dbconn.MigrateDB(dbconn.Global, databaseName); err != nil {
for _, database := range []*dbconn.Database{
dbconn.Frontend,
dbconn.CodeIntel,
} {
if err := dbconn.MigrateDB(dbconn.Global, database); err != nil {
return err
}
}
Expand Down
30 changes: 15 additions & 15 deletions internal/database/schemadoc/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ var logger = log.New(os.Stderr, "", log.LstdFlags)

var versionRe = lazyregexp.New(fmt.Sprintf(`\b%s\b`, regexp.QuoteMeta("9.6")))

var databases = map[string]string{
"frontend": "schema.md",
"codeintel": "schema.codeintel.md",
var databases = map[*dbconn.Database]string{
dbconn.Frontend: "schema.md",
dbconn.CodeIntel: "schema.codeintel.md",
}

// This script generates markdown formatted output containing descriptions of
Expand All @@ -61,8 +61,8 @@ func mainErr() error {
func mainLocal() error {
dataSourcePrefix := "dbname=" + databaseNamePrefix

for databaseName, destinationFile := range databases {
if err := generateAndWrite(databaseName, dataSourcePrefix+databaseName, nil, destinationFile); err != nil {
for database, destinationFile := range databases {
if err := generateAndWrite(database, dataSourcePrefix+database.Name, nil, destinationFile); err != nil {
return err
}
}
Expand All @@ -81,29 +81,29 @@ func mainContainer() error {

dataSourcePrefix := "postgres://postgres@127.0.0.1:5433/postgres?dbname=" + databaseNamePrefix

for databaseName, destinationFile := range databases {
if err := generateAndWrite(databaseName, dataSourcePrefix+databaseName, prefix, destinationFile); err != nil {
for database, destinationFile := range databases {
if err := generateAndWrite(database, dataSourcePrefix+database.Name, prefix, destinationFile); err != nil {
return err
}
}

return nil
}

func generateAndWrite(databaseName, dataSource string, commandPrefix []string, destinationFile string) error {
func generateAndWrite(database *dbconn.Database, dataSource string, commandPrefix []string, destinationFile string) error {
run := runWithPrefix(commandPrefix)

// Try to drop a database if it already exists
_, _ = run(true, "dropdb", databaseNamePrefix+databaseName)
_, _ = run(true, "dropdb", databaseNamePrefix+database.Name)

// Let's also try to clean up after ourselves
defer func() { _, _ = run(true, "dropdb", databaseNamePrefix+databaseName) }()
defer func() { _, _ = run(true, "dropdb", databaseNamePrefix+database.Name) }()

if out, err := run(false, "createdb", databaseNamePrefix+databaseName); err != nil {
if out, err := run(false, "createdb", databaseNamePrefix+database.Name); err != nil {
return errors.Wrap(err, fmt.Sprintf("run: %s", out))
}

out, err := generateInternal(databaseName, dataSource, run)
out, err := generateInternal(database, dataSource, run)
if err != nil {
return err
}
Expand Down Expand Up @@ -153,7 +153,7 @@ func startDocker() (commandPrefix []string, shutdown func(), _ error) {
return []string{"docker", "exec", "-u", "postgres", containerName}, shutdown, nil
}

func generateInternal(databaseName, dataSource string, run runFunc) (string, error) {
func generateInternal(database *dbconn.Database, dataSource string, run runFunc) (string, error) {
db, err := dbconn.NewRaw(dataSource)
if err != nil {
return "", errors.Wrap(err, "NewRaw")
Expand All @@ -164,7 +164,7 @@ func generateInternal(databaseName, dataSource string, run runFunc) (string, err
}
}()

if err := dbconn.MigrateDB(db, databaseName); err != nil {
if err := dbconn.MigrateDB(db, database); err != nil {
return "", errors.Wrap(err, "MigrateDB")
}

Expand Down Expand Up @@ -197,7 +197,7 @@ func generateInternal(databaseName, dataSource string, run runFunc) (string, err
for table := range ch {
logger.Println("describe", table)

doc, err := describeTable(db, databaseName, table, run)
doc, err := describeTable(db, database.Name, table, run)
if err != nil {
logger.Fatalf("error: %s", err)
continue
Expand Down