From f16dfaa9f0d94b496b7cfc7067af557c37e20497 Mon Sep 17 00:00:00 2001 From: Brad Rydzewski Date: Tue, 23 May 2017 12:43:58 +0200 Subject: [PATCH] increment build from counter --- model/repo.go | 1 + store/datastore/builds.go | 45 +++++++- store/datastore/builds_test.go | 106 ++++++++++++++---- store/datastore/config_test.go | 7 +- store/datastore/ddl/mysql/ddl_gen.go | 26 ++++- ...po_seq.sql => 014_add_column_repo_seq.sql} | 0 store/datastore/ddl/postgres/ddl_gen.go | 26 ++++- ...po_seq.sql => 014_add_column_repo_seq.sql} | 0 store/datastore/ddl/sqlite/ddl_gen.go | 26 ++++- ...po_seq.sql => 014_add_column_repo_seq.sql} | 0 store/datastore/sql/postgres/files/repos.sql | 5 + store/datastore/sql/postgres/sql_gen.go | 7 ++ store/datastore/sql/sqlite/files/repos.sql | 5 + store/datastore/sql/sqlite/sql_gen.go | 7 ++ 14 files changed, 233 insertions(+), 28 deletions(-) rename store/datastore/ddl/mysql/files/{013_add_column_repo_seq.sql => 014_add_column_repo_seq.sql} (100%) rename store/datastore/ddl/postgres/files/{013_add_column_repo_seq.sql => 014_add_column_repo_seq.sql} (100%) rename store/datastore/ddl/sqlite/files/{013_add_column_repo_seq.sql => 014_add_column_repo_seq.sql} (100%) create mode 100644 store/datastore/sql/postgres/files/repos.sql create mode 100644 store/datastore/sql/sqlite/files/repos.sql diff --git a/model/repo.go b/model/repo.go index c650e5dd9..7b1a04914 100644 --- a/model/repo.go +++ b/model/repo.go @@ -30,6 +30,7 @@ type Repo struct { AllowPush bool `json:"allow_push" meddler:"repo_allow_push"` AllowDeploy bool `json:"allow_deploys" meddler:"repo_allow_deploys"` AllowTag bool `json:"allow_tags" meddler:"repo_allow_tags"` + Counter int `json:"last_build" meddler:"repo_counter"` Config string `json:"config_file" meddler:"repo_config_path"` Hash string `json:"-" meddler:"repo_hash"` } diff --git a/store/datastore/builds.go b/store/datastore/builds.go index e43fb639e..2e97d34b4 100644 --- a/store/datastore/builds.go +++ b/store/datastore/builds.go @@ -1,9 +1,11 @@ package datastore import ( + "fmt" "time" "github.com/drone/drone/model" + "github.com/drone/drone/store/datastore/sql" "github.com/russross/meddler" ) @@ -56,12 +58,14 @@ func (db *datastore) GetBuildQueue() ([]*model.Feed, error) { } func (db *datastore) CreateBuild(build *model.Build, procs ...*model.Proc) error { - var number int - db.QueryRow(rebind(buildNumberLast), build.RepoID).Scan(&number) - build.Number = number + 1 + id, err := db.incrementRepoRetry(build.RepoID) + if err != nil { + return err + } + build.Number = id build.Created = time.Now().UTC().Unix() build.Enqueued = build.Created - err := meddler.Insert(db, buildTable, build) + err = meddler.Insert(db, buildTable, build) if err != nil { return err } @@ -75,6 +79,39 @@ func (db *datastore) CreateBuild(build *model.Build, procs ...*model.Proc) error return nil } +func (db *datastore) incrementRepoRetry(id int64) (int, error) { + repo, err := db.GetRepo(id) + if err != nil { + return 0, fmt.Errorf("database: cannot fetch repository: %s", err) + } + for i := 0; i < 10; i++ { + seq, err := db.incrementRepo(repo.ID, repo.Counter+i, repo.Counter+i+1) + if err != nil { + return 0, err + } + if seq == 0 { + continue + } + return seq, nil + } + return 0, fmt.Errorf("cannot increment next build number") +} + +func (db *datastore) incrementRepo(id int64, old, new int) (int, error) { + results, err := db.Exec(sql.Lookup(db.driver, "repo-update-counter"), new, old, id) + if err != nil { + return 0, fmt.Errorf("database: update repository counter: %s", err) + } + updated, err := results.RowsAffected() + if err != nil { + return 0, fmt.Errorf("database: update repository counter: %s", err) + } + if updated == 0 { + return 0, nil + } + return new, nil +} + func (db *datastore) UpdateBuild(build *model.Build) error { return meddler.Update(db, buildTable, build) } diff --git a/store/datastore/builds_test.go b/store/datastore/builds_test.go index 783578216..ed1147019 100644 --- a/store/datastore/builds_test.go +++ b/store/datastore/builds_test.go @@ -8,12 +8,23 @@ import ( ) func TestBuilds(t *testing.T) { + repo := &model.Repo{ + UserID: 1, + FullName: "bradrydzewski/drone", + Owner: "bradrydzewski", + Name: "drone", + } + db := openTest() defer db.Close() s := From(db) g := goblin.Goblin(t) g.Describe("Builds", func() { + g.Before(func() { + db.Exec("DELETE FROM repos") + s.CreateRepo(repo) + }) // before each test be sure to purge the package // table data from the database. @@ -24,11 +35,11 @@ func TestBuilds(t *testing.T) { g.It("Should Post a Build", func() { build := model.Build{ - RepoID: 1, + RepoID: repo.ID, Status: model.StatusSuccess, Commit: "85f8c029b902ed9400bc600bac301a0aadb144ac", } - err := s.CreateBuild(&build, []*model.Proc{}...) + err := s.CreateBuild(&build) g.Assert(err == nil).IsTrue() g.Assert(build.ID != 0).IsTrue() g.Assert(build.Number).Equal(1) @@ -37,12 +48,12 @@ func TestBuilds(t *testing.T) { g.It("Should Put a Build", func() { build := model.Build{ - RepoID: 1, + RepoID: repo.ID, Number: 5, Status: model.StatusSuccess, Commit: "85f8c029b902ed9400bc600bac301a0aadb144ac", } - s.CreateBuild(&build, []*model.Proc{}...) + s.CreateBuild(&build) build.Status = model.StatusRunning err1 := s.UpdateBuild(&build) getbuild, err2 := s.GetBuild(build.ID) @@ -56,7 +67,7 @@ func TestBuilds(t *testing.T) { g.It("Should Get a Build", func() { build := model.Build{ - RepoID: 1, + RepoID: repo.ID, Status: model.StatusSuccess, } s.CreateBuild(&build, []*model.Proc{}...) @@ -69,11 +80,11 @@ func TestBuilds(t *testing.T) { g.It("Should Get a Build by Number", func() { build1 := &model.Build{ - RepoID: 1, + RepoID: repo.ID, Status: model.StatusPending, } build2 := &model.Build{ - RepoID: 1, + RepoID: repo.ID, Status: model.StatusPending, } err1 := s.CreateBuild(build1, []*model.Proc{}...) @@ -89,12 +100,12 @@ func TestBuilds(t *testing.T) { g.It("Should Get a Build by Ref", func() { build1 := &model.Build{ - RepoID: 1, + RepoID: repo.ID, Status: model.StatusPending, Ref: "refs/pull/5", } build2 := &model.Build{ - RepoID: 1, + RepoID: repo.ID, Status: model.StatusPending, Ref: "refs/pull/6", } @@ -112,12 +123,12 @@ func TestBuilds(t *testing.T) { g.It("Should Get a Build by Ref", func() { build1 := &model.Build{ - RepoID: 1, + RepoID: repo.ID, Status: model.StatusPending, Ref: "refs/pull/5", } build2 := &model.Build{ - RepoID: 1, + RepoID: repo.ID, Status: model.StatusPending, Ref: "refs/pull/6", } @@ -135,13 +146,13 @@ func TestBuilds(t *testing.T) { g.It("Should Get a Build by Commit", func() { build1 := &model.Build{ - RepoID: 1, + RepoID: repo.ID, Status: model.StatusPending, Branch: "master", Commit: "85f8c029b902ed9400bc600bac301a0aadb144ac", } build2 := &model.Build{ - RepoID: 1, + RepoID: repo.ID, Status: model.StatusPending, Branch: "dev", Commit: "85f8c029b902ed9400bc600bac301a0aadb144aa", @@ -161,14 +172,14 @@ func TestBuilds(t *testing.T) { g.It("Should Get the last Build", func() { build1 := &model.Build{ - RepoID: 1, + RepoID: repo.ID, Status: model.StatusFailure, Branch: "master", Commit: "85f8c029b902ed9400bc600bac301a0aadb144ac", Event: model.EventPush, } build2 := &model.Build{ - RepoID: 1, + RepoID: repo.ID, Status: model.StatusSuccess, Branch: "master", Commit: "85f8c029b902ed9400bc600bac301a0aadb144aa", @@ -190,19 +201,19 @@ func TestBuilds(t *testing.T) { g.It("Should Get the last Build Before Build N", func() { build1 := &model.Build{ - RepoID: 1, + RepoID: repo.ID, Status: model.StatusFailure, Branch: "master", Commit: "85f8c029b902ed9400bc600bac301a0aadb144ac", } build2 := &model.Build{ - RepoID: 1, + RepoID: repo.ID, Status: model.StatusSuccess, Branch: "master", Commit: "85f8c029b902ed9400bc600bac301a0aadb144aa", } build3 := &model.Build{ - RepoID: 1, + RepoID: repo.ID, Status: model.StatusRunning, Branch: "master", Commit: "85f8c029b902ed9400bc600bac301a0aadb144aa", @@ -225,11 +236,11 @@ func TestBuilds(t *testing.T) { g.It("Should get recent Builds", func() { build1 := &model.Build{ - RepoID: 1, + RepoID: repo.ID, Status: model.StatusFailure, } build2 := &model.Build{ - RepoID: 1, + RepoID: repo.ID, Status: model.StatusSuccess, } s.CreateBuild(build1, []*model.Proc{}...) @@ -243,3 +254,58 @@ func TestBuilds(t *testing.T) { }) }) } + +func TestBuildIncrement(t *testing.T) { + s := newTest() + defer func() { + s.Exec("delete from repos") + s.Exec("delete from builds") + s.Close() + }() + + repo := &model.Repo{ + UserID: 1, + FullName: "bradrydzewski/drone", + Owner: "bradrydzewski", + Name: "drone", + } + if err := s.CreateRepo(repo); err != nil { + t.Error(err) + } + + num, err := s.incrementRepo(repo.ID, 0, 1) + if err != nil { + t.Error(err) + } + + if got, want := num, 1; got != want { + t.Errorf("Want repository counter incremented to %d, got %d", want, got) + } + + num, err = s.incrementRepo(repo.ID, 1, 2) + if err != nil { + t.Error(err) + } + + if got, want := num, 2; got != want { + t.Errorf("Want repository counter incremented to %d, got %d", want, got) + } + + // this block tests incrementing the repository counter + // should fail when attempting to increment the counter + // from a stale base. + num, _ = s.incrementRepo(repo.ID, 1, 2) + if num != 0 { + t.Errorf("Want error when trying to increment stale number") + } + + // this block tests incrementing the repository counter + // using the given repository id with backoff. + num, err = s.incrementRepoRetry(repo.ID) + if err != nil { + t.Error(err) + } + if got, want := num, 3; got != want { + t.Errorf("Want repository counter incremented to %d, got %d", want, got) + } +} diff --git a/store/datastore/config_test.go b/store/datastore/config_test.go index 9586fed0a..54b689820 100644 --- a/store/datastore/config_test.go +++ b/store/datastore/config_test.go @@ -79,7 +79,12 @@ func TestConfigApproved(t *testing.T) { t.Errorf("Unexpected error: insert config: %s", err) return } - + s.CreateRepo(&model.Repo{ + UserID: 1, + FullName: "bradrydzewski/drone", + Owner: "bradrydzewski", + Name: "drone", + }) s.CreateBuild(&model.Build{ RepoID: 1, ConfigID: conf.ID, diff --git a/store/datastore/ddl/mysql/ddl_gen.go b/store/datastore/ddl/mysql/ddl_gen.go index 75800edc8..e3f43b609 100644 --- a/store/datastore/ddl/mysql/ddl_gen.go +++ b/store/datastore/ddl/mysql/ddl_gen.go @@ -88,6 +88,14 @@ var migrations = []struct { name: "create-index-sender-repos", stmt: createIndexSenderRepos, }, + { + name: "alter-table-add-repo-seq", + stmt: alterTableAddRepoSeq, + }, + { + name: "update-table-set-repo-seq", + stmt: updateTableSetRepoSeq, + }, } // Migrate performs the database migration. If the migration fails @@ -150,7 +158,7 @@ func selectCompleted(db *sql.DB) (map[string]struct{}, error) { var migrationTableCreate = ` CREATE TABLE IF NOT EXISTS migrations ( - name VARCHAR(512) + name VARCHAR(255) ,UNIQUE(name) ) ` @@ -442,3 +450,19 @@ CREATE TABLE IF NOT EXISTS senders ( var createIndexSenderRepos = ` CREATE INDEX sender_repo_ix ON senders (sender_repo_id); ` + +// +// 014_add_column_repo_seq.sql +// + +var alterTableAddRepoSeq = ` +ALTER TABLE repos ADD COLUMN repo_counter INTEGER; +` + +var updateTableSetRepoSeq = ` +UPDATE repos SET repo_counter = ( + SELECT max(build_number) + FROM builds + WHERE builds.build_repo_id = repos.repo_id +) +` diff --git a/store/datastore/ddl/mysql/files/013_add_column_repo_seq.sql b/store/datastore/ddl/mysql/files/014_add_column_repo_seq.sql similarity index 100% rename from store/datastore/ddl/mysql/files/013_add_column_repo_seq.sql rename to store/datastore/ddl/mysql/files/014_add_column_repo_seq.sql diff --git a/store/datastore/ddl/postgres/ddl_gen.go b/store/datastore/ddl/postgres/ddl_gen.go index 81053a1ed..8a84d8a51 100644 --- a/store/datastore/ddl/postgres/ddl_gen.go +++ b/store/datastore/ddl/postgres/ddl_gen.go @@ -88,6 +88,14 @@ var migrations = []struct { name: "create-index-sender-repos", stmt: createIndexSenderRepos, }, + { + name: "alter-table-add-repo-seq", + stmt: alterTableAddRepoSeq, + }, + { + name: "update-table-set-repo-seq", + stmt: updateTableSetRepoSeq, + }, } // Migrate performs the database migration. If the migration fails @@ -150,7 +158,7 @@ func selectCompleted(db *sql.DB) (map[string]struct{}, error) { var migrationTableCreate = ` CREATE TABLE IF NOT EXISTS migrations ( - name VARCHAR(512) + name VARCHAR(255) ,UNIQUE(name) ) ` @@ -442,3 +450,19 @@ CREATE TABLE IF NOT EXISTS senders ( var createIndexSenderRepos = ` CREATE INDEX IF NOT EXISTS sender_repo_ix ON senders (sender_repo_id); ` + +// +// 014_add_column_repo_seq.sql +// + +var alterTableAddRepoSeq = ` +ALTER TABLE repos ADD COLUMN repo_counter INTEGER; +` + +var updateTableSetRepoSeq = ` +UPDATE repos SET repo_counter = ( + SELECT max(build_number) + FROM builds + WHERE builds.build_repo_id = repos.repo_id +) +` diff --git a/store/datastore/ddl/postgres/files/013_add_column_repo_seq.sql b/store/datastore/ddl/postgres/files/014_add_column_repo_seq.sql similarity index 100% rename from store/datastore/ddl/postgres/files/013_add_column_repo_seq.sql rename to store/datastore/ddl/postgres/files/014_add_column_repo_seq.sql diff --git a/store/datastore/ddl/sqlite/ddl_gen.go b/store/datastore/ddl/sqlite/ddl_gen.go index d00be5140..b7eb2fbeb 100644 --- a/store/datastore/ddl/sqlite/ddl_gen.go +++ b/store/datastore/ddl/sqlite/ddl_gen.go @@ -92,6 +92,14 @@ var migrations = []struct { name: "create-index-sender-repos", stmt: createIndexSenderRepos, }, + { + name: "alter-table-add-repo-seq", + stmt: alterTableAddRepoSeq, + }, + { + name: "update-table-set-repo-seq", + stmt: updateTableSetRepoSeq, + }, } // Migrate performs the database migration. If the migration fails @@ -154,7 +162,7 @@ func selectCompleted(db *sql.DB) (map[string]struct{}, error) { var migrationTableCreate = ` CREATE TABLE IF NOT EXISTS migrations ( - name VARCHAR(512) + name VARCHAR(255) ,UNIQUE(name) ) ` @@ -443,3 +451,19 @@ CREATE TABLE IF NOT EXISTS senders ( var createIndexSenderRepos = ` CREATE INDEX IF NOT EXISTS sender_repo_ix ON senders (sender_repo_id); ` + +// +// 014_add_column_repo_seq.sql +// + +var alterTableAddRepoSeq = ` +ALTER TABLE repos ADD COLUMN repo_counter INTEGER; +` + +var updateTableSetRepoSeq = ` +UPDATE repos SET repo_counter = ( + SELECT max(build_number) + FROM builds + WHERE builds.build_repo_id = repos.repo_id +) +` diff --git a/store/datastore/ddl/sqlite/files/013_add_column_repo_seq.sql b/store/datastore/ddl/sqlite/files/014_add_column_repo_seq.sql similarity index 100% rename from store/datastore/ddl/sqlite/files/013_add_column_repo_seq.sql rename to store/datastore/ddl/sqlite/files/014_add_column_repo_seq.sql diff --git a/store/datastore/sql/postgres/files/repos.sql b/store/datastore/sql/postgres/files/repos.sql new file mode 100644 index 000000000..0958acbf1 --- /dev/null +++ b/store/datastore/sql/postgres/files/repos.sql @@ -0,0 +1,5 @@ +-- name: repo-update-counter + +UPDATE repos SET repo_counter = ? +WHERE repo_counter = ? + AND repo_id = ? diff --git a/store/datastore/sql/postgres/sql_gen.go b/store/datastore/sql/postgres/sql_gen.go index e14760336..d731dfe5b 100644 --- a/store/datastore/sql/postgres/sql_gen.go +++ b/store/datastore/sql/postgres/sql_gen.go @@ -25,6 +25,7 @@ var index = map[string]string{ "registry-find-repo-addr": registryFindRepoAddr, "registry-delete-repo": registryDeleteRepo, "registry-delete": registryDelete, + "repo-update-counter": repoUpdateCounter, "secret-find-repo": secretFindRepo, "secret-find-repo-name": secretFindRepoName, "secret-delete": secretDelete, @@ -249,6 +250,12 @@ var registryDelete = ` DELETE FROM registry WHERE registry_id = $1 ` +var repoUpdateCounter = ` +UPDATE repos SET repo_counter = ? +WHERE repo_counter = ? + AND repo_id = ? +` + var secretFindRepo = ` SELECT secret_id diff --git a/store/datastore/sql/sqlite/files/repos.sql b/store/datastore/sql/sqlite/files/repos.sql new file mode 100644 index 000000000..0958acbf1 --- /dev/null +++ b/store/datastore/sql/sqlite/files/repos.sql @@ -0,0 +1,5 @@ +-- name: repo-update-counter + +UPDATE repos SET repo_counter = ? +WHERE repo_counter = ? + AND repo_id = ? diff --git a/store/datastore/sql/sqlite/sql_gen.go b/store/datastore/sql/sqlite/sql_gen.go index 1a06db44e..5c5329160 100644 --- a/store/datastore/sql/sqlite/sql_gen.go +++ b/store/datastore/sql/sqlite/sql_gen.go @@ -25,6 +25,7 @@ var index = map[string]string{ "registry-find-repo-addr": registryFindRepoAddr, "registry-delete-repo": registryDeleteRepo, "registry-delete": registryDelete, + "repo-update-counter": repoUpdateCounter, "secret-find-repo": secretFindRepo, "secret-find-repo-name": secretFindRepoName, "secret-delete": secretDelete, @@ -248,6 +249,12 @@ var registryDelete = ` DELETE FROM registry WHERE registry_id = ? ` +var repoUpdateCounter = ` +UPDATE repos SET repo_counter = ? +WHERE repo_counter = ? + AND repo_id = ? +` + var secretFindRepo = ` SELECT secret_id